From patchwork Fri Sep 5 01:30:46 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Poirier X-Patchwork-Id: 386069 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 466881400A0 for ; Fri, 5 Sep 2014 11:32:06 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755713AbaIEBbe (ORCPT ); Thu, 4 Sep 2014 21:31:34 -0400 Received: from mail-pd0-f180.google.com ([209.85.192.180]:64172 "EHLO mail-pd0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755293AbaIEBbc (ORCPT ); Thu, 4 Sep 2014 21:31:32 -0400 Received: by mail-pd0-f180.google.com with SMTP id p10so14835643pdj.11 for ; Thu, 04 Sep 2014 18:31:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=ivgJ9O65HV+rJS81shS1jg9SMxG68Qtw3uswXO3AeVU=; b=rPWEgFsCfICRg6BIVxnCPuj6C1waYRFbJl746KKkCC4amJgANNDTv4YidfMwUnDGWO 5aBv6hDlZOMFwRJZ3le2h9twwyC2JfM3Cw3o59NPERiYwYD9WfnVs9bTAfnk5PoHYlOL Sm9JkkmTVrkDrBv3d/K0KjoL4dFrm5xh97fmgVaqgkNoEHK4om0O3ogsU055NitZrtLV g5ducLBtvvA021SiglZfoc6vNhgmUolJzPBpopEasp4hlBfDXxXYCoM9LBlVXupNp1WE cscpm6/Pko5CaKF39AfUj7j6j3g56mr/5nlwasZhjyYUuHTtAXDZmO25fQV25gjWsT2a lHEw== X-Received: by 10.66.176.97 with SMTP id ch1mr10045269pac.101.1409880691677; Thu, 04 Sep 2014 18:31:31 -0700 (PDT) Received: from f1.synalogic.ca ([108.203.77.233]) by mx.google.com with ESMTPSA id q1sm336548pdq.67.2014.09.04.18.31.29 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Sep 2014 18:31:31 -0700 (PDT) From: Benjamin Poirier To: Prashant Sreedharan , Michael Chan Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH net v6 3/4] tg3: Move tx queue stop logic to its own function Date: Thu, 4 Sep 2014 18:30:46 -0700 Message-Id: <1409880647-14887-4-git-send-email-bpoirier@suse.de> X-Mailer: git-send-email 1.8.4.5 In-Reply-To: <1409880647-14887-1-git-send-email-bpoirier@suse.de> References: <1409880647-14887-1-git-send-email-bpoirier@suse.de> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org It is duplicated. Also, the first instance in tg3_start_xmit() is racy. Consider: tg3_start_xmit() if budget <= ... tg3_tx() (free up the entire ring) tx_cons = smp_mb if queue_stopped and tx_avail, NO if !queue_stopped stop queue return NETDEV_TX_BUSY ... tx queue stopped forever Signed-off-by: Benjamin Poirier --- Changes v2->v3 * new patch to avoid repeatedly open coding this block in the next patch. Changes v3->v4 * added a comment to clarify the return value, as suggested * replaced the BUG_ON with netdev_err(). No need to be so dramatic, this situation will trigger a netdev watchdog anyways. --- drivers/net/ethernet/broadcom/tg3.c | 75 ++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index c5061c3..6e6b07c 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -7831,6 +7831,35 @@ static int tigon3_dma_hwbug_workaround(struct tg3_napi *tnapi, static netdev_tx_t tg3_start_xmit(struct sk_buff *, struct net_device *); +/* Returns true if the queue has been stopped. Note that it may have been + * restarted since. + */ +static inline bool tg3_maybe_stop_txq(struct tg3_napi *tnapi, + struct netdev_queue *txq, + u32 stop_thresh, u32 wakeup_thresh) +{ + bool stopped = false; + + if (unlikely(tg3_tx_avail(tnapi) <= stop_thresh)) { + if (!netif_tx_queue_stopped(txq)) { + stopped = true; + netif_tx_stop_queue(txq); + if (wakeup_thresh >= tnapi->tx_pending) + netdev_err(tnapi->tp->dev, + "BUG! wakeup_thresh too large (%u >= %u)\n", + wakeup_thresh, tnapi->tx_pending); + } + /* netif_tx_stop_queue() must be done before checking tx index + * in tg3_tx_avail(), because in tg3_tx(), we update tx index + * before checking for netif_tx_queue_stopped(). + */ + smp_mb(); + if (tg3_tx_avail(tnapi) > wakeup_thresh) + netif_tx_wake_queue(txq); + } + return stopped; +} + /* Use GSO to workaround all TSO packets that meet HW bug conditions * indicated in tg3_tx_frag_set() */ @@ -7841,20 +7870,9 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi, u32 frag_cnt_est = skb_shinfo(skb)->gso_segs * 3; /* Estimate the number of fragments in the worst case */ - if (unlikely(tg3_tx_avail(tnapi) <= frag_cnt_est)) { - netif_tx_stop_queue(txq); - - /* netif_tx_stop_queue() must be done before checking - * checking tx index in tg3_tx_avail() below, because in - * tg3_tx(), we update tx index before checking for - * netif_tx_queue_stopped(). - */ - smp_mb(); - if (tg3_tx_avail(tnapi) <= frag_cnt_est) - return NETDEV_TX_BUSY; - - netif_tx_wake_queue(txq); - } + tg3_maybe_stop_txq(tnapi, txq, frag_cnt_est, frag_cnt_est); + if (netif_tx_queue_stopped(txq)) + return NETDEV_TX_BUSY; segs = skb_gso_segment(skb, tp->dev->features & ~(NETIF_F_TSO | NETIF_F_TSO6)); @@ -7902,16 +7920,13 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) * interrupt. Furthermore, IRQ processing runs lockless so we have * no IRQ context deadlocks to worry about either. Rejoice! */ - if (unlikely(budget <= (skb_shinfo(skb)->nr_frags + 1))) { - if (!netif_tx_queue_stopped(txq)) { - netif_tx_stop_queue(txq); - - /* This is a hard error, log it. */ - netdev_err(dev, - "BUG! Tx Ring full when queue awake!\n"); - } - return NETDEV_TX_BUSY; + if (tg3_maybe_stop_txq(tnapi, txq, skb_shinfo(skb)->nr_frags + 1, + TG3_TX_WAKEUP_THRESH(tnapi))) { + /* This is a hard error, log it. */ + netdev_err(dev, "BUG! Tx Ring full when queue awake!\n"); } + if (netif_tx_queue_stopped(txq)) + return NETDEV_TX_BUSY; entry = tnapi->tx_prod; base_flags = 0; @@ -8087,18 +8102,8 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) tw32_tx_mbox(tnapi->prodmbox, entry); tnapi->tx_prod = entry; - if (unlikely(tg3_tx_avail(tnapi) <= (MAX_SKB_FRAGS + 1))) { - netif_tx_stop_queue(txq); - - /* netif_tx_stop_queue() must be done before checking - * checking tx index in tg3_tx_avail() below, because in - * tg3_tx(), we update tx index before checking for - * netif_tx_queue_stopped(). - */ - smp_mb(); - if (tg3_tx_avail(tnapi) > TG3_TX_WAKEUP_THRESH(tnapi)) - netif_tx_wake_queue(txq); - } + tg3_maybe_stop_txq(tnapi, txq, MAX_SKB_FRAGS + 1, + TG3_TX_WAKEUP_THRESH(tnapi)); mmiowb(); return NETDEV_TX_OK;