Skip to content

Commit cae03e5

Browse files
Stanislav Fomichevkuba-moo
Stanislav Fomichev
authored and
kuba-moo
committed
net: hold netdev instance lock during queue operations
For the drivers that use queue management API, switch to the mode where core stack holds the netdev instance lock. This affects the following drivers: - bnxt - gve - netdevsim Originally I locked only start/stop, but switched to holding the lock over all iterations to make them look atomic to the device (feels like it should be easier to reason about). Reviewed-by: Eric Dumazet <[email protected]> Cc: Saeed Mahameed <[email protected]> Signed-off-by: Stanislav Fomichev <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent a0527ee commit cae03e5

File tree

6 files changed

+41
-25
lines changed

6 files changed

+41
-25
lines changed

‎drivers/net/ethernet/broadcom/bnxt/bnxt.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11508,7 +11508,7 @@ static int bnxt_request_irq(struct bnxt *bp)
1150811508
if (rc)
1150911509
break;
1151011510

11511-
netif_napi_set_irq(&bp->bnapi[i]->napi, irq->vector);
11511+
netif_napi_set_irq_locked(&bp->bnapi[i]->napi, irq->vector);
1151211512
irq->requested = 1;
1151311513

1151411514
if (zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL)) {
@@ -11557,9 +11557,9 @@ static void bnxt_del_napi(struct bnxt *bp)
1155711557
for (i = 0; i < bp->cp_nr_rings; i++) {
1155811558
struct bnxt_napi *bnapi = bp->bnapi[i];
1155911559

11560-
__netif_napi_del(&bnapi->napi);
11560+
__netif_napi_del_locked(&bnapi->napi);
1156111561
}
11562-
/* We called __netif_napi_del(), we need
11562+
/* We called __netif_napi_del_locked(), we need
1156311563
* to respect an RCU grace period before freeing napi structures.
1156411564
*/
1156511565
synchronize_net();
@@ -11578,12 +11578,12 @@ static void bnxt_init_napi(struct bnxt *bp)
1157811578
cp_nr_rings--;
1157911579
for (i = 0; i < cp_nr_rings; i++) {
1158011580
bnapi = bp->bnapi[i];
11581-
netif_napi_add_config(bp->dev, &bnapi->napi, poll_fn,
11582-
bnapi->index);
11581+
netif_napi_add_config_locked(bp->dev, &bnapi->napi, poll_fn,
11582+
bnapi->index);
1158311583
}
1158411584
if (BNXT_CHIP_TYPE_NITRO_A0(bp)) {
1158511585
bnapi = bp->bnapi[cp_nr_rings];
11586-
netif_napi_add(bp->dev, &bnapi->napi, bnxt_poll_nitroa0);
11586+
netif_napi_add_locked(bp->dev, &bnapi->napi, bnxt_poll_nitroa0);
1158711587
}
1158811588
}
1158911589

@@ -11604,7 +11604,7 @@ static void bnxt_disable_napi(struct bnxt *bp)
1160411604
cpr->sw_stats->tx.tx_resets++;
1160511605
if (bnapi->in_reset)
1160611606
cpr->sw_stats->rx.rx_resets++;
11607-
napi_disable(&bnapi->napi);
11607+
napi_disable_locked(&bnapi->napi);
1160811608
}
1160911609
}
1161011610

@@ -11626,7 +11626,7 @@ static void bnxt_enable_napi(struct bnxt *bp)
1162611626
INIT_WORK(&cpr->dim.work, bnxt_dim_work);
1162711627
cpr->dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
1162811628
}
11629-
napi_enable(&bnapi->napi);
11629+
napi_enable_locked(&bnapi->napi);
1163011630
}
1163111631
}
1163211632

‎drivers/net/ethernet/google/gve/gve_main.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1968,7 +1968,7 @@ static void gve_turndown(struct gve_priv *priv)
19681968
netif_queue_set_napi(priv->dev, idx,
19691969
NETDEV_QUEUE_TYPE_TX, NULL);
19701970

1971-
napi_disable(&block->napi);
1971+
napi_disable_locked(&block->napi);
19721972
}
19731973
for (idx = 0; idx < priv->rx_cfg.num_queues; idx++) {
19741974
int ntfy_idx = gve_rx_idx_to_ntfy(priv, idx);
@@ -1979,7 +1979,7 @@ static void gve_turndown(struct gve_priv *priv)
19791979

19801980
netif_queue_set_napi(priv->dev, idx, NETDEV_QUEUE_TYPE_RX,
19811981
NULL);
1982-
napi_disable(&block->napi);
1982+
napi_disable_locked(&block->napi);
19831983
}
19841984

19851985
/* Stop tx queues */
@@ -2009,7 +2009,7 @@ static void gve_turnup(struct gve_priv *priv)
20092009
if (!gve_tx_was_added_to_block(priv, idx))
20102010
continue;
20112011

2012-
napi_enable(&block->napi);
2012+
napi_enable_locked(&block->napi);
20132013

20142014
if (idx < priv->tx_cfg.num_queues)
20152015
netif_queue_set_napi(priv->dev, idx,
@@ -2037,7 +2037,7 @@ static void gve_turnup(struct gve_priv *priv)
20372037
if (!gve_rx_was_added_to_block(priv, idx))
20382038
continue;
20392039

2040-
napi_enable(&block->napi);
2040+
napi_enable_locked(&block->napi);
20412041
netif_queue_set_napi(priv->dev, idx, NETDEV_QUEUE_TYPE_RX,
20422042
&block->napi);
20432043

@@ -2887,6 +2887,7 @@ static int gve_suspend(struct pci_dev *pdev, pm_message_t state)
28872887

28882888
priv->suspend_cnt++;
28892889
rtnl_lock();
2890+
netdev_lock(netdev);
28902891
if (was_up && gve_close(priv->dev)) {
28912892
/* If the dev was up, attempt to close, if close fails, reset */
28922893
gve_reset_and_teardown(priv, was_up);
@@ -2895,6 +2896,7 @@ static int gve_suspend(struct pci_dev *pdev, pm_message_t state)
28952896
gve_teardown_priv_resources(priv);
28962897
}
28972898
priv->up_before_suspend = was_up;
2899+
netdev_unlock(netdev);
28982900
rtnl_unlock();
28992901
return 0;
29002902
}
@@ -2907,7 +2909,9 @@ static int gve_resume(struct pci_dev *pdev)
29072909

29082910
priv->resume_cnt++;
29092911
rtnl_lock();
2912+
netdev_lock(netdev);
29102913
err = gve_reset_recovery(priv, priv->up_before_suspend);
2914+
netdev_unlock(netdev);
29112915
rtnl_unlock();
29122916
return err;
29132917
}

