Skip to content

Commit

Permalink
iavf: fix reset task race with iavf_remove()
Browse files Browse the repository at this point in the history
The reset task is currently scheduled from the watchdog or adminq tasks.
First, all direct calls to schedule the reset task are replaced with the
iavf_schedule_reset(), which is modified to accept the flag showing the
type of reset.

To prevent the reset task from starting once iavf_remove() starts, we need
to check the __IAVF_IN_REMOVE_TASK bit before we schedule it. This is now
easily added to iavf_schedule_reset().

Finally, remove the check for IAVF_FLAG_RESET_NEEDED in the watchdog task.
It is redundant since all callers who set the flag immediately schedules
the reset task.

Fixes: 3ccd54e ("iavf: Fix init state closure on remove")
Fixes: 14756b2 ("iavf: Fix __IAVF_RESETTING state usage")
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
  • Loading branch information
azaki1 authored and anguy11 committed Jul 17, 2023
1 parent d1639a1 commit c34743d
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 29 deletions.
2 changes: 1 addition & 1 deletion drivers/net/ethernet/intel/iavf/iavf.h
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ int iavf_up(struct iavf_adapter *adapter);
void iavf_down(struct iavf_adapter *adapter);
int iavf_process_config(struct iavf_adapter *adapter);
int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter);
void iavf_schedule_reset(struct iavf_adapter *adapter);
void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags);
void iavf_schedule_request_stats(struct iavf_adapter *adapter);
void iavf_schedule_finish_config(struct iavf_adapter *adapter);
void iavf_reset(struct iavf_adapter *adapter);
Expand Down
8 changes: 3 additions & 5 deletions drivers/net/ethernet/intel/iavf/iavf_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,7 @@ static int iavf_set_priv_flags(struct net_device *netdev, u32 flags)
/* issue a reset to force legacy-rx change to take effect */
if (changed_flags & IAVF_FLAG_LEGACY_RX) {
if (netif_running(netdev)) {
adapter->flags |= IAVF_FLAG_RESET_NEEDED;
queue_work(adapter->wq, &adapter->reset_task);
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);
ret = iavf_wait_for_reset(adapter);
if (ret)
netdev_warn(netdev, "Changing private flags timeout or interrupted waiting for reset");
Expand Down Expand Up @@ -676,8 +675,7 @@ static int iavf_set_ringparam(struct net_device *netdev,
}

if (netif_running(netdev)) {
adapter->flags |= IAVF_FLAG_RESET_NEEDED;
queue_work(adapter->wq, &adapter->reset_task);
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);
ret = iavf_wait_for_reset(adapter);
if (ret)
netdev_warn(netdev, "Changing ring parameters timeout or interrupted waiting for reset");
Expand Down Expand Up @@ -1860,7 +1858,7 @@ static int iavf_set_channels(struct net_device *netdev,

adapter->num_req_queues = num_req;
adapter->flags |= IAVF_FLAG_REINIT_ITR_NEEDED;
iavf_schedule_reset(adapter);
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);

ret = iavf_wait_for_reset(adapter);
if (ret)
Expand Down
32 changes: 11 additions & 21 deletions drivers/net/ethernet/intel/iavf/iavf_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,14 @@ static int iavf_lock_timeout(struct mutex *lock, unsigned int msecs)
/**
* iavf_schedule_reset - Set the flags and schedule a reset event
* @adapter: board private structure
* @flags: IAVF_FLAG_RESET_PENDING or IAVF_FLAG_RESET_NEEDED
**/
void iavf_schedule_reset(struct iavf_adapter *adapter)
void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags)
{
if (!(adapter->flags &
(IAVF_FLAG_RESET_PENDING | IAVF_FLAG_RESET_NEEDED))) {
adapter->flags |= IAVF_FLAG_RESET_NEEDED;
if (!test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section) &&
!(adapter->flags &
(IAVF_FLAG_RESET_PENDING | IAVF_FLAG_RESET_NEEDED))) {
adapter->flags |= flags;
queue_work(adapter->wq, &adapter->reset_task);
}
}
Expand Down Expand Up @@ -334,7 +336,7 @@ static void iavf_tx_timeout(struct net_device *netdev, unsigned int txqueue)
struct iavf_adapter *adapter = netdev_priv(netdev);

