* [patch] ipv4: fix lock usage in udp_ioctl
@ 2006-06-14 19:43 Heiko Carstens
2006-06-14 22:16 ` David Miller
2006-06-14 23:12 ` Herbert Xu
0 siblings, 2 replies; 6+ messages in thread
From: Heiko Carstens @ 2006-06-14 19:43 UTC (permalink / raw)
To: David S. Miller, Jeff Garzik, Andrew Morton
Cc: linux-kernel, netdev, Ingo Molnar, Frank Pavlic
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Fix lock usage in udp_ioctl().
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
udp_poll() seems to have the same problem, right?
As reported by the lock validator:
============================
[ BUG: illegal lock usage! ]
----------------------------
illegal {in-hardirq-W} -> {hardirq-on-W} usage.
syslogd/739 [HC0[0]:SC0[1]:HE1:SE0] takes:
(&list->lock){++..}, at: [<002e36d6>] udp_ioctl+0x96/0x100
{in-hardirq-W} state was registered at:
[<00062128>] lock_acquire+0x9c/0xc0
[<0036209e>] _spin_lock_irqsave+0x66/0x84
[<002912ce>] skb_dequeue+0x32/0xb0
[<00263160>] qeth_qdio_output_handler+0x3e8/0xf8c
[<00219fdc>] tiqdio_thinint_handler+0xde0/0x2234
[<0020448c>] do_adapter_IO+0x5c/0xa8
[<0020842c>] do_IRQ+0x13c/0x18c
[<000208a2>] io_no_vtime+0x16/0x1c
[<0001978c>] cpu_idle+0x1d0/0x20c
irq event stamp: 1694
hardirqs last enabled at (1693): [<003629c2>] _spin_unlock_irqrestore+0x92/0xa8
hardirqs last disabled at (1692): [<00362074>] _spin_lock_irqsave+0x3c/0x84
softirqs last enabled at (1682): [<0028c7c4>] release_sock+0xe4/0xf4
softirqs last disabled at (1694): [<00361f7e>] _spin_lock_bh+0x2e/0x70
other info that might help us debug this:
no locks held by syslogd/739.
stack backtrace:
000000000fd6c148 000000000de2f960 0000000000000002 0000000000000000
000000000de2fa00 000000000de2f978 000000000de2f978 000000000001737c
0000000000000000 0000000000000000 0000000000000000 0000000000000000
000000000de2f960 000000000000000c 000000000de2f960 000000000de2f9d0
000000000036fe70 000000000001737c 000000000de2f960 000000000de2f9b0
Call Trace:
([<000000000001730a>] show_trace+0x166/0x16c)
[<00000000000173d6>] show_stack+0xc6/0xf8
[<0000000000017436>] dump_stack+0x2e/0x3c
[<000000000005f978>] print_usage_bug+0x23c/0x250
[<00000000000607cc>] mark_lock+0x594/0x714
[<00000000000613be>] __lock_acquire+0x252/0xf20
[<0000000000062128>] lock_acquire+0x9c/0xc0
[<0000000000361fa8>] _spin_lock_bh+0x58/0x70
[<00000000002e36d6>] udp_ioctl+0x96/0x100
[<00000000002eadd6>] inet_ioctl+0x72/0x11c
[<00000000002893f2>] sock_ioctl+0x1ca/0x2c0
[<00000000000c13ee>] do_ioctl+0x56/0xe0
[<00000000000c14f2>] vfs_ioctl+0x7a/0x384
[<00000000000c184e>] sys_ioctl+0x52/0x84
[<00000000000e80a2>] do_ioctl32_pointer+0x2a/0x3c
[<00000000000e55c8>] compat_sys_ioctl+0x168/0x378
[<0000000000020338>] sysc_noemu+0x10/0x16
diffstat:
net/ipv4/udp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 3f93292..b15a17b 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -740,7 +740,7 @@ int udp_ioctl(struct sock *sk, int cmd,
unsigned long amount;
amount = 0;
- spin_lock_bh(&sk->sk_receive_queue.lock);
+ spin_lock_irq(&sk->sk_receive_queue.lock);
skb = skb_peek(&sk->sk_receive_queue);
if (skb != NULL) {
/*
@@ -750,7 +750,7 @@ int udp_ioctl(struct sock *sk, int cmd,
*/
amount = skb->len - sizeof(struct udphdr);
}
- spin_unlock_bh(&sk->sk_receive_queue.lock);
+ spin_unlock_irq(&sk->sk_receive_queue.lock);
return put_user(amount, (int __user *)arg);
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] ipv4: fix lock usage in udp_ioctl
2006-06-14 19:43 [patch] ipv4: fix lock usage in udp_ioctl Heiko Carstens
@ 2006-06-14 22:16 ` David Miller
2006-06-14 23:12 ` Herbert Xu
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2006-06-14 22:16 UTC (permalink / raw)
To: heiko.carstens; +Cc: jgarzik, akpm, linux-kernel, netdev, mingo, fpavlic
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Wed, 14 Jun 2006 21:43:05 +0200
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>
> Fix lock usage in udp_ioctl().
>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
More likely the qeth driver shouldn't call into the socket code in
hardware interrupt context. From your logs that's what it seems is
happening.
The socket receive queue should only be touched in software
interrupt context, never in hardware interrupt context. That's
why the locking does BH disabling at best.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ipv4: fix lock usage in udp_ioctl
2006-06-14 19:43 [patch] ipv4: fix lock usage in udp_ioctl Heiko Carstens
2006-06-14 22:16 ` David Miller
@ 2006-06-14 23:12 ` Herbert Xu
2006-06-15 5:28 ` Ingo Molnar
1 sibling, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2006-06-14 23:12 UTC (permalink / raw)
To: Heiko Carstens; +Cc: davem, jgarzik, akpm, linux-kernel, netdev, mingo, fpavlic
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
>
> As reported by the lock validator:
>
> ============================
> [ BUG: illegal lock usage! ]
> ----------------------------
> illegal {in-hardirq-W} -> {hardirq-on-W} usage.
> syslogd/739 [HC0[0]:SC0[1]:HE1:SE0] takes:
> (&list->lock){++..}, at: [<002e36d6>] udp_ioctl+0x96/0x100
> {in-hardirq-W} state was registered at:
> [<00062128>] lock_acquire+0x9c/0xc0
> [<0036209e>] _spin_lock_irqsave+0x66/0x84
> [<002912ce>] skb_dequeue+0x32/0xb0
> [<00263160>] qeth_qdio_output_handler+0x3e8/0xf8c
> [<00219fdc>] tiqdio_thinint_handler+0xde0/0x2234
> [<0020448c>] do_adapter_IO+0x5c/0xa8
> [<0020842c>] do_IRQ+0x13c/0x18c
> [<000208a2>] io_no_vtime+0x16/0x1c
> [<0001978c>] cpu_idle+0x1d0/0x20c
This is bogus. These two locks belong to two different queues and they
never intersect.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ipv4: fix lock usage in udp_ioctl
2006-06-14 23:12 ` Herbert Xu
@ 2006-06-15 5:28 ` Ingo Molnar
2006-06-15 6:55 ` Heiko Carstens
0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2006-06-15 5:28 UTC (permalink / raw)
To: Herbert Xu
Cc: Heiko Carstens, davem, jgarzik, akpm, linux-kernel, netdev,
fpavlic
* Herbert Xu <herbert@gondor.apana.org.au> wrote:
> This is bogus. These two locks belong to two different queues and
> they never intersect.
yeah - qeth does its own skb-queue management here, and it's done in an
irq-safe manner.
Heiko, in qeth_main.c, could you do something like:
+ static struct lockdep_type_key qdio_out_skb_queue_key;
...
skb_queue_head_init(&card->qdio.out_qs[i]->bufs[j].
skb_list);
+ lockdep_reinit_key(&card->qdio.out_qs[i]->bufs[j].skb_list,
&qdio_out_skb_queue_key)
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ipv4: fix lock usage in udp_ioctl
2006-06-15 5:28 ` Ingo Molnar
@ 2006-06-15 6:55 ` Heiko Carstens
2006-06-15 10:52 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Heiko Carstens @ 2006-06-15 6:55 UTC (permalink / raw)
To: Ingo Molnar
Cc: Herbert Xu, davem, jgarzik, akpm, linux-kernel, netdev, fpavlic
On Thu, Jun 15, 2006 at 08:28:07AM +0200, Ingo Molnar wrote:
>
> * Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> > This is bogus. These two locks belong to two different queues and
> > they never intersect.
>
> yeah - qeth does its own skb-queue management here, and it's done in an
> irq-safe manner.
>
> Heiko, in qeth_main.c, could you do something like:
>
> + static struct lockdep_type_key qdio_out_skb_queue_key;
>
> ...
> skb_queue_head_init(&card->qdio.out_qs[i]->bufs[j].
> skb_list);
> + lockdep_reinit_key(&card->qdio.out_qs[i]->bufs[j].skb_list,
> &qdio_out_skb_queue_key)
How about the patch below? The warning goes away and I assume "tmp_list" needs
lockdep_reinit_key too, since it should have the same locking rules as the
rest of qeth's skb-queue management.
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Avoid false positive illegal lock usage message in qeth driver.
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
drivers/s390/net/qeth_main.c | 6 ++++++
1 file changed, 6 insertions(+)
--- a/drivers/s390/net/qeth_main.c 2006-06-15 08:46:26.000000000 +0200
+++ b/drivers/s390/net/qeth_main.c 2006-06-15 08:29:58.000000000 +0200
@@ -85,6 +85,8 @@ static debug_info_t *qeth_dbf_qerr = NUL
DEFINE_PER_CPU(char[256], qeth_dbf_txt_buf);
+static struct lockdep_type_key qdio_out_skb_queue_key;
+
/**
* some more definitions and declarations
*/
@@ -3230,6 +3232,9 @@ qeth_alloc_qdio_buffers(struct qeth_card
&card->qdio.out_qs[i]->qdio_bufs[j];
skb_queue_head_init(&card->qdio.out_qs[i]->bufs[j].
skb_list);
+ lockdep_reinit_key(
+ &card->qdio.out_qs[i]->bufs[j].skb_list.lock,
+ &qdio_out_skb_queue_key);
INIT_LIST_HEAD(&card->qdio.out_qs[i]->bufs[j].ctx_list);
}
}
@@ -5273,6 +5278,7 @@ qeth_free_vlan_buffer(struct qeth_card *
struct sk_buff_head tmp_list;
skb_queue_head_init(&tmp_list);
+ lockdep_reinit_key(&tmp_list.lock, &qdio_out_skb_queue_key);
for(i = 0; i < QETH_MAX_BUFFER_ELEMENTS(card); ++i){
while ((skb = skb_dequeue(&buf->skb_list))){
if (vlan_tx_tag_present(skb) &&
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ipv4: fix lock usage in udp_ioctl
2006-06-15 6:55 ` Heiko Carstens
@ 2006-06-15 10:52 ` Ingo Molnar
0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2006-06-15 10:52 UTC (permalink / raw)
To: Heiko Carstens
Cc: Herbert Xu, davem, jgarzik, akpm, linux-kernel, netdev, fpavlic
* Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> How about the patch below? The warning goes away and I assume
> "tmp_list" needs lockdep_reinit_key too, since it should have the same
> locking rules as the rest of qeth's skb-queue management.
yeah, looks good.
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-06-15 10:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-14 19:43 [patch] ipv4: fix lock usage in udp_ioctl Heiko Carstens
2006-06-14 22:16 ` David Miller
2006-06-14 23:12 ` Herbert Xu
2006-06-15 5:28 ` Ingo Molnar
2006-06-15 6:55 ` Heiko Carstens
2006-06-15 10:52 ` Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).