Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757914AbXKWUJW (ORCPT ); Fri, 23 Nov 2007 15:09:22 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756903AbXKWUJH (ORCPT ); Fri, 23 Nov 2007 15:09:07 -0500 Received: from relay.2ka.mipt.ru ([194.85.82.65]:33500 "EHLO 2ka.mipt.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752627AbXKWUJF (ORCPT ); Fri, 23 Nov 2007 15:09:05 -0500 Date: Fri, 23 Nov 2007 23:07:47 +0300 From: Evgeniy Polyakov To: Matt Mackall Cc: Andrew Morton , Simon Arlott , Linux Kernel Mailing List , netdev@vger.kernel.org, Ingo Molnar Subject: Re: 2.6.23 WARNING: at kernel/softirq.c:139 local_bh_enable() Message-ID: <20071123200747.GA18433@2ka.mipt.ru> References: <20071123105518.GA22062@2ka.mipt.ru> <20071123170756.GV19691@waste.org> <20071123175757.GA23991@2ka.mipt.ru> <20071123184851.GA14415@2ka.mipt.ru> <20071123185101.GA17582@2ka.mipt.ru> <20071123185906.GA23710@2ka.mipt.ru> <20071123191120.GA19691@waste.org> <20071123193222.GA22168@2ka.mipt.ru> <20071123194139.GC19691@waste.org> <20071123195410.GA11401@2ka.mipt.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20071123195410.GA11401@2ka.mipt.ru> User-Agent: Mutt/1.5.9i Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2440 Lines: 76 On Fri, Nov 23, 2007 at 10:54:10PM +0300, Evgeniy Polyakov (johnpol@2ka.mipt.ru) wrote: > On Fri, Nov 23, 2007 at 01:41:39PM -0600, Matt Mackall (mpm@selenic.com) wrote: > > Here's another thought: move all this logic into the networking core, > > unify it with current softirq zapper, then allow it to be called from > > various other places (like atomic allocators). Then it'll all be in > > central maintained place with more users. > > This can be done quite easily - put a check into __kfree_skb() if > netpoll is compiled-in and we are in hardirq context, then put skb > into softirq freeing queue. Then zap_completion_queue() can free > anything without ever knowing about nature of the packet, since this > will be checked in __kfree_skb() anyway. And let's add some mess... But should fix the case when netpoll code is being executed in interrupt context and is about to free skb, which should not be freed. Frankly saying this looks like crap. Crap-added-by: Evgeniy Polyakov diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 758dafe..88f8ea9 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -196,10 +196,7 @@ static void zap_completion_queue(void) while (clist != NULL) { struct sk_buff *skb = clist; clist = clist->next; - if (skb->destructor) - dev_kfree_skb_any(skb); /* put this one back */ - else - __kfree_skb(skb); + __kfree_skb(skb); } } diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 27cfe5f..8642097 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -318,6 +318,26 @@ void kfree_skbmem(struct sk_buff *skb) void __kfree_skb(struct sk_buff *skb) { +#if defined(CONFIG_NETPOLL) || defined(CONFIG_NETPOLL_TRAP) + if (in_irq() || irqs_disabled()) { + if (skb->destructor) { + dev_kfree_skb_irq(skb); + return; + } +#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE) + if (skb->nfct || skb->nfct_reasm) { + dev_kfree_skb_irq(skb); + return; + } +#endif +#ifdef CONFIG_XFRM + if (skb->sp) { + dev_kfree_skb_irq(skb); + return; + } +#endif + } +#endif dst_release(skb->dst); #ifdef CONFIG_XFRM secpath_put(skb->sp); -- Evgeniy Polyakov - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/