adapter->tx_timeout_count++;
iavf_schedule_reset(adapter);
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);
}

/**
Expand Down Expand Up @@ -2478,7 +2480,7 @@ int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter)
adapter->vsi_res->num_queue_pairs);
adapter->flags |= IAVF_FLAG_REINIT_MSIX_NEEDED;
adapter->num_req_queues = adapter->vsi_res->num_queue_pairs;
iavf_schedule_reset(adapter);
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);

return -EAGAIN;
}
Expand Down Expand Up @@ -2775,14 +2777,6 @@ static void iavf_watchdog_task(struct work_struct *work)
if (adapter->flags & IAVF_FLAG_PF_COMMS_FAILED)
iavf_change_state(adapter, __IAVF_COMM_FAILED);

if (adapter->flags & IAVF_FLAG_RESET_NEEDED) {
adapter->aq_required = 0;
adapter->current_op = VIRTCHNL_OP_UNKNOWN;
mutex_unlock(&adapter->crit_lock);
queue_work(adapter->wq, &adapter->reset_task);
return;
}

switch (adapter->state) {
case __IAVF_STARTUP:
iavf_startup(adapter);
Expand Down Expand Up @@ -2910,11 +2904,10 @@ static void iavf_watchdog_task(struct work_struct *work)
/* check for hw reset */
reg_val = rd32(hw, IAVF_VF_ARQLEN1) & IAVF_VF_ARQLEN1_ARQENABLE_MASK;
if (!reg_val) {
adapter->flags |= IAVF_FLAG_RESET_PENDING;
adapter->aq_required = 0;
adapter->current_op = VIRTCHNL_OP_UNKNOWN;
dev_err(&adapter->pdev->dev, "Hardware reset detected\n");
queue_work(adapter->wq, &adapter->reset_task);
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_PENDING);
mutex_unlock(&adapter->crit_lock);
queue_delayed_work(adapter->wq,
&adapter->watchdog_task, HZ * 2);
Expand Down Expand Up @@ -3288,9 +3281,7 @@ static void iavf_adminq_task(struct work_struct *work)
} while (pending);
mutex_unlock(&adapter->crit_lock);

if ((adapter->flags &
(IAVF_FLAG_RESET_PENDING | IAVF_FLAG_RESET_NEEDED)) ||
adapter->state == __IAVF_RESETTING)
if (iavf_is_reset_in_progress(adapter))
goto freedom;

/* check for error indications */
Expand Down Expand Up @@ -4387,8 +4378,7 @@ static int iavf_change_mtu(struct net_device *netdev, int new_mtu)
}

if (netif_running(netdev)) {
adapter->flags |= IAVF_FLAG_RESET_NEEDED;
queue_work(adapter->wq, &adapter->reset_task);
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);
ret = iavf_wait_for_reset(adapter);
if (ret < 0)
netdev_warn(netdev, "MTU change interrupted waiting for reset");
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1961,9 +1961,8 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
case VIRTCHNL_EVENT_RESET_IMPENDING:
dev_info(&adapter->pdev->dev, "Reset indication received from the PF\n");
if (!(adapter->flags & IAVF_FLAG_RESET_PENDING)) {
adapter->flags |= IAVF_FLAG_RESET_PENDING;
dev_info(&adapter->pdev->dev, "Scheduling reset task\n");
queue_work(adapter->wq, &adapter->reset_task);
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_PENDING);
}
break;
default:
Expand Down

0 comments on commit c34743d

Please # to comment.