Skip to content

Commit

Permalink
Merge branch '40GbE' of git://git.kernel.org/pub/scm/linux/kernel/git…
Browse files Browse the repository at this point in the history 
…/tnguy/net-queue

Tony Nguyen says:

====================
Intel Wired LAN Driver Updates 2022-02-25

This series contains updates to iavf driver only.

Slawomir fixes stability issues that can be seen when stressing the
driver using a large number of VFs with a multitude of operations.
Among the fixes are reworking mutexes to provide more effective locking,
ensuring initialization is complete before teardown, preventing
operations which could race while removing the driver, stopping certain
tasks from being queued when the device is down, and adding a missing
mutex unlock.
====================

Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
@davem330
davem330 committed Feb 26, 2022
2 parents 328e765 + 14756b2 commit 519ca6f
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 75 deletions.
6 changes: 5 additions & 1 deletion drivers/net/ethernet/intel/iavf/iavf.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ enum iavf_state_t {
__IAVF_RUNNING, /* opened, working */
};

enum iavf_critical_section_t {
__IAVF_IN_REMOVE_TASK, /* device being removed */
};

#define IAVF_CLOUD_FIELD_OMAC 0x01
#define IAVF_CLOUD_FIELD_IMAC 0x02
#define IAVF_CLOUD_FIELD_IVLAN 0x04
Expand Down Expand Up @@ -246,7 +250,6 @@ struct iavf_adapter {
struct list_head mac_filter_list;
struct mutex crit_lock;
struct mutex client_lock;
struct mutex remove_lock;
/* Lock to protect accesses to MAC and VLAN lists */
spinlock_t mac_vlan_list_lock;
char misc_vector_name[IFNAMSIZ + 9];
Expand Down Expand Up @@ -284,6 +287,7 @@ struct iavf_adapter {
#define IAVF_FLAG_LEGACY_RX BIT(15)
#define IAVF_FLAG_REINIT_ITR_NEEDED BIT(16)
#define IAVF_FLAG_QUEUES_DISABLED BIT(17)
#define IAVF_FLAG_SETUP_NETDEV_FEATURES BIT(18)
/* duplicates for common code */
#define IAVF_FLAG_DCB_ENABLED 0
/* flags for admin queue service task */
Expand Down
159 changes: 108 additions & 51 deletions drivers/net/ethernet/intel/iavf/iavf_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,9 @@ static irqreturn_t iavf_msix_aq(int irq, void *data)
rd32(hw, IAVF_VFINT_ICR01);
rd32(hw, IAVF_VFINT_ICR0_ENA1);

/* schedule work on the private workqueue */
queue_work(iavf_wq, &adapter->adminq_task);
if (adapter->state != __IAVF_REMOVE)
/* schedule work on the private workqueue */
queue_work(iavf_wq, &adapter->adminq_task);

return IRQ_HANDLED;
}
Expand Down Expand Up @@ -1136,8 +1137,7 @@ void iavf_down(struct iavf_adapter *adapter)
rss->state = IAVF_ADV_RSS_DEL_REQUEST;
spin_unlock_bh(&adapter->adv_rss_lock);

if (!(adapter->flags & IAVF_FLAG_PF_COMMS_FAILED) &&
adapter->state != __IAVF_RESETTING) {
if (!(adapter->flags & IAVF_FLAG_PF_COMMS_FAILED)) {
/* cancel any current operation */
adapter->current_op = VIRTCHNL_OP_UNKNOWN;
/* Schedule operations to close down the HW. Don't wait
Expand Down Expand Up @@ -2374,17 +2374,22 @@ static void iavf_watchdog_task(struct work_struct *work)
struct iavf_hw *hw = &adapter->hw;
u32 reg_val;

if (!mutex_trylock(&adapter->crit_lock))
if (!mutex_trylock(&adapter->crit_lock)) {
if (adapter->state == __IAVF_REMOVE)
return;

goto restart_watchdog;
}

if (adapter->flags & IAVF_FLAG_PF_COMMS_FAILED)
iavf_change_state(adapter, __IAVF_COMM_FAILED);

if (adapter->flags & IAVF_FLAG_RESET_NEEDED &&
adapter->state != __IAVF_RESETTING) {
iavf_change_state(adapter, __IAVF_RESETTING);
if (adapter->flags & IAVF_FLAG_RESET_NEEDED) {
adapter->aq_required = 0;
adapter->current_op = VIRTCHNL_OP_UNKNOWN;
mutex_unlock(&adapter->crit_lock);
queue_work(iavf_wq, &adapter->reset_task);
return;
}

switch (adapter->state) {
Expand Down Expand Up @@ -2419,6 +2424,15 @@ static void iavf_watchdog_task(struct work_struct *work)
msecs_to_jiffies(1));
return;
case __IAVF_INIT_FAILED:
if (test_bit(__IAVF_IN_REMOVE_TASK,
&adapter->crit_section)) {
/* Do not update the state and do not reschedule
* watchdog task, iavf_remove should handle this state
* as it can loop forever
*/
mutex_unlock(&adapter->crit_lock);
return;
}
if (++adapter->aq_wait_count > IAVF_AQ_MAX_ERR) {
dev_err(&adapter->pdev->dev,
"Failed to communicate with PF; waiting before retry\n");
Expand All @@ -2435,6 +2449,17 @@ static void iavf_watchdog_task(struct work_struct *work)
queue_delayed_work(iavf_wq, &adapter->watchdog_task, HZ);
return;
case __IAVF_COMM_FAILED:
if (test_bit(__IAVF_IN_REMOVE_TASK,
&adapter->crit_section)) {
/* Set state to __IAVF_INIT_FAILED and perform remove
* steps. Remove IAVF_FLAG_PF_COMMS_FAILED so the task
* doesn't bring the state back to __IAVF_COMM_FAILED.
*/
iavf_change_state(adapter, __IAVF_INIT_FAILED);
adapter->flags &= ~IAVF_FLAG_PF_COMMS_FAILED;
mutex_unlock(&adapter->crit_lock);
return;
}
reg_val = rd32(hw, IAVF_VFGEN_RSTAT) &
IAVF_VFGEN_RSTAT_VFR_STATE_MASK;
if (reg_val == VIRTCHNL_VFR_VFACTIVE ||
Expand Down Expand Up @@ -2507,7 +2532,8 @@ static void iavf_watchdog_task(struct work_struct *work)
schedule_delayed_work(&adapter->client_task, msecs_to_jiffies(5));
mutex_unlock(&adapter->crit_lock);
restart_watchdog:
queue_work(iavf_wq, &adapter->adminq_task);
if (adapter->state >= __IAVF_DOWN)
queue_work(iavf_wq, &adapter->adminq_task);
if (adapter->aq_required)
queue_delayed_work(iavf_wq, &adapter->watchdog_task,
msecs_to_jiffies(20));
Expand Down Expand Up @@ -2601,13 +2627,13 @@ static void iavf_reset_task(struct work_struct *work)
/* When device is being removed it doesn't make sense to run the reset
* task, just return in such a case.
*/
if (mutex_is_locked(&adapter->remove_lock))
return;
if (!mutex_trylock(&adapter->crit_lock)) {
if (adapter->state != __IAVF_REMOVE)
queue_work(iavf_wq, &adapter->reset_task);

if (iavf_lock_timeout(&adapter->crit_lock, 200)) {
schedule_work(&adapter->reset_task);
return;
}

while (!mutex_trylock(&adapter->client_lock))
usleep_range(500, 1000);
if (CLIENT_ENABLED(adapter)) {
Expand Down Expand Up @@ -2662,6 +2688,7 @@ static void iavf_reset_task(struct work_struct *work)
reg_val);
iavf_disable_vf(adapter);
mutex_unlock(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
return; /* Do not attempt to reinit. It's dead, Jim. */
}

Expand All @@ -2670,8 +2697,7 @@ static void iavf_reset_task(struct work_struct *work)
* ndo_open() returning, so we can't assume it means all our open
* tasks have finished, since we're not holding the rtnl_lock here.
*/
running = ((adapter->state == __IAVF_RUNNING) ||
(adapter->state == __IAVF_RESETTING));
running = adapter->state == __IAVF_RUNNING;

if (running) {
netdev->flags &= ~IFF_UP;
Expand Down Expand Up @@ -2826,13 +2852,19 @@ static void iavf_adminq_task(struct work_struct *work)
if (adapter->flags & IAVF_FLAG_PF_COMMS_FAILED)
goto out;

if (!mutex_trylock(&adapter->crit_lock)) {
if (adapter->state == __IAVF_REMOVE)
return;

queue_work(iavf_wq, &adapter->adminq_task);
goto out;
}

event.buf_len = IAVF_MAX_AQ_BUF_SIZE;
event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
if (!event.msg_buf)
goto out;

if (iavf_lock_timeout(&adapter->crit_lock, 200))
goto freedom;
do {
ret = iavf_clean_arq_element(hw, &event, &pending);
v_op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high);
Expand All @@ -2848,6 +2880,24 @@ static void iavf_adminq_task(struct work_struct *work)
} while (pending);
mutex_unlock(&adapter->crit_lock);

if ((adapter->flags & IAVF_FLAG_SETUP_NETDEV_FEATURES)) {
if (adapter->netdev_registered ||
!test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) {
struct net_device *netdev = adapter->netdev;

rtnl_lock();
netdev_update_features(netdev);
rtnl_unlock();
/* Request VLAN offload settings */
if (VLAN_V2_ALLOWED(adapter))
iavf_set_vlan_offload_features
(adapter, 0, netdev->features);

iavf_set_queue_vlan_tag_loc(adapter);
}

adapter->flags &= ~IAVF_FLAG_SETUP_NETDEV_FEATURES;
}
if ((adapter->flags &
(IAVF_FLAG_RESET_PENDING | IAVF_FLAG_RESET_NEEDED)) ||
adapter->state == __IAVF_RESETTING)
Expand Down Expand Up @@ -3800,11 +3850,12 @@ static int iavf_close(struct net_device *netdev)
struct iavf_adapter *adapter = netdev_priv(netdev);
int status;

if (adapter->state <= __IAVF_DOWN_PENDING)
return 0;
mutex_lock(&adapter->crit_lock);

while (!mutex_trylock(&adapter->crit_lock))
usleep_range(500, 1000);
if (adapter->state <= __IAVF_DOWN_PENDING) {
mutex_unlock(&adapter->crit_lock);
return 0;
}

set_bit(__IAVF_VSI_DOWN, adapter->vsi.state);
if (CLIENT_ENABLED(adapter))
Expand Down Expand Up @@ -3853,8 +3904,11 @@ static int iavf_change_mtu(struct net_device *netdev, int new_mtu)
iavf_notify_client_l2_params(&adapter->vsi);
adapter->flags |= IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
}
adapter->flags |= IAVF_FLAG_RESET_NEEDED;
queue_work(iavf_wq, &adapter->reset_task);

if (netif_running(netdev)) {
adapter->flags |= IAVF_FLAG_RESET_NEEDED;
queue_work(iavf_wq, &adapter->reset_task);
}

return 0;
}
Expand Down Expand Up @@ -4431,7 +4485,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
mutex_init(&adapter->crit_lock);
mutex_init(&adapter->client_lock);
mutex_init(&adapter->remove_lock);
mutex_init(&hw->aq.asq_mutex);
mutex_init(&hw->aq.arq_mutex);

Expand Down Expand Up @@ -4547,7 +4600,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 @@ -4556,14 +4608,30 @@ static void iavf_remove(struct pci_dev *pdev)
struct iavf_cloud_filter *cf, *cftmp;
struct iavf_hw *hw = &adapter->hw;
int err;
/* Indicate we are in remove and not to run reset_task */
mutex_lock(&adapter->remove_lock);
cancel_work_sync(&adapter->reset_task);

set_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section);
/* 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 ||
adapter->state == __IAVF_INIT_FAILED) {
mutex_unlock(&adapter->crit_lock);
break;
}

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

if (adapter->netdev_registered) {
unregister_netdev(netdev);
rtnl_lock();
unregister_netdevice(netdev);
adapter->netdev_registered = false;
rtnl_unlock();
}
if (CLIENT_ALLOWED(adapter)) {
err = iavf_lan_del_device(adapter);
Expand All @@ -4572,44 +4640,35 @@ static void iavf_remove(struct pci_dev *pdev)
err);
}

mutex_lock(&adapter->crit_lock);
dev_info(&adapter->pdev->dev, "Remove device\n");
iavf_change_state(adapter, __IAVF_REMOVE);

iavf_request_reset(adapter);
msleep(50);
/* If the FW isn't responding, kick it once, but only once. */
if (!iavf_asq_done(hw)) {
iavf_request_reset(adapter);
msleep(50);
}
if (iavf_lock_timeout(&adapter->crit_lock, 5000))
dev_warn(&adapter->pdev->dev, "failed to acquire crit_lock in %s\n", __FUNCTION__);

dev_info(&adapter->pdev->dev, "Removing device\n");
iavf_misc_irq_disable(adapter);
/* Shut down all the garbage mashers on the detention level */
iavf_change_state(adapter, __IAVF_REMOVE);
cancel_work_sync(&adapter->reset_task);
cancel_delayed_work_sync(&adapter->watchdog_task);
cancel_work_sync(&adapter->adminq_task);
cancel_delayed_work_sync(&adapter->client_task);

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);
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);

