From mboxrd@z Thu Jan 1 00:00:00 1970 From: "David S. Miller" Subject: Re: [BUG 2.6.0-test11] pcnet32 oops Date: Fri, 5 Dec 2003 16:59:00 -0800 Sender: netdev-bounce@oss.sgi.com Message-ID: <20031205165900.2920ea6a.davem@redhat.com> References: <20031205234510.GA2319@bougret.hpl.hp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: jt@bougret.hpl.hp.com, linux-kernel@vger.kernel.org, tsbogend@alpha.franken.de, jgarzik@pobox.com, netdev@oss.sgi.com Return-path: To: jt@hpl.hp.com In-Reply-To: <20031205234510.GA2319@bougret.hpl.hp.com> Errors-to: netdev-bounce@oss.sgi.com List-Id: netdev.vger.kernel.org On Fri, 5 Dec 2003 15:45:10 -0800 Jean Tourrilhes wrote: > Badness in local_bh_enable at kernel/softirq.c:121 > Call Trace: > [] local_bh_enable+0x35/0x58 > [] ip_ct_find_proto+0xd2/0xd8 > [] destroy_conntrack+0x13/0x19c > [] __kfree_skb+0xc8/0xfc > [] pcnet32_purge_tx_ring+0x94/0xc4 [pcnet32] > [] pcnet32_restart+0x14/0x6c [pcnet32] > [] 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);