Skip to content

Commit

Permalink
iavf: Add waiting so the port is initialized in remove
Browse files Browse the repository at this point in the history
There exist races when port is being configured and remove is
triggered.

unregister_netdev is not and can't be called under crit_lock
mutex since it is calling ndo_stop -> iavf_close which requires
this lock. Depending on init state the netdev could be still
unregistered so unregister_netdev never cleans up, when shortly
after that the device could become registered.

Make iavf_remove wait until port finishes initialization.
All critical state changes are atomic (under crit_lock).
Crashes that come from iavf_reset_interrupt_capability and
iavf_free_traffic_irqs should now be solved in a graceful
manner.

Fixes: 605ca7c ("iavf: Fix kernel BUG in free_msi_irqs")
Signed-off-by: Slawomir Laba <[email protected]>
Signed-off-by: Phani Burra <[email protected]>
Signed-off-by: Jacob Keller <[email protected]>
Signed-off-by: Mateusz Palczewski <[email protected]>
Tested-by: Konrad Jankowski <[email protected]>
Signed-off-by: Tony Nguyen <[email protected]>
  • Loading branch information
SlawomirLaba authored and anguy11 committed Feb 25, 2022
1 parent fc2e6b3 commit 9745780
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions drivers/net/ethernet/intel/iavf/iavf_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -4558,7 +4558,6 @@ static int __maybe_unused iavf_resume(struct device *dev_d)
static void iavf_remove(struct pci_dev *pdev)
{
struct iavf_adapter *adapter = iavf_pdev_to_adapter(pdev);
enum iavf_state_t prev_state = adapter->last_state;
struct net_device *netdev = adapter->netdev;
struct iavf_fdir_fltr *fdir, *fdirtmp;
struct iavf_vlan_filter *vlf, *vlftmp;
Expand All @@ -4568,6 +4567,22 @@ static void iavf_remove(struct pci_dev *pdev)
struct iavf_hw *hw = &adapter->hw;
int err;

/* Wait until port initialization is complete.
* There are flows where register/unregister netdev may race.
*/
while (1) {
mutex_lock(&adapter->crit_lock);
if (adapter->state == __IAVF_RUNNING ||
adapter->state == __IAVF_DOWN) {
mutex_unlock(&adapter->crit_lock);
break;
}

mutex_unlock(&adapter->crit_lock);
usleep_range(500, 1000);
}
cancel_delayed_work_sync(&adapter->watchdog_task);

if (adapter->netdev_registered) {
unregister_netdev(netdev);
adapter->netdev_registered = false;
Expand Down Expand Up @@ -4605,16 +4620,6 @@ static void iavf_remove(struct pci_dev *pdev)
iavf_free_all_rx_resources(adapter);
iavf_free_misc_irq(adapter);

/* In case we enter iavf_remove from erroneous state, free traffic irqs
* here, so as to not cause a kernel crash, when calling
* iavf_reset_interrupt_capability.
*/
if ((adapter->last_state == __IAVF_RESETTING &&
prev_state != __IAVF_DOWN) ||
(adapter->last_state == __IAVF_RUNNING &&
!(netdev->flags & IFF_UP)))
iavf_free_traffic_irqs(adapter);

iavf_reset_interrupt_capability(adapter);
iavf_free_q_vectors(adapter);

Expand Down

0 comments on commit 9745780

Please sign in to comment.