cancel_delayed_work_sync(&adapter->watchdog_task);

cancel_work_sync(&adapter->adminq_task);

iavf_free_rss(adapter);

if (hw->aq.asq.count)
Expand All @@ -4621,8 +4680,6 @@ static void iavf_remove(struct pci_dev *pdev)
mutex_destroy(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
mutex_destroy(&adapter->crit_lock);
mutex_unlock(&adapter->remove_lock);
mutex_destroy(&adapter->remove_lock);

iounmap(hw->hw_addr);
pci_release_regions(pdev);
Expand Down
24 changes: 1 addition & 23 deletions drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2146,29 +2146,7 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
sizeof(adapter->vlan_v2_caps)));

iavf_process_config(adapter);

/* unlock crit_lock before acquiring rtnl_lock as other
* processes holding rtnl_lock could be waiting for the same
* crit_lock
*/
mutex_unlock(&adapter->crit_lock);
/* VLAN capabilities can change during VFR, so make sure to
* update the netdev features with the new capabilities
*/
rtnl_lock();
netdev_update_features(netdev);
rtnl_unlock();
if (iavf_lock_timeout(&adapter->crit_lock, 10000))
dev_warn(&adapter->pdev->dev, "failed to acquire crit_lock in %s\n",
__FUNCTION__);

/* Request VLAN offload settings */
if (VLAN_V2_ALLOWED(adapter))
iavf_set_vlan_offload_features(adapter, 0,
netdev->features);

iavf_set_queue_vlan_tag_loc(adapter);

adapter->flags |= IAVF_FLAG_SETUP_NETDEV_FEATURES;
}
break;
case VIRTCHNL_OP_ENABLE_QUEUES:
Expand Down

0 comments on commit 519ca6f

Please sign in to comment.