Commit 226d5285 authored by Stefan Assmann's avatar Stefan Assmann Committed by Tony Nguyen
Browse files

iavf: fix locking of critical sections 



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: default avatarStefan Assmann <sassmann@kpanic.de>
Tested-by: default avatarKonrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent 22c8fd71

drivers/net/ethernet/intel/iavf/iavf_main.c

+50 −7
Original line number Diff line number Diff line
@@ -131,6 +131,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
@@ -2101,6 +2125,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);
@@ -2307,6 +2335,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);
@@ -2320,6 +2350,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)) ||
@@ -3624,6 +3655,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)
@@ -3636,14 +3671,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,
@@ -3652,9 +3687,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);

}



/**
@@ -3671,9 +3708,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);
@@ -3907,10 +3947,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. */
@@ -3918,6 +3954,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);