Re: [patch] ipv4: fix lock usage in udp_ioctl
* Heiko Carstens <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: fix lock usage in udp_ioctl
On Thu, Jun 15, 2006 at 08:28:07AM +0200, Ingo Molnar wrote:
>
> * Herbert Xu <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
Avoid false positive illegal lock usage message in qeth driver.
Signed-off-by: Heiko Carstens <[EMAIL PROTECTED]>
---
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.0 +0200
+++ b/drivers/s390/net/qeth_main.c 2006-06-15 08:29:58.0 +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) &&
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: fix lock usage in udp_ioctl
* Herbert Xu <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: fix lock usage in udp_ioctl
Heiko Carstens <[EMAIL PROTECTED]> 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~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: fix lock usage in udp_ioctl
From: Heiko Carstens <[EMAIL PROTECTED]> Date: Wed, 14 Jun 2006 21:43:05 +0200 > From: Heiko Carstens <[EMAIL PROTECTED]> > > Fix lock usage in udp_ioctl(). > > Signed-off-by: Heiko Carstens <[EMAIL PROTECTED]> 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. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] ipv4: fix lock usage in udp_ioctl
From: Heiko Carstens <[EMAIL PROTECTED]>
Fix lock usage in udp_ioctl().
Signed-off-by: Heiko Carstens <[EMAIL PROTECTED]>
---
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:
0fd6c148 0de2f960 0002
0de2fa00 0de2f978 0de2f978 0001737c
0de2f960 000c 0de2f960 0de2f9d0
0036fe70 0001737c 0de2f960 0de2f9b0
Call Trace:
([<0001730a>] show_trace+0x166/0x16c)
[<000173d6>] show_stack+0xc6/0xf8
[<00017436>] dump_stack+0x2e/0x3c
[<0005f978>] print_usage_bug+0x23c/0x250
[<000607cc>] mark_lock+0x594/0x714
[<000613be>] __lock_acquire+0x252/0xf20
[<00062128>] lock_acquire+0x9c/0xc0
[<00361fa8>] _spin_lock_bh+0x58/0x70
[<002e36d6>] udp_ioctl+0x96/0x100
[<002eadd6>] inet_ioctl+0x72/0x11c
[<002893f2>] sock_ioctl+0x1ca/0x2c0
[<000c13ee>] do_ioctl+0x56/0xe0
[<000c14f2>] vfs_ioctl+0x7a/0x384
[<000c184e>] sys_ioctl+0x52/0x84
[<000e80a2>] do_ioctl32_pointer+0x2a/0x3c
[<000e55c8>] compat_sys_ioctl+0x168/0x378
[<00020338>] 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);
}
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
