Subject: Re: [BUG 2.6.0-test11] pcnet32 oops

From: "David S. Miller" <[email-masked]>
Date: Fri, 5 Dec 2003 16:59:00 -0800

On Fri, 5 Dec 2003 15:45:10 -0800
Jean Tourrilhes <[email-masked]> wrote:

> Badness in local_bh_enable at kernel/softirq.c:121
> Call Trace:
>  [<c011ff91>] local_bh_enable+0x35/0x58
>  [<c022d19a>] ip_ct_find_proto+0xd2/0xd8
>  [<c022d923>] destroy_conntrack+0x13/0x19c
>  [<c01e68c8>] __kfree_skb+0xc8/0xfc
>  [<d085b160>] pcnet32_purge_tx_ring+0x94/0xc4 [pcnet32]
>  [<d085b300>] pcnet32_restart+0x14/0x6c [pcnet32]
>  [<d085c089>] pcnet32_set_multicast_list+0x7d/0x90 [pcnet32]

This is the classic case of doing disabling/enabling of software
interrupts with hardware interrupts disabled, which is a bug.

In this case pcnet32_set_multicast_list() is disabling hardware
interrupts, and the packet freeing of pcnet32_purge_tx_ring()
is what leads to the software interrupt disable/enable.

However, I'm inclined to believe that we should change dev_kfree_skb_any()
to fix this class of problems, by making it check for hardware interrupts
being disabled as well as being in an interrupt.

But we might not want to do it like this, just reenabling the hardware
interrupts at the top level won't cause the software interrupt to fire
to actually execute the SKB freeing work.... Need to think about this
some more.

===== include/linux/netdevice.h 1.66 vs edited =====
--- 1.66/include/linux/netdevice.h	Sat Nov  1 14:11:04 2003
+++ edited/include/linux/netdevice.h	Fri Dec  5 16:53:01 2003
@@ -634,7 +634,7 @@
  */
 static inline void dev_kfree_skb_any(struct sk_buff *skb)
 {
-	if (in_irq())
+	if (in_irq() || irqs_disabled())
 		dev_kfree_skb_irq(skb);
 	else
 		dev_kfree_skb(skb);