From patchwork Wed Nov 6 18:54:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Salil Mehta X-Patchwork-Id: 178753 Delivered-To: patch@linaro.org Received: by 2002:a92:38d5:0:0:0:0:0 with SMTP id g82csp1024059ilf; Wed, 6 Nov 2019 10:56:27 -0800 (PST) X-Google-Smtp-Source: APXvYqwaCi4KoLKDrgA6NEPz4hJpc8lvY4/VdG0gTRjpVYWrnK/Ji5PrQh45W1MGv/VCw0cqHlFD X-Received: by 2002:a50:b536:: with SMTP id y51mr4448241edd.271.1573066587564; Wed, 06 Nov 2019 10:56:27 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1573066587; cv=none; d=google.com; s=arc-20160816; b=xumkIpEs/c5abDCAoDh3eX7b9ENEgSUNWuRT40yCCnndrONT4DsLy4mZOT8yDSviqd Zjln7AeG87ZjA+ygpaHgQfoUJNkYe0quJAjQFVSbYzf5tByWN1OJyBx3LCw98/xAwZwj 0yo48gLGzdCUpSq4HAPUF2stHp2DOJWoNIosZUAHwlNWSbdxT9/56KQqI7iizbLxzVgr 3k1vVsJNG16+z4kHalBAtbYZcXnvSTzYj6jBmVyINRpbdWcoBj0vt5BjrRqFOlUJOXSX XGfDC9E60oVuBCgxGXIc3z6K/+QUMwlFa8KmrtSwJzUtm86Hah6XQDDqFIxOdLhh2uXL NqaA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:message-id:date:subject:cc :to:from; bh=0KpVd3mlfE3+EGGcGvOGI7OLyAd9eel8HxekGpxAnNc=; b=Lfhd8E4S9N4/lbTDQNPjAH34Ob/p0fijPvfk34fky5LyksYthNxwUd82DV6dlyw0JE Hw1/Rh9B6BeOsS/v+/Lh6pA1KBg1mUG6jZsF2xibkGQP2GC3Fac4fbqjTD0RBNKXqJM9 l8O4X+B2gur82+FEkoFFalIklD7Xsm3Oeb+NKOncZpkoPcUddnV09gm5NuZr2Wh1oEbH iKSw1j+EY58UHhdMB81z3YjGPeoCnV8mQRiu59skEulctxMSAtYrdAft0F8mklC0llnI jxHXB9EOy72x8bNYW5cYdaNoTwmajt7BhcOappa8dXB02of0DbNcCLU21rSIPCR3rZ+w Ww8g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w8si5572316ejq.57.2019.11.06.10.56.27; Wed, 06 Nov 2019 10:56:27 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731897AbfKFS40 (ORCPT + 26 others); Wed, 6 Nov 2019 13:56:26 -0500 Received: from szxga05-in.huawei.com ([45.249.212.191]:5739 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727208AbfKFS4Z (ORCPT ); Wed, 6 Nov 2019 13:56:25 -0500 Received: from DGGEMS408-HUB.china.huawei.com (unknown [172.30.72.59]) by Forcepoint Email with ESMTP id B9BAA58BFBDC6172BC23; Thu, 7 Nov 2019 02:56:23 +0800 (CST) Received: from A190218597.china.huawei.com (10.202.226.45) by DGGEMS408-HUB.china.huawei.com (10.3.19.208) with Microsoft SMTP Server id 14.3.439.0; Thu, 7 Nov 2019 02:56:14 +0800 From: Salil Mehta To: , CC: , , , , , , , Subject: [PATCH net] net: hns: Fix the stray netpoll locks causing deadlock in NAPI path Date: Wed, 6 Nov 2019 18:54:05 +0000 Message-ID: <20191106185405.23112-1-salil.mehta@huawei.com> X-Mailer: git-send-email 2.8.3 MIME-Version: 1.0 X-Originating-IP: [10.202.226.45] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This patch fixes the problem of the spin locks, originally meant for the netpoll path of hns driver, causing deadlock in the normal NAPI poll path. The issue happened due presence of the stray leftover spin lock code related to the netpoll, whose support was earlier removed from the HNS[1], got activated due to enabling of NET_POLL_CONTROLLER switch. Earlier background: The netpoll handling code originally had this bug(as identified by Marc Zyngier[2]) of wrong spin lock API being used which did not disable the interrupts and hence could cause locking issues. i.e. if the lock were first acquired in context to thread like 'ip' util and this lock if ever got later acquired again in context to the interrupt context like TX/RX (Interrupts could always pre-empt the lock holding task and acquire the lock again) and hence could cause deadlock. Proposed Solution: 1. If the netpoll was enabled in the HNS driver, which is not right now, we could have simply used spin_[un]lock_irqsave() 2. But as netpoll is disabled, therefore, it is best to get rid of the existing locks and stray code for now. This should solve the problem reported by Marc. @Marc, Could you please test this patch and confirm if the problem is fixed at your end? Many Thanks [1] https://git.kernel.org/torvalds/c/4bd2c03be7 [2] https://patchwork.ozlabs.org/patch/1189139/ Fixes: 4bd2c03be707 ("net: hns: remove ndo_poll_controller") Cc: lipeng Cc: Yisen Zhuang Cc: Eric Dumazet Cc: David S. Miller Reported-by: Marc Zyngier Signed-off-by: Salil Mehta --- drivers/net/ethernet/hisilicon/hns/hns_enet.c | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) -- 2.17.1 Acked-by: Marc Zyngier Tested-by: Marc Zyngier diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c index a48396dd4ebb..14ab20491fd0 100644 --- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c +++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c @@ -943,15 +943,6 @@ static int is_valid_clean_head(struct hnae_ring *ring, int h) return u > c ? (h > c && h <= u) : (h > c || h <= u); } -/* netif_tx_lock will turn down the performance, set only when necessary */ -#ifdef CONFIG_NET_POLL_CONTROLLER -#define NETIF_TX_LOCK(ring) spin_lock(&(ring)->lock) -#define NETIF_TX_UNLOCK(ring) spin_unlock(&(ring)->lock) -#else -#define NETIF_TX_LOCK(ring) -#define NETIF_TX_UNLOCK(ring) -#endif - /* reclaim all desc in one budget * return error or number of desc left */ @@ -965,21 +956,16 @@ static int hns_nic_tx_poll_one(struct hns_nic_ring_data *ring_data, int head; int bytes, pkts; - NETIF_TX_LOCK(ring); - head = readl_relaxed(ring->io_base + RCB_REG_HEAD); rmb(); /* make sure head is ready before touch any data */ - if (is_ring_empty(ring) || head == ring->next_to_clean) { - NETIF_TX_UNLOCK(ring); + if (is_ring_empty(ring) || head == ring->next_to_clean) return 0; /* no data to poll */ - } if (!is_valid_clean_head(ring, head)) { netdev_err(ndev, "wrong head (%d, %d-%d)\n", head, ring->next_to_use, ring->next_to_clean); ring->stats.io_err_cnt++; - NETIF_TX_UNLOCK(ring); return -EIO; } @@ -994,8 +980,6 @@ static int hns_nic_tx_poll_one(struct hns_nic_ring_data *ring_data, ring->stats.tx_pkts += pkts; ring->stats.tx_bytes += bytes; - NETIF_TX_UNLOCK(ring); - dev_queue = netdev_get_tx_queue(ndev, ring_data->queue_index); netdev_tx_completed_queue(dev_queue, pkts, bytes); @@ -1055,16 +1039,12 @@ static void hns_nic_tx_clr_all_bufs(struct hns_nic_ring_data *ring_data) int head; int bytes, pkts; - NETIF_TX_LOCK(ring); - head = ring->next_to_use; /* ntu :soft setted ring position*/ bytes = 0; pkts = 0; while (head != ring->next_to_clean) hns_nic_reclaim_one_desc(ring, &bytes, &pkts); - NETIF_TX_UNLOCK(ring); - dev_queue = netdev_get_tx_queue(ndev, ring_data->queue_index); netdev_tx_reset_queue(dev_queue); }