Received: by 2002:a05:6a10:1d13:0:0:0:0 with SMTP id pp19csp2900900pxb; Tue, 24 Aug 2021 10:11:29 -0700 (PDT) X-Google-Smtp-Source: ABdhPJylV4ixJhQg9uxr1Al0RtmeKxNCotacFeYuDHt185NgjvO9PsYNNzW8dPU1dyidXRrcCiwF X-Received: by 2002:a50:e0c3:: with SMTP id j3mr45518016edl.307.1629825088932; Tue, 24 Aug 2021 10:11:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1629825088; cv=none; d=google.com; s=arc-20160816; b=zCVoOvOkN8E24daWzXbl4zjGTlwf6HBuVq5jjtToF6nhrzBgAOJl4qMYSN4b2DaAvi mFUuQ4ealodds3ij17S57VnNQw+c36KLyNk/fLuvWRzUnnduh89OwKNrZemcbzj7LQIw SQB5NaRK7QR0vV8RULfJ5Uitnev9x8TxETo4jO82ck8D2BAt62GDWIIUDLCahlR678ms SbW7U5+bphoIxR2sLwz1Bl3Fim5nCfMbCs1VMK2KUBXcwkXHNSmIpURi0+T4PSHPS3ss WL5vjagCGXyW27Uh26G/hlniYCaiAHFAqRj/vYgV1F2J1B7xwMW+MHaLsMh/EPfVYsoG rELg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=fllrqjH8+ueEEVxd2huFBJlURZ5ZDAmsWLMZE+GmWAM=; b=pZL4vnVMJVq/sn+4aSLXDwQEg/Dhu5GjWLw0663VkVJDitSUuBnaBdnOLa9eX4fn45 TtyUCCBekirqqsCQ3fKJ3ukttDkcUk66e4zy+cEerZ0Y35rwutnTGzVw+DxQXxC1gLvS +aGqWYq66pqiaTFgHn72Y6ZYUo9sd02OK4lXDHcWMJJ4mwP6WJo4qhiRMSxU/Ua2X/7H 1xlCvKNSiX4MDZ0H9HMX+JGEyzxfx7Vu7THie2qOnmBriVlHeg9OWKFJWXI9DBgx8PZ5 nnVXm/sakEaWHdH9BzaNO+JDtHFavmMl2oeS+RGOYb4lP7U7SQ2j5RGdnER9DASK12jF i+vg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=jnVdcanX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id d13si18497043ejz.741.2021.08.24.10.11.03; Tue, 24 Aug 2021 10:11:28 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=jnVdcanX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239765AbhHXRJV (ORCPT + 99 others); Tue, 24 Aug 2021 13:09:21 -0400 Received: from mail.kernel.org ([198.145.29.99]:46494 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239443AbhHXRHD (ORCPT ); Tue, 24 Aug 2021 13:07:03 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id C7D3261989; Tue, 24 Aug 2021 16:59:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1629824398; bh=i1IRNI5ohgov/d7464i6KzuA3LGXp2ckoLzCQ8kXESo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jnVdcanXjrS26F/zQOZ93eSSvrt4xdN26b9IyVtL/OqcuYrwud7vTe96FyA21ryIG QY8XZgpS3+gmCHHnbgCgyCnP9bmQKYTAA5wWiaF4EjZxCh85o8LlyT1w6WDasNVM5b xYijeHHVH58BGJO47w5O3rEvouaoB7lFopRitnsMIOKcMecpAHY1OneyiJRHJo2+hv oVPatcJJjoYk6KBgXJLgaOF9E7CzddRnjFdJapYJrPjI7vpYysVv+loZEw++Rr4d1k dhbAkKzeJjixZTxXePRhDr+lMtRjVXBUfTjOPzFZVLv1AwIssvAWcN9Mny2YuH4eUr 9aWM+AFqipx7Q== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Jakub Kicinski , Michael Chan , Edwin Peer , Sasha Levin Subject: [PATCH 5.10 49/98] bnxt: don't lock the tx queue from napi poll Date: Tue, 24 Aug 2021 12:58:19 -0400 Message-Id: <20210824165908.709932-50-sashal@kernel.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210824165908.709932-1-sashal@kernel.org> References: <20210824165908.709932-1-sashal@kernel.org> MIME-Version: 1.0 X-KernelTest-Patch: http://kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.10.61-rc1.gz X-KernelTest-Tree: git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git X-KernelTest-Branch: linux-5.10.y X-KernelTest-Patches: git://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git X-KernelTest-Version: 5.10.61-rc1 X-KernelTest-Deadline: 2021-08-26T16:58+00:00 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Jakub Kicinski [ Upstream commit 3c603136c9f82833813af77185618de5af67676c ] We can't take the tx lock from the napi poll routine, because netpoll can poll napi at any moment, including with the tx lock already held. The tx lock is protecting against two paths - the disable path, and (as Michael points out) the NETDEV_TX_BUSY case which may occur if NAPI completions race with start_xmit and both decide to re-enable the queue. For the disable/ifdown path use synchronize_net() to make sure closing the device does not race we restarting the queues. Annotate accesses to dev_state against data races. For the NAPI cleanup vs start_xmit path - appropriate barriers are already in place in the main spot where Tx queue is stopped but we need to do the same careful dance in the TX_BUSY case. Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.") Reviewed-by: Michael Chan Reviewed-by: Edwin Peer Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 54 ++++++++++++++--------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 8f169508a90a..65e41612b9da 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -360,6 +360,26 @@ static u16 bnxt_xmit_get_cfa_action(struct sk_buff *skb) return md_dst->u.port_info.port_id; } +static bool bnxt_txr_netif_try_stop_queue(struct bnxt *bp, + struct bnxt_tx_ring_info *txr, + struct netdev_queue *txq) +{ + netif_tx_stop_queue(txq); + + /* netif_tx_stop_queue() must be done before checking + * tx index in bnxt_tx_avail() below, because in + * bnxt_tx_int(), we update tx index before checking for + * netif_tx_queue_stopped(). + */ + smp_mb(); + if (bnxt_tx_avail(bp, txr) > bp->tx_wake_thresh) { + netif_tx_wake_queue(txq); + return false; + } + + return true; +} + static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) { struct bnxt *bp = netdev_priv(dev); @@ -387,8 +407,8 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) free_size = bnxt_tx_avail(bp, txr); if (unlikely(free_size < skb_shinfo(skb)->nr_frags + 2)) { - netif_tx_stop_queue(txq); - return NETDEV_TX_BUSY; + if (bnxt_txr_netif_try_stop_queue(bp, txr, txq)) + return NETDEV_TX_BUSY; } length = skb->len; @@ -597,16 +617,7 @@ tx_done: if (netdev_xmit_more() && !tx_buf->is_push) bnxt_db_write(bp, &txr->tx_db, prod); - netif_tx_stop_queue(txq); - - /* netif_tx_stop_queue() must be done before checking - * tx index in bnxt_tx_avail() below, because in - * bnxt_tx_int(), we update tx index before checking for - * netif_tx_queue_stopped(). - */ - smp_mb(); - if (bnxt_tx_avail(bp, txr) > bp->tx_wake_thresh) - netif_tx_wake_queue(txq); + bnxt_txr_netif_try_stop_queue(bp, txr, txq); } return NETDEV_TX_OK; @@ -690,14 +701,9 @@ next_tx_int: smp_mb(); if (unlikely(netif_tx_queue_stopped(txq)) && - (bnxt_tx_avail(bp, txr) > bp->tx_wake_thresh)) { - __netif_tx_lock(txq, smp_processor_id()); - if (netif_tx_queue_stopped(txq) && - bnxt_tx_avail(bp, txr) > bp->tx_wake_thresh && - txr->dev_state != BNXT_DEV_STATE_CLOSING) - netif_tx_wake_queue(txq); - __netif_tx_unlock(txq); - } + bnxt_tx_avail(bp, txr) > bp->tx_wake_thresh && + READ_ONCE(txr->dev_state) != BNXT_DEV_STATE_CLOSING) + netif_tx_wake_queue(txq); } static struct page *__bnxt_alloc_rx_page(struct bnxt *bp, dma_addr_t *mapping, @@ -8885,9 +8891,11 @@ void bnxt_tx_disable(struct bnxt *bp) if (bp->tx_ring) { for (i = 0; i < bp->tx_nr_rings; i++) { txr = &bp->tx_ring[i]; - txr->dev_state = BNXT_DEV_STATE_CLOSING; + WRITE_ONCE(txr->dev_state, BNXT_DEV_STATE_CLOSING); } } + /* Make sure napi polls see @dev_state change */ + synchronize_net(); /* Drop carrier first to prevent TX timeout */ netif_carrier_off(bp->dev); /* Stop all TX queues */ @@ -8901,8 +8909,10 @@ void bnxt_tx_enable(struct bnxt *bp) for (i = 0; i < bp->tx_nr_rings; i++) { txr = &bp->tx_ring[i]; - txr->dev_state = 0; + WRITE_ONCE(txr->dev_state, 0); } + /* Make sure napi polls see @dev_state change */ + synchronize_net(); netif_tx_wake_all_queues(bp->dev); if (bp->link_info.link_up) netif_carrier_on(bp->dev); -- 2.30.2