From patchwork Thu Dec 11 18:15:28 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Neil Horman X-Patchwork-Id: 13471 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 5EFC2DDF1F for ; Fri, 12 Dec 2008 05:18:01 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755487AbYLKSRw (ORCPT ); Thu, 11 Dec 2008 13:17:52 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755525AbYLKSRw (ORCPT ); Thu, 11 Dec 2008 13:17:52 -0500 Received: from charlotte.tuxdriver.com ([70.61.120.58]:53235 "EHLO smtp.tuxdriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755349AbYLKSRv (ORCPT ); Thu, 11 Dec 2008 13:17:51 -0500 Received: from nat-pool-rdu.redhat.com ([66.187.233.202] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1LAq6b-0004Zt-0X; Thu, 11 Dec 2008 13:17:45 -0500 Date: Thu, 11 Dec 2008 13:15:28 -0500 From: Neil Horman To: Stephen Hemminger Cc: Jarek Poplawski , netdev@vger.kernel.org, davem@davemloft.net Subject: Re: [PATCH] netpoll: fix race on poll_list resulting in garbage entry Message-ID: <20081211181528.GB10558@hmsendeavour.rdu.redhat.com> References: <20081209210644.GC3785@hmsreliant.think-freely.org> <20081211130728.GB5910@ff.dom.local> <20081211090104.02cab3d4@s6510> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20081211090104.02cab3d4@s6510> User-Agent: Mutt/1.5.12-2006-07-14 X-Spam-Score: -1.4 (-) X-Spam-Status: No Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Dec 11, 2008 at 09:01:04AM -0800, Stephen Hemminger wrote: > On Thu, 11 Dec 2008 13:07:28 +0000 > Jarek Poplawski wrote: > > > On 09-12-2008 22:06, Neil Horman wrote: > > ... > > > When executing napi->poll from the netpoll_path, this bit will > > > be set. When a driver calls netif_rx_complete, if that bit is set, it will not > > > remove the napi_struct from the poll_list. That work will be saved for the next > > > iteration of net_rx_action. > > > > This could be not enough: some drivers, e.g. sky2, call napi_complete() > > directly. > > > > There is good reason for this. Although most drivers only have one NAPI > instance per device, and multiqueue drivers have several NAPI structures > per device, a few devices like sky2 need to support multiple devices > running off one NAPI receive. The Marvell hardware has a common receive > interrupt for both ports on a dual port card. > > This kind of hardware limits usage of netpoll. Only one port can be > used with netpoll because netpoll makes assumptions about NAPI > association. > There was previously good cause to use __netif_rx_complete instead of netif_rx_complete some time ago when multiqueue rx was implemented using a set of dummy netdevices. But with the separation of the napi code, there is no longer any reason for this to be done. I just took a quick look, and it appears that sky2 is the last remaining driver to use the underlying napi routines. This patch maintains exactly the same functionality that it previously had, but allows for the netpoll patch to be safe with respect to the per-cpu poll_lists used by net_rx_action. Regards Neil Signed-off-by: Neil Horman sky2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c index 3813d15..84bdc3c 100644 --- a/drivers/net/sky2.c +++ b/drivers/net/sky2.c @@ -2694,7 +2694,7 @@ static int sky2_poll(struct napi_struct *napi, int work_limit) sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_STOP); sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_START); } - napi_complete(napi); + netif_rx_complete(napi->dev, napi); sky2_read32(hw, B0_Y2_SP_LISR); done: