From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTP id DDC5367B6C for ; Sat, 9 Dec 2006 09:47:26 +1100 (EST) Subject: Re: [PATCH 3/16] Spidernet RX Locking From: Benjamin Herrenschmidt To: Linas Vepstas In-Reply-To: <20061207175046.GB4614@austin.ibm.com> References: <20061206223223.GH17931@austin.ibm.com> <20061206233134.GC4649@austin.ibm.com> <4577E850.6040500@pobox.com> <20061207175046.GB4614@austin.ibm.com> Content-Type: text/plain Date: Sat, 09 Dec 2006 09:47:05 +1100 Message-Id: <1165618025.1103.85.camel@localhost.localdomain> Mime-Version: 1.0 Cc: Andrew Morton , Arnd Bergmann , netdev@vger.kernel.org, James K Lewis , linuxppc-dev@ozlabs.org, Jeff Garzik List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , A spinlock is expensive in the fast path, which is why Jeff says it's invasive. > spider_net_decode_one_descr() is called from > spider_net_poll() (which is the netdev->poll callback) > and also from spider_net_handle_rxram_full(). > > The rxramfull routine is called from a tasklet that > is fired off after a "RX ram full" interrupt is receved. > This interrupt is generated when the hardware runs out > of space to store incoming packets. We are seeing this > interrupt fire when the CPU is heavily loaded, and a > lot of traffic is being fired at the device. How often does that interrupt happen in that case ? A better approach is to keep the fast path (ie. poll()) lockless, and in handle_rxram_full(), the slow path, protect against poll using netif_disable_poll(). Though that means using a work queue, not a tasklet, since it needs to schedule. > > and what other > > non-sledgehammer approaches were discarded before arriving at this one? > > Well, I'm not that good at kernel programming, so I guess > I did not perceive this as a "sledgehammer." And alternative > approach is to simply ignore the rxramfull interrupt entirely, > and depend on poll() do all the work. I'll try this shortly. or you can schedule rx work from the rxramfull interrupt after setting a "something bad happened" flag. Then, poll can check this flag and do the right thing. Ben. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Herrenschmidt Subject: Re: [PATCH 3/16] Spidernet RX Locking Date: Sat, 09 Dec 2006 09:47:05 +1100 Message-ID: <1165618025.1103.85.camel@localhost.localdomain> References: <20061206223223.GH17931@austin.ibm.com> <20061206233134.GC4649@austin.ibm.com> <4577E850.6040500@pobox.com> <20061207175046.GB4614@austin.ibm.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Jeff Garzik , Andrew Morton , Arnd Bergmann , netdev@vger.kernel.org, James K Lewis , linuxppc-dev@ozlabs.org Return-path: Received: from gate.crashing.org ([63.228.1.57]:34695 "EHLO gate.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761248AbWLHWrg (ORCPT ); Fri, 8 Dec 2006 17:47:36 -0500 To: Linas Vepstas In-Reply-To: <20061207175046.GB4614@austin.ibm.com> Sender: netdev-owner@vger.kernel.org List-Id: netdev.vger.kernel.org A spinlock is expensive in the fast path, which is why Jeff says it's invasive. > spider_net_decode_one_descr() is called from > spider_net_poll() (which is the netdev->poll callback) > and also from spider_net_handle_rxram_full(). > > The rxramfull routine is called from a tasklet that > is fired off after a "RX ram full" interrupt is receved. > This interrupt is generated when the hardware runs out > of space to store incoming packets. We are seeing this > interrupt fire when the CPU is heavily loaded, and a > lot of traffic is being fired at the device. How often does that interrupt happen in that case ? A better approach is to keep the fast path (ie. poll()) lockless, and in handle_rxram_full(), the slow path, protect against poll using netif_disable_poll(). Though that means using a work queue, not a tasklet, since it needs to schedule. > > and what other > > non-sledgehammer approaches were discarded before arriving at this one? > > Well, I'm not that good at kernel programming, so I guess > I did not perceive this as a "sledgehammer." And alternative > approach is to simply ignore the rxramfull interrupt entirely, > and depend on poll() do all the work. I'll try this shortly. or you can schedule rx work from the rxramfull interrupt after setting a "something bad happened" flag. Then, poll can check this flag and do the right thing. Ben.