From: Bernard Pidoux <pidoux@ccr•jussieu.fr>
To: Jarek Poplawski <jarkao2@o2•pl>
Cc: ralf@linux-mips•org, davem@davemloft•net, netdev@vger•kernel.org,
Alexey Dobriyan <adobriyan@gmail•com>
Subject: Re: Inconsistent lock state and possible irq lock inversion dependency detected in ax25.ko
Date: Tue, 04 Dec 2007 23:26:52 +0100 [thread overview]
Message-ID: <4755D42C.9000107@ccr.jussieu.fr> (raw)
In-Reply-To: <47532B60.8030205@o2.pl>
[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]
Jarek Poplawski wrote:
> Bernard Pidoux wrote, On 12/02/2007 06:37 PM:
>
>> Hi,
>>
>> Many thanks for your patch for ~/net/ax25/ax25_subr.c
>>
>> Introduction of local_bh_disable() ... local_bh_enable()
>>
>> cured the inconsistent lock state related to AX25 connect timeout.
>>
>> I have now a stable monoprocessor system running AX25 and ROSE network
>> packet switching application FPAC, whether kernel is compiled with or
>> without hack option.
>>
>> There is no more problem during normal operations.
>>
>> This was achieved, thanks to your AX25 patch and the patch from Alexey
>> Dobriyan for rose module.
>>
>> I also patched rose module in order to get packet routing more
>> efficient, taking into account the "restarted" flag that is raised when
>> a neighbour node is already connected.
>>
>> To summarize the present situation on my Linux machine, I built a patch
>> against kernel 2.6.23.9.
>>
>> I would appreciate if you could make it included into a next kernel release.
> ...
>
> Bernard, I'm very glad I could be a little helpful, but I'm not sure of
> your intentions: my patch proposal is rather trivial interpretation of
> lockdep's report; I haven't studied AX25 enough even to be sure there is
> a real lockup possible in this place. Since this change looks not very
> costly and quite safe, I can 'take a risk' to sign this off after your
> testing. But anything more is beyond my 'range'.
>
> So, since you've spent quite a lot of time on this all, maybe it would
> be simpler if you've tried the same with the current kernel, and resent
> "proper" (not gzipped and with changelog) patch or patches. Then, I hope,
> Ralf, as the maintainer, will make the rest.
>
> Regards,
> Jarek P.
>
>
As required I send again in clear text the summary of ax25 and rose
patch against 2.6.23.9.
As I said, the kernel stability, after applying these patch, is behind us.
I did not observe any warning nor lockup after a few days of AX25
intensive use.
Also rose module is handling routing of frames much more efficiently.
This will considerably help us to focus on application programs now.
I am now concentrating my efforts on ROSE/FPAC and Linux FBB code
adjustement.
Thanks to you all for your help.
73 de Bernard, f6bvp
[-- Attachment #2: rose-patch-2.6.23.9 --]
[-- Type: text/plain, Size: 5103 bytes --]
diff -pruN a/include/net/rose.h b/include/net/rose.h
--- a/include/net/rose.h 2007-10-12 18:43:44.000000000 +0200
+++ b/include/net/rose.h 2007-12-01 23:56:57.000000000 +0100
@@ -202,6 +202,7 @@ extern struct net_device *rose_dev_first
extern struct net_device *rose_dev_get(rose_address *);
extern struct rose_route *rose_route_free_lci(unsigned int, struct rose_neigh *);
extern struct rose_neigh *rose_get_neigh(rose_address *, unsigned char *, unsigned char *);
+extern struct rose_neigh *__rose_get_neigh(rose_address *, unsigned char *, unsigned char *);
extern int rose_rt_ioctl(unsigned int, void __user *);
extern void rose_link_failed(ax25_cb *, int);
extern int rose_route_frame(struct sk_buff *, ax25_cb *);
diff -pruN a/net/ax25/ax25_subr.c b/net/ax25/ax25_subr.c
--- a/net/ax25/ax25_subr.c 2007-10-12 18:43:44.000000000 +0200
+++ b/net/ax25/ax25_subr.c 2007-12-01 23:32:01.000000000 +0100
@@ -279,6 +279,7 @@ void ax25_disconnect(ax25_cb *ax25, int
ax25_link_failed(ax25, reason);
if (ax25->sk != NULL) {
+ local_bh_disable();
bh_lock_sock(ax25->sk);
ax25->sk->sk_state = TCP_CLOSE;
ax25->sk->sk_err = reason;
@@ -288,5 +289,6 @@ void ax25_disconnect(ax25_cb *ax25, int
sock_set_flag(ax25->sk, SOCK_DEAD);
}
bh_unlock_sock(ax25->sk);
+ local_bh_enable();
}
}
diff -pruN a/net/rose/af_rose.c b/net/rose/af_rose.c
--- a/net/rose/af_rose.c 2007-10-12 18:43:44.000000000 +0200
+++ b/net/rose/af_rose.c 2007-12-02 10:06:31.000000000 +0100
@@ -62,7 +62,7 @@ int sysctl_rose_window_size
static HLIST_HEAD(rose_list);
static DEFINE_SPINLOCK(rose_list_lock);
-static struct proto_ops rose_proto_ops;
+static const struct proto_ops rose_proto_ops;
ax25_address rose_callsign;
@@ -741,7 +741,7 @@ static int rose_connect(struct socket *s
sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
- rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause,
+ rose->neighbour = __rose_get_neigh(&addr->srose_addr, &cause,
&diagnostic);
if (!rose->neighbour)
return -ENETUNREACH;
@@ -773,7 +773,6 @@ static int rose_connect(struct socket *s
rose_insert_socket(sk); /* Finish the bind */
}
-rose_try_next_neigh:
rose->dest_addr = addr->srose_addr;
rose->dest_call = addr->srose_call;
rose->rand = ((long)rose & 0xFFFF) + rose->lci;
@@ -835,12 +834,6 @@ rose_try_next_neigh:
}
if (sk->sk_state != TCP_ESTABLISHED) {
- /* Try next neighbour */
- rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause, &diagnostic);
- if (rose->neighbour)
- goto rose_try_next_neigh;
-
- /* No more neighbours */
sock->state = SS_UNCONNECTED;
err = sock_error(sk); /* Always set at this point */
goto out_release;
@@ -1481,7 +1474,7 @@ static struct net_proto_family rose_fami
.owner = THIS_MODULE,
};
-static struct proto_ops rose_proto_ops = {
+static const struct proto_ops rose_proto_ops = {
.family = PF_ROSE,
.owner = THIS_MODULE,
.release = rose_release,
Les fichiers linux-2.6.23.9-orig/net/rose/rose.ko et linux-2.6.23.9/net/rose/rose.ko sont différents.
diff -pruN a/net/rose/rose_route.c b/net/rose/rose_route.c
--- a/net/rose/rose_route.c 2007-10-12 18:43:44.000000000 +0200
+++ b/net/rose/rose_route.c 2007-12-02 00:15:24.000000000 +0100
@@ -664,25 +664,22 @@ struct rose_route *rose_route_free_lci(u
/*
* Find a neighbour given a ROSE address.
*/
-struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
+struct rose_neigh *__rose_get_neigh(rose_address *addr, unsigned char *cause,
unsigned char *diagnostic)
{
- struct rose_neigh *res = NULL;
struct rose_node *node;
int failed = 0;
int i;
- spin_lock_bh(&rose_node_list_lock);
for (node = rose_node_list; node != NULL; node = node->next) {
if (rosecmpm(addr, &node->address, node->mask) == 0) {
for (i = 0; i < node->count; i++) {
- if (!rose_ftimer_running(node->neighbour[i])) {
- res = node->neighbour[i];
- goto out;
- } else
- failed = 1;
+ if (node->neighbour[i]->restarted)
+ return node->neighbour[i];
+ if (!rose_ftimer_running(node->neighbour[i]))
+ return node->neighbour[i];
+ failed = 1;
}
- break;
}
}
@@ -694,7 +691,16 @@ struct rose_neigh *rose_get_neigh(rose_a
*diagnostic = 0;
}
-out:
+ return NULL;
+}
+
+struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
+ unsigned char *diagnostic)
+{
+ struct rose_neigh *res;
+
+ spin_lock_bh(&rose_node_list_lock);
+ res = __rose_get_neigh(addr, cause, diagnostic);
spin_unlock_bh(&rose_node_list_lock);
return res;
@@ -1019,7 +1025,7 @@ int rose_route_frame(struct sk_buff *skb
rose_route = rose_route->next;
}
- if ((new_neigh = rose_get_neigh(dest_addr, &cause, &diagnostic)) == NULL) {
+ if ((new_neigh = __rose_get_neigh(dest_addr, &cause, &diagnostic)) == NULL) {
rose_transmit_clear_request(rose_neigh, lci, cause, diagnostic);
goto out;
}
next prev parent reply other threads:[~2007-12-05 0:34 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-03 18:54 [PATCH] Fix rose.ko oops on unload Alexey Dobriyan
2007-10-03 19:04 ` Jeff Garzik
2007-10-03 19:21 ` Alexey Dobriyan
2007-10-08 6:44 ` David Miller
2007-12-14 21:58 ` [PATCH] [ROSE] reverts commits d85838c55d836c33077344fab424f200f2827d84 Bernard Pidoux
2007-11-21 22:13 ` Inconsistent lock state and possible irq lock inversion dependency detected in ax25.ko Bernard Pidoux
2007-11-28 13:48 ` Jarek Poplawski
2007-12-02 17:37 ` Bernard Pidoux
2007-12-02 22:02 ` Jarek Poplawski
2007-12-04 22:26 ` Bernard Pidoux [this message]
2007-12-04 23:17 ` Jarek Poplawski
2007-12-05 0:45 ` Ralf Baechle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4755D42C.9000107@ccr.jussieu.fr \
--to=pidoux@ccr$(echo .)jussieu.fr \
--cc=adobriyan@gmail$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=jarkao2@o2$(echo .)pl \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ralf@linux-mips$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox