Skip to content

Commit

Permalink
net: add netdev_lock() / netdev_unlock() helpers
Browse files Browse the repository at this point in the history 
Add helpers for locking the netdev instance, use it in drivers
and the shaper code. This will make grepping for the lock usage
much easier, as we extend the lock to cover more fields.

Reviewed-by: Joe Damato <jdamato@fastly.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Link: https://patch.msgid.link/20250115035319.559603-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Jan 16, 2025
1 parent 0b6f659 commit ebda2f0
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 44 deletions.
74 changes: 37 additions & 37 deletions drivers/net/ethernet/intel/iavf/iavf_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1977,7 +1977,7 @@ static void iavf_finish_config(struct work_struct *work)
* The dev->lock is needed to update the queue number
*/
rtnl_lock();
mutex_lock(&adapter->netdev->lock);
netdev_lock(adapter->netdev);
mutex_lock(&adapter->crit_lock);

if ((adapter->flags & IAVF_FLAG_SETUP_NETDEV_FEATURES) &&
Expand All @@ -1997,7 +1997,7 @@ static void iavf_finish_config(struct work_struct *work)
netif_set_real_num_tx_queues(adapter->netdev, pairs);

if (adapter->netdev->reg_state != NETREG_REGISTERED) {
mutex_unlock(&adapter->netdev->lock);
netdev_unlock(adapter->netdev);
netdev_released = true;
err = register_netdevice(adapter->netdev);
if (err) {
Expand Down Expand Up @@ -2027,7 +2027,7 @@ static void iavf_finish_config(struct work_struct *work)
out:
mutex_unlock(&adapter->crit_lock);
if (!netdev_released)
mutex_unlock(&adapter->netdev->lock);
netdev_unlock(adapter->netdev);
rtnl_unlock();
}

Expand Down Expand Up @@ -2724,10 +2724,10 @@ static void iavf_watchdog_task(struct work_struct *work)
struct iavf_hw *hw = &adapter->hw;
u32 reg_val;

mutex_lock(&netdev->lock);
netdev_lock(netdev);
if (!mutex_trylock(&adapter->crit_lock)) {
if (adapter->state == __IAVF_REMOVE) {
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
return;
}

Expand All @@ -2741,35 +2741,35 @@ static void iavf_watchdog_task(struct work_struct *work)
case __IAVF_STARTUP:
iavf_startup(adapter);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq, &adapter->watchdog_task,
msecs_to_jiffies(30));
return;
case __IAVF_INIT_VERSION_CHECK:
iavf_init_version_check(adapter);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq, &adapter->watchdog_task,
msecs_to_jiffies(30));
return;
case __IAVF_INIT_GET_RESOURCES:
iavf_init_get_resources(adapter);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq, &adapter->watchdog_task,
msecs_to_jiffies(1));
return;
case __IAVF_INIT_EXTENDED_CAPS:
iavf_init_process_extended_caps(adapter);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq, &adapter->watchdog_task,
msecs_to_jiffies(1));
return;
case __IAVF_INIT_CONFIG_ADAPTER:
iavf_init_config_adapter(adapter);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq, &adapter->watchdog_task,
msecs_to_jiffies(1));
return;
Expand All @@ -2781,7 +2781,7 @@ static void iavf_watchdog_task(struct work_struct *work)
* as it can loop forever
*/
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
return;
}
if (++adapter->aq_wait_count > IAVF_AQ_MAX_ERR) {
Expand All @@ -2790,15 +2790,15 @@ static void iavf_watchdog_task(struct work_struct *work)
adapter->flags |= IAVF_FLAG_PF_COMMS_FAILED;
iavf_shutdown_adminq(hw);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq,
&adapter->watchdog_task, (5 * HZ));
return;
}
/* Try again from failed step*/
iavf_change_state(adapter, adapter->last_state);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq, &adapter->watchdog_task, HZ);
return;
case __IAVF_COMM_FAILED:
Expand All @@ -2811,7 +2811,7 @@ static void iavf_watchdog_task(struct work_struct *work)
iavf_change_state(adapter, __IAVF_INIT_FAILED);
adapter->flags &= ~IAVF_FLAG_PF_COMMS_FAILED;
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
return;
}
reg_val = rd32(hw, IAVF_VFGEN_RSTAT) &
Expand All @@ -2831,14 +2831,14 @@ static void iavf_watchdog_task(struct work_struct *work)
adapter->aq_required = 0;
adapter->current_op = VIRTCHNL_OP_UNKNOWN;
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq,
&adapter->watchdog_task,
msecs_to_jiffies(10));
return;
case __IAVF_RESETTING:
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq, &adapter->watchdog_task,
HZ * 2);
return;
Expand Down Expand Up @@ -2869,7 +2869,7 @@ static void iavf_watchdog_task(struct work_struct *work)
case __IAVF_REMOVE:
default:
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
return;
}

Expand All @@ -2881,14 +2881,14 @@ static void iavf_watchdog_task(struct work_struct *work)
dev_err(&adapter->pdev->dev, "Hardware reset detected\n");
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_PENDING);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
queue_delayed_work(adapter->wq,
&adapter->watchdog_task, HZ * 2);
return;
}

mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
restart_watchdog:
if (adapter->state >= __IAVF_DOWN)
queue_work(adapter->wq, &adapter->adminq_task);
Expand Down Expand Up @@ -3015,12 +3015,12 @@ 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.
*/
mutex_lock(&netdev->lock);
netdev_lock(netdev);
if (!mutex_trylock(&adapter->crit_lock)) {
if (adapter->state != __IAVF_REMOVE)
queue_work(adapter->wq, &adapter->reset_task);

mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
return;
}

Expand Down Expand Up @@ -3068,7 +3068,7 @@ static void iavf_reset_task(struct work_struct *work)
reg_val);
iavf_disable_vf(adapter);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
return; /* Do not attempt to reinit. It's dead, Jim. */
}

Expand Down Expand Up @@ -3209,7 +3209,7 @@ static void iavf_reset_task(struct work_struct *work)

wake_up(&adapter->reset_waitqueue);
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);

return;
reset_err:
Expand All @@ -3220,7 +3220,7 @@ static void iavf_reset_task(struct work_struct *work)
iavf_disable_vf(adapter);

mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
dev_err(&adapter->pdev->dev, "failed to allocate resources during reinit\n");
}

Expand Down Expand Up @@ -3692,10 +3692,10 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
if (test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section))
return 0;

mutex_lock(&netdev->lock);
netdev_lock(netdev);
netif_set_real_num_rx_queues(netdev, total_qps);
netif_set_real_num_tx_queues(netdev, total_qps);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);

return ret;
}
Expand Down Expand Up @@ -4365,15 +4365,15 @@ static int iavf_open(struct net_device *netdev)
return -EIO;
}

mutex_lock(&netdev->lock);
netdev_lock(netdev);
while (!mutex_trylock(&adapter->crit_lock)) {
/* If we are in __IAVF_INIT_CONFIG_ADAPTER state the crit_lock
* is already taken and iavf_open is called from an upper
* device's notifier reacting on NETDEV_REGISTER event.
* We have to leave here to avoid dead lock.
*/
if (adapter->state == __IAVF_INIT_CONFIG_ADAPTER) {
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
return -EBUSY;
}

Expand Down Expand Up @@ -4424,7 +4424,7 @@ static int iavf_open(struct net_device *netdev)
iavf_irq_enable(adapter, true);

mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);

return 0;

Expand All @@ -4437,7 +4437,7 @@ static int iavf_open(struct net_device *netdev)
iavf_free_all_tx_resources(adapter);
err_unlock:
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);

return err;
}
Expand All @@ -4459,12 +4459,12 @@ static int iavf_close(struct net_device *netdev)
u64 aq_to_restore;
int status;

mutex_lock(&netdev->lock);
netdev_lock(netdev);
mutex_lock(&adapter->crit_lock);

if (adapter->state <= __IAVF_DOWN_PENDING) {
mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);
return 0;
}

Expand Down Expand Up @@ -4498,7 +4498,7 @@ static int iavf_close(struct net_device *netdev)
iavf_free_traffic_irqs(adapter);

mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);

/* We explicitly don't free resources here because the hardware is
* still active and can DMA into memory. Resources are cleared in
Expand Down Expand Up @@ -5375,7 +5375,7 @@ static int iavf_suspend(struct device *dev_d)

netif_device_detach(netdev);

mutex_lock(&netdev->lock);
netdev_lock(netdev);
mutex_lock(&adapter->crit_lock);

if (netif_running(netdev)) {
Expand All @@ -5387,7 +5387,7 @@ static int iavf_suspend(struct device *dev_d)
iavf_reset_interrupt_capability(adapter);

mutex_unlock(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);

return 0;
}
Expand Down Expand Up @@ -5486,7 +5486,7 @@ static void iavf_remove(struct pci_dev *pdev)
if (netdev->reg_state == NETREG_REGISTERED)
unregister_netdev(netdev);

mutex_lock(&netdev->lock);
netdev_lock(netdev);
mutex_lock(&adapter->crit_lock);
dev_info(&adapter->pdev->dev, "Removing device\n");
iavf_change_state(adapter, __IAVF_REMOVE);
Expand Down Expand Up @@ -5523,7 +5523,7 @@ static void iavf_remove(struct pci_dev *pdev)
mutex_destroy(&hw->aq.asq_mutex);
mutex_unlock(&adapter->crit_lock);
mutex_destroy(&adapter->crit_lock);
mutex_unlock(&netdev->lock);
netdev_unlock(netdev);

iounmap(hw->hw_addr);
pci_release_regions(pdev);
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/netdevsim/ethtool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ nsim_set_channels(struct net_device *dev, struct ethtool_channels *ch)
struct netdevsim *ns = netdev_priv(dev);
int err;

mutex_lock(&dev->lock);
netdev_lock(dev);
err = netif_set_real_num_queues(dev, ch->combined_count,
ch->combined_count);
mutex_unlock(&dev->lock);
netdev_unlock(dev);
if (err)
return err;

Expand Down
23 changes: 21 additions & 2 deletions include/linux/netdevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2444,8 +2444,12 @@ struct net_device {
u32 napi_defer_hard_irqs;

/**
* @lock: protects @net_shaper_hierarchy, feel free to use for other
* netdev-scope protection. Ordering: take after rtnl_lock.
* @lock: netdev-scope lock, protects a small selection of fields.
* Should always be taken using netdev_lock() / netdev_unlock() helpers.
* Drivers are free to use it for other protection.
*
* Protects: @net_shaper_hierarchy.
* Ordering: take after rtnl_lock.
*/
struct mutex lock;

Expand Down Expand Up @@ -2671,6 +2675,21 @@ void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index,
enum netdev_queue_type type,
struct napi_struct *napi);

static inline void netdev_lock(struct net_device *dev)
{
mutex_lock(&dev->lock);
}

static inline void netdev_unlock(struct net_device *dev)
{
mutex_unlock(&dev->lock);
}

static inline void netdev_assert_locked(struct net_device *dev)
{
lockdep_assert_held(&dev->lock);
}

static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)
{
napi->irq = irq;
Expand Down
6 changes: 3 additions & 3 deletions net/shaper/shaper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ static void net_shaper_lock(struct net_shaper_binding *binding)
{
switch (binding->type) {
case NET_SHAPER_BINDING_TYPE_NETDEV:
mutex_lock(&binding->netdev->lock);
netdev_lock(binding->netdev);
break;
}
}
Expand All @@ -49,7 +49,7 @@ static void net_shaper_unlock(struct net_shaper_binding *binding)
{
switch (binding->type) {
case NET_SHAPER_BINDING_TYPE_NETDEV:
mutex_unlock(&binding->netdev->lock);
netdev_unlock(binding->netdev);
break;
}
}
Expand Down Expand Up @@ -1398,7 +1398,7 @@ void net_shaper_set_real_num_tx_queues(struct net_device *dev,
/* Only drivers implementing shapers support ensure
* the lock is acquired in advance.
*/
lockdep_assert_held(&dev->lock);
netdev_assert_locked(dev);

/* Take action only when decreasing the tx queue number. */
for (i = txq; i < dev->real_num_tx_queues; ++i) {
Expand Down

0 comments on commit ebda2f0

Please sign in to comment.