From patchwork Thu Sep 20 08:06:56 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joakim Tjernlund X-Patchwork-Id: 185357 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.180.67]) by ozlabs.org (Postfix) with ESMTP id 80A892C007D for ; Thu, 20 Sep 2012 18:07:16 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751749Ab2ITIHL (ORCPT ); Thu, 20 Sep 2012 04:07:11 -0400 Received: from gw1.transmode.se ([195.58.98.146]:62182 "EHLO gw1.transmode.se" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751601Ab2ITIHA (ORCPT ); Thu, 20 Sep 2012 04:07:00 -0400 Received: from mail1.transmode.se (mail1.transmode.se [192.168.201.18]) by gw1.transmode.se (Postfix) with ESMTP id F055D258378; Thu, 20 Sep 2012 10:06:57 +0200 (CEST) In-Reply-To: <20120919223416.GA16087@electric-eye.fr.zoreil.com> References: <1347987385-19071-1-git-send-email-Joakim.Tjernlund@transmode.se> <20120919223416.GA16087@electric-eye.fr.zoreil.com> Subject: Re: [PATCH 1/5] ucc_geth: Reduce IRQ off in xmit path X-KeepSent: 540E33B1:6013BC69-C1257A7F:002C410C; type=4; name=$KeepSent To: Francois Romieu Cc: netdev@vger.kernel.org X-Mailer: Lotus Notes Release 8.5.3 September 15, 2011 Message-ID: From: Joakim Tjernlund Date: Thu, 20 Sep 2012 10:06:56 +0200 X-MIMETrack: Serialize by Router on mail1/Transmode(Release 8.5.3FP1|March 07, 2012) at 20/09/2012 10:06:57 MIME-Version: 1.0 Content-type: text/plain; charset=US-ASCII Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Francois Romieu wrote on 2012/09/20 00:34:16: > > Joakim Tjernlund : > > Currently ucc_geth_start_xmit wraps IRQ off for the > > whole body just to be safe. > > Reduce the IRQ off period to a minimum. > > It opens a window in ucc_geth_start_xmit where the skb slot in > ugeth->tx_skbuff[txQ] is set and T_RA has not been written into > the descriptor status. Consider a racing poll : the !skb test in > ucc_geth_tx may not work as expected. Right, good catch! Surprisingly the driver never showed any malfunction even though I hit it pretty hard. I will send a V2 of this patch where I move the assignment inside the IRQ off part: Jocke --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/drivers/net/ethernet/freescale/ucc_geth.c +++ b/drivers/net/ethernet/freescale/ucc_geth.c @@ -3200,8 +3200,6 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev) /* Start from the next BD that should be filled */ bd = ugeth->txBd[txQ]; bd_status = in_be32((u32 __iomem *)bd); - /* Save the skb pointer so we can free it later */ - ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] = skb; /* Update the current skb pointer (wrapping if this was the last) */ ugeth->skb_curtx[txQ] = @@ -3209,6 +3207,8 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev) 1) & TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]); spin_lock_irqsave(&ugeth->lock, flags); + /* Save the skb pointer so we can free it later */ + ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] = skb; /* set up the buffer descriptor */ out_be32(&((struct qe_bd __iomem *)bd)->buf, dma_map_single(ugeth->dev, skb->data,