‎drivers/net/ethernet/google/gve/gve_utils.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,13 @@ void gve_add_napi(struct gve_priv *priv, int ntfy_idx,
110110
{
111111
struct gve_notify_block *block = &priv->ntfy_blocks[ntfy_idx];
112112

113-
netif_napi_add(priv->dev, &block->napi, gve_poll);
114-
netif_napi_set_irq(&block->napi, block->irq);
113+
netif_napi_add_locked(priv->dev, &block->napi, gve_poll);
114+
netif_napi_set_irq_locked(&block->napi, block->irq);
115115
}
116116

117117
void gve_remove_napi(struct gve_priv *priv, int ntfy_idx)
118118
{
119119
struct gve_notify_block *block = &priv->ntfy_blocks[ntfy_idx];
120120

121-
netif_napi_del(&block->napi);
121+
netif_napi_del_locked(&block->napi);
122122
}

‎drivers/net/netdevsim/netdev.c

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,8 @@ nsim_queue_mem_alloc(struct net_device *dev, void *per_queue_mem, int idx)
683683
goto err_free;
684684

685685
if (!ns->rq_reset_mode)
686-
netif_napi_add_config(dev, &qmem->rq->napi, nsim_poll, idx);
686+
netif_napi_add_config_locked(dev, &qmem->rq->napi, nsim_poll,
687+
idx);
687688

688689
return 0;
689690

@@ -700,7 +701,7 @@ static void nsim_queue_mem_free(struct net_device *dev, void *per_queue_mem)
700701
page_pool_destroy(qmem->pp);
701702
if (qmem->rq) {
702703
if (!ns->rq_reset_mode)
703-
netif_napi_del(&qmem->rq->napi);
704+
netif_napi_del_locked(&qmem->rq->napi);
704705
page_pool_destroy(qmem->rq->page_pool);
705706
nsim_queue_free(qmem->rq);
706707
}
@@ -712,25 +713,29 @@ nsim_queue_start(struct net_device *dev, void *per_queue_mem, int idx)
712713
struct nsim_queue_mem *qmem = per_queue_mem;
713714
struct netdevsim *ns = netdev_priv(dev);
714715

716+
netdev_assert_locked(dev);
717+
715718
if (ns->rq_reset_mode == 1) {
716719
ns->rq[idx]->page_pool = qmem->pp;
717-
napi_enable(&ns->rq[idx]->napi);
720+
napi_enable_locked(&ns->rq[idx]->napi);
718721
return 0;
719722
}
720723

721724
/* netif_napi_add()/_del() should normally be called from alloc/free,
722725
* here we want to test various call orders.
723726
*/
724727
if (ns->rq_reset_mode == 2) {
725-
netif_napi_del(&ns->rq[idx]->napi);
726-
netif_napi_add_config(dev, &qmem->rq->napi, nsim_poll, idx);
728+
netif_napi_del_locked(&ns->rq[idx]->napi);
729+
netif_napi_add_config_locked(dev, &qmem->rq->napi, nsim_poll,
730+
idx);
727731
} else if (ns->rq_reset_mode == 3) {
728-
netif_napi_add_config(dev, &qmem->rq->napi, nsim_poll, idx);
729-
netif_napi_del(&ns->rq[idx]->napi);
732+
netif_napi_add_config_locked(dev, &qmem->rq->napi, nsim_poll,
733+
idx);
734+
netif_napi_del_locked(&ns->rq[idx]->napi);
730735
}
731736

732737
ns->rq[idx] = qmem->rq;
733-
napi_enable(&ns->rq[idx]->napi);
738+
napi_enable_locked(&ns->rq[idx]->napi);
734739

735740
return 0;
736741
}
@@ -740,7 +745,9 @@ static int nsim_queue_stop(struct net_device *dev, void *per_queue_mem, int idx)
740745
struct nsim_queue_mem *qmem = per_queue_mem;
741746
struct netdevsim *ns = netdev_priv(dev);
742747

743-
napi_disable(&ns->rq[idx]->napi);
748+
netdev_assert_locked(dev);
749+
750+
napi_disable_locked(&ns->rq[idx]->napi);
744751

745752
if (ns->rq_reset_mode == 1) {
746753
qmem->pp = ns->rq[idx]->page_pool;

‎include/linux/netdevice.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2755,7 +2755,7 @@ static inline void netdev_assert_locked_or_invisible(struct net_device *dev)
27552755

27562756
static inline bool netdev_need_ops_lock(struct net_device *dev)
27572757
{
2758-
bool ret = false;
2758+
bool ret = !!dev->queue_mgmt_ops;
27592759

27602760
#if IS_ENABLED(CONFIG_NET_SHAPER)
27612761
ret |= !!dev->netdev_ops->net_shaper_ops;

‎net/core/netdev_rx_queue.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
3030
goto err_free_new_mem;
3131
}
3232

33+
netdev_lock(dev);
34+
3335
err = qops->ndo_queue_mem_alloc(dev, new_mem, rxq_idx);
3436
if (err)
3537
goto err_free_old_mem;
@@ -52,6 +54,8 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
5254

5355
qops->ndo_queue_mem_free(dev, old_mem);
5456

57+
netdev_unlock(dev);
58+
5559
kvfree(old_mem);
5660
kvfree(new_mem);
5761

@@ -76,6 +80,7 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
7680
qops->ndo_queue_mem_free(dev, new_mem);
7781

7882
err_free_old_mem:
83+
netdev_unlock(dev);
7984
kvfree(old_mem);
8085

8186
err_free_new_mem:

0 commit comments

Comments
 (0)