Received: by 2002:a05:6520:4d:b0:139:a872:a4c9 with SMTP id i13csp2566422lkm; Mon, 20 Sep 2021 18:53:19 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxKNwjeaSLGj+nNhdcNayMU6lODLBmclLY7QQ3+v5tz4L8gYJOEXTWbdquWQ/3vxfoWfUIo X-Received: by 2002:a92:cecf:: with SMTP id z15mr19634275ilq.96.1632189099461; Mon, 20 Sep 2021 18:51:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1632189099; cv=none; d=google.com; s=arc-20160816; b=H2AHLMPLU613o0254vN8Euh6cKl2iXd9o+vzJ1r9jYnA8eXkAVvrhMZgjbmNA1dN60 PAzBETg+2G+5QuQzxYzbLbie+0RBFb0MbyK0z7l5F7hXhTrnKC3wL5mZxPLENRjMFmhk 2YBDAqIpXgkwlIcGWRHbC5GofUxRyJ7JGwkvuZwc97EtTBB/DDPy30djZikgquabRny8 rUQVKzU8lbp2xweafMJYM6hZpxTcXguTfplIXbHPcD4F8cO3gV4k4QpmKqGLIrbipCCu hfqYgiH58x/yDSYVJp1xEFTfriI8HOPiW5kpJ3oYIrbgUv5pBKNPn65CYi3+gSFkMvGf FCmA== 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 :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=7QYtN3ik5bHT1PiIYDoLZKtMu1eCS2kdWk82Lrn1Ht4=; b=C/viqsV7AKl8+Ehn2j3MEcl8NpRD/NbowZdGJx+Po7PP6u8/OoPWtFQeCJxt01xiXR /r1/WyhyWGuqFEDNJTp2gvCy8texq1DqZhv7uC58QIJigDz32zWcXHQyUq1POGqpMFL8 jrai/WJvhxSVhNcPLEKEf2WEJfsPDaSOLzmB3+WDr85ng+Dq3omS9AhzSaInyxfCnSLC FpI6pitdAfqXJUtLoagfHePDvd5nhSgKaZLHAMVjwyol3N5Qf681pcjx2sLlKgRQgFa3 5PysUdU1uEU9swSIal0Oh5e0/fOHSS7Gv+cVHXzxNJcftjLRBlNRFIkEnLhcbLyl6C7r DGbg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=LLsezGMk; 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=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id d16si5000128ile.92.2021.09.20.18.51.28; Mon, 20 Sep 2021 18:51:39 -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=@linuxfoundation.org header.s=korg header.b=LLsezGMk; 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=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1376449AbhITSMU (ORCPT + 99 others); Mon, 20 Sep 2021 14:12:20 -0400 Received: from mail.kernel.org ([198.145.29.99]:33252 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1358305AbhITSGN (ORCPT ); Mon, 20 Sep 2021 14:06:13 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 54C5A6324C; Mon, 20 Sep 2021 17:17:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1632158275; bh=/DGxrYYuOKelqppobJQ/urlkm51EvYrMJz4/Fs1C7WU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LLsezGMk8u4OyXL+BFs5kaiBcuAiK1qYr5NBEQsuHQnwT7Fkf8xnQ6BXRVNyptPRP 4JlcMxebdaxj9r7n/LWhb3IJ+Xx7Ybq0FTzxd+t9EAmLow7ktH8Gw0ZHhUN8XGd8HM 64Rye/EO0l7pgy7/3o1lV72ro/JNwzxWKKabMREY= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Stefan Assmann , Konrad Jankowski , Tony Nguyen , Sasha Levin Subject: [PATCH 5.4 080/260] iavf: fix locking of critical sections Date: Mon, 20 Sep 2021 18:41:38 +0200 Message-Id: <20210920163933.856382307@linuxfoundation.org> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210920163931.123590023@linuxfoundation.org> References: <20210920163931.123590023@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Stefan Assmann [ Upstream commit 226d528512cfac890a1619aea4301f3dd314fe60 ] To avoid races between iavf_init_task(), iavf_reset_task(), iavf_watchdog_task(), iavf_adminq_task() as well as the shutdown and remove functions more locking is required. The current protection by __IAVF_IN_CRITICAL_TASK is needed in additional places. - The reset task performs state transitions, therefore needs locking. - The adminq task acts on replies from the PF in iavf_virtchnl_completion() which may alter the states. - The init task is not only run during probe but also if a VF gets stuck to reinitialize it. - The shutdown function performs a state transition. - The remove function performs a state transition and also free's resources. iavf_lock_timeout() is introduced to avoid waiting infinitely and cause a deadlock. Rather unlock and print a warning. Signed-off-by: Stefan Assmann Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen Signed-off-by: Sasha Levin --- drivers/net/ethernet/intel/iavf/iavf_main.c | 57 ++++++++++++++++++--- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 3e76111af872..bc46c262b42d 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -142,6 +142,30 @@ enum iavf_status iavf_free_virt_mem_d(struct iavf_hw *hw, return 0; } +/** + * iavf_lock_timeout - try to set bit but give up after timeout + * @adapter: board private structure + * @bit: bit to set + * @msecs: timeout in msecs + * + * Returns 0 on success, negative on failure + **/ +static int iavf_lock_timeout(struct iavf_adapter *adapter, + enum iavf_critical_section_t bit, + unsigned int msecs) +{ + unsigned int wait, delay = 10; + + for (wait = 0; wait < msecs; wait += delay) { + if (!test_and_set_bit(bit, &adapter->crit_section)) + return 0; + + msleep(delay); + } + + return -1; +} + /** * iavf_schedule_reset - Set the flags and schedule a reset event * @adapter: board private structure @@ -2076,6 +2100,10 @@ static void iavf_reset_task(struct work_struct *work) if (test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) return; + if (iavf_lock_timeout(adapter, __IAVF_IN_CRITICAL_TASK, 200)) { + schedule_work(&adapter->reset_task); + return; + } while (test_and_set_bit(__IAVF_IN_CLIENT_TASK, &adapter->crit_section)) usleep_range(500, 1000); @@ -2290,6 +2318,8 @@ static void iavf_adminq_task(struct work_struct *work) if (!event.msg_buf) goto out; + if (iavf_lock_timeout(adapter, __IAVF_IN_CRITICAL_TASK, 200)) + goto freedom; do { ret = iavf_clean_arq_element(hw, &event, &pending); v_op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high); @@ -2303,6 +2333,7 @@ static void iavf_adminq_task(struct work_struct *work) if (pending != 0) memset(event.msg_buf, 0, IAVF_MAX_AQ_BUF_SIZE); } while (pending); + clear_bit(__IAVF_IN_CRITICAL_TASK, &adapter->crit_section); if ((adapter->flags & (IAVF_FLAG_RESET_PENDING | IAVF_FLAG_RESET_NEEDED)) || @@ -3599,6 +3630,10 @@ static void iavf_init_task(struct work_struct *work) init_task.work); struct iavf_hw *hw = &adapter->hw; + if (iavf_lock_timeout(adapter, __IAVF_IN_CRITICAL_TASK, 5000)) { + dev_warn(&adapter->pdev->dev, "failed to set __IAVF_IN_CRITICAL_TASK in %s\n", __FUNCTION__); + return; + } switch (adapter->state) { case __IAVF_STARTUP: if (iavf_startup(adapter) < 0) @@ -3611,14 +3646,14 @@ static void iavf_init_task(struct work_struct *work) case __IAVF_INIT_GET_RESOURCES: if (iavf_init_get_resources(adapter) < 0) goto init_failed; - return; + goto out; default: goto init_failed; } queue_delayed_work(iavf_wq, &adapter->init_task, msecs_to_jiffies(30)); - return; + goto out; init_failed: if (++adapter->aq_wait_count > IAVF_AQ_MAX_ERR) { dev_err(&adapter->pdev->dev, @@ -3627,9 +3662,11 @@ static void iavf_init_task(struct work_struct *work) iavf_shutdown_adminq(hw); adapter->state = __IAVF_STARTUP; queue_delayed_work(iavf_wq, &adapter->init_task, HZ * 5); - return; + goto out; } queue_delayed_work(iavf_wq, &adapter->init_task, HZ); +out: + clear_bit(__IAVF_IN_CRITICAL_TASK, &adapter->crit_section); } /** @@ -3646,9 +3683,12 @@ static void iavf_shutdown(struct pci_dev *pdev) if (netif_running(netdev)) iavf_close(netdev); + if (iavf_lock_timeout(adapter, __IAVF_IN_CRITICAL_TASK, 5000)) + dev_warn(&adapter->pdev->dev, "failed to set __IAVF_IN_CRITICAL_TASK in %s\n", __FUNCTION__); /* Prevent the watchdog from running. */ adapter->state = __IAVF_REMOVE; adapter->aq_required = 0; + clear_bit(__IAVF_IN_CRITICAL_TASK, &adapter->crit_section); #ifdef CONFIG_PM pci_save_state(pdev); @@ -3877,10 +3917,6 @@ static void iavf_remove(struct pci_dev *pdev) err); } - /* Shut down all the garbage mashers on the detention level */ - adapter->state = __IAVF_REMOVE; - adapter->aq_required = 0; - adapter->flags &= ~IAVF_FLAG_REINIT_ITR_NEEDED; iavf_request_reset(adapter); msleep(50); /* If the FW isn't responding, kick it once, but only once. */ @@ -3888,6 +3924,13 @@ static void iavf_remove(struct pci_dev *pdev) iavf_request_reset(adapter); msleep(50); } + if (iavf_lock_timeout(adapter, __IAVF_IN_CRITICAL_TASK, 5000)) + dev_warn(&adapter->pdev->dev, "failed to set __IAVF_IN_CRITICAL_TASK in %s\n", __FUNCTION__); + + /* Shut down all the garbage mashers on the detention level */ + adapter->state = __IAVF_REMOVE; + adapter->aq_required = 0; + adapter->flags &= ~IAVF_FLAG_REINIT_ITR_NEEDED; iavf_free_all_tx_resources(adapter); iavf_free_all_rx_resources(adapter); iavf_misc_irq_disable(adapter); -- 2.30.2