iavf: fix locking of critical sections

[ 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 <sassmann@kpanic.de>
Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index da401d5..f06c079 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -132,6 +132,30 @@ enum iavf_status iavf_free_virt_mem_d(struct iavf_hw *hw,
 }
 
 /**
+ * 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
  **/
@@ -2064,6 +2088,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);
@@ -2278,6 +2306,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);
@@ -2291,6 +2321,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)) ||
@@ -3593,6 +3624,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)
@@ -3605,14 +3640,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,
@@ -3621,9 +3656,11 @@ init_failed:
                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);
 }
 
 /**
@@ -3640,9 +3677,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);
@@ -3870,10 +3910,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. */
@@ -3881,6 +3917,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);