Re: [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: migrate to netdev ops lock

2026-02-06 Thread Nowlin, Alexander 
> -Original Message-
> From: Intel-wired-lan  On Behalf Of 
> Alexander Lobakin
> Sent: Thursday, December 4, 2025 7:52 AM
> To: [email protected]
> Cc: Lobakin, Aleksander ; Nguyen, Anthony L 
> ; Kitszel, Przemyslaw 
> ; Andrew Lunn ; 
> David S. Miller ; Eric Dumazet ; 
> Jakub Kicinski ; Paolo Abeni ; Simon 
> Horman ; Keller, Jacob E 
> ; Loktionov, Aleksandr 
> ; NXNE CNSE OSDT ITP Upstreaming 
> ; [email protected]; linux-
> [email protected]
> Subject: [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: migrate to netdev ops 
> lock
> 
> Queue management ops unconditionally enable netdev locking. The same lock is 
> taken by default by several NAPI configuration functions, such as 
> napi_enable() and netif_napi_set_irq().
> Request ops locking in advance and make sure we use the _locked counterparts 
> of those functions to avoid deadlocks, taking the lock manually where needed 
> (suspend/resume, queue rebuild and resets).
> 
> Reviewed-by: Jacob Keller 
> Reviewed-by: Aleksandr Loktionov 
> Signed-off-by: Alexander Lobakin 
> ---
>  drivers/net/ethernet/intel/ice/ice_lib.h|  6 ++-
>  drivers/net/ethernet/intel/ice/ice_lib.c| 56 +
>  drivers/net/ethernet/intel/ice/ice_main.c   | 49 ++
>  drivers/net/ethernet/intel/ice/ice_sf_eth.c |  1 +
>  drivers/net/ethernet/intel/ice/ice_xsk.c|  4 +-
>  5 files changed, 82 insertions(+), 34 deletions(-)

Tested-by: Alexander Nowlin

Re: [Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: migrate to netdev ops lock

2026-01-19 Thread Alexander Lobakin 
From: Alexander Lobakin 
Date: Thu,  4 Dec 2025 16:51:31 +0100

> Queue management ops unconditionally enable netdev locking. The same
> lock is taken by default by several NAPI configuration functions,
> such as napi_enable() and netif_napi_set_irq().
> Request ops locking in advance and make sure we use the _locked
> counterparts of those functions to avoid deadlocks, taking the lock
> manually where needed (suspend/resume, queue rebuild and resets).
> 
> Reviewed-by: Jacob Keller 
> Reviewed-by: Aleksandr Loktionov 
> Signed-off-by: Alexander Lobakin 
Note: Larysa found that this commit breaks `ethtool -L` -- the system
hangs. Seems like some of the functions called during the queue
reconfiguration still take the netdev lock and a deadlock happens
(I definitely tested `ethtool -G`, but might've forgotten to test
`-L`...).

I'll try to fix this ASAP and send a fixup patch. Since nobody (?)
reported this earlier, maybe it's not worth dropping the series from
the next-queue in the meantime...

Thanks,
Olek

[Intel-wired-lan] [PATCH iwl-next v2 3/5] ice: migrate to netdev ops lock

2025-12-04 Thread Alexander Lobakin 
Queue management ops unconditionally enable netdev locking. The same
lock is taken by default by several NAPI configuration functions,
such as napi_enable() and netif_napi_set_irq().
Request ops locking in advance and make sure we use the _locked
counterparts of those functions to avoid deadlocks, taking the lock
manually where needed (suspend/resume, queue rebuild and resets).

Reviewed-by: Jacob Keller 
Reviewed-by: Aleksandr Loktionov 
Signed-off-by: Alexander Lobakin 
---
 drivers/net/ethernet/intel/ice/ice_lib.h|  6 ++-
 drivers/net/ethernet/intel/ice/ice_lib.c| 56 +
 drivers/net/ethernet/intel/ice/ice_main.c   | 49 ++
 drivers/net/ethernet/intel/ice/ice_sf_eth.c |  1 +
 drivers/net/ethernet/intel/ice/ice_xsk.c|  4 +-
 5 files changed, 82 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h 
b/drivers/net/ethernet/intel/ice/ice_lib.h
index 49454d98dcfe..347b63e497e7 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -53,9 +53,11 @@ struct ice_vsi *
 ice_vsi_setup(struct ice_pf *pf, struct ice_vsi_cfg_params *params);
 
 void ice_vsi_set_napi_queues(struct ice_vsi *vsi);
-void ice_napi_add(struct ice_vsi *vsi);
-
+void ice_vsi_set_napi_queues_locked(struct ice_vsi *vsi);
 void ice_vsi_clear_napi_queues(struct ice_vsi *vsi);
+void ice_vsi_clear_napi_queues_locked(struct ice_vsi *vsi);
+
+void ice_napi_add(struct ice_vsi *vsi);
 
 int ice_vsi_release(struct ice_vsi *vsi);
 
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c 
b/drivers/net/ethernet/intel/ice/ice_lib.c
index 17d92ba65128..ac5d95a28f72 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -2703,7 +2703,7 @@ void ice_vsi_close(struct ice_vsi *vsi)
if (!test_and_set_bit(ICE_VSI_DOWN, vsi->state))
ice_down(vsi);
 
-   ice_vsi_clear_napi_queues(vsi);
+   ice_vsi_clear_napi_queues_locked(vsi);
ice_vsi_free_irq(vsi);
ice_vsi_free_tx_rings(vsi);
ice_vsi_free_rx_rings(vsi);
@@ -2772,12 +2772,13 @@ void ice_dis_vsi(struct ice_vsi *vsi, bool locked)
 }
 
 /**
- * ice_vsi_set_napi_queues - associate netdev queues with napi
+ * ice_vsi_set_napi_queues_locked - associate netdev queues with napi
  * @vsi: VSI pointer
  *
  * Associate queue[s] with napi for all vectors.
+ * Must be called only with the netdev_lock taken.
  */
-void ice_vsi_set_napi_queues(struct ice_vsi *vsi)
+void ice_vsi_set_napi_queues_locked(struct ice_vsi *vsi)
 {
struct net_device *netdev = vsi->netdev;
int q_idx, v_idx;
@@ -2785,7 +2786,6 @@ void ice_vsi_set_napi_queues(struct ice_vsi *vsi)
if (!netdev)
return;
 
-   ASSERT_RTNL();
ice_for_each_rxq(vsi, q_idx)
netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_RX,
 &vsi->rx_rings[q_idx]->q_vector->napi);
@@ -2797,17 +2797,37 @@ void ice_vsi_set_napi_queues(struct ice_vsi *vsi)
ice_for_each_q_vector(vsi, v_idx) {
struct ice_q_vector *q_vector = vsi->q_vectors[v_idx];
 
-   netif_napi_set_irq(&q_vector->napi, q_vector->irq.virq);
+   netif_napi_set_irq_locked(&q_vector->napi, q_vector->irq.virq);
}
 }
 
 /**
- * ice_vsi_clear_napi_queues - dissociate netdev queues from napi
+ * ice_vsi_set_napi_queues - associate VSI queues with NAPIs
  * @vsi: VSI pointer
  *
+ * Version of ice_vsi_set_napi_queues_locked() that takes the netdev_lock,
+ * to use it outside of the net_device_ops context.
+ */
+void ice_vsi_set_napi_queues(struct ice_vsi *vsi)
+{
+   struct net_device *netdev = vsi->netdev;
+
+   if (!netdev)
+   return;
+
+   netdev_lock(netdev);
+   ice_vsi_set_napi_queues_locked(vsi);
+   netdev_unlock(netdev);
+}
+
+/**
+ * ice_vsi_clear_napi_queues_locked - dissociate netdev queues from napi
+ * @vsi: VSI to process
+ *
  * Clear the association between all VSI queues queue[s] and napi.
+ * Must be called only with the netdev_lock taken.
  */
-void ice_vsi_clear_napi_queues(struct ice_vsi *vsi)
+void ice_vsi_clear_napi_queues_locked(struct ice_vsi *vsi)
 {
struct net_device *netdev = vsi->netdev;
int q_idx, v_idx;
@@ -2815,12 +2835,11 @@ void ice_vsi_clear_napi_queues(struct ice_vsi *vsi)
if (!netdev)
return;
 
-   ASSERT_RTNL();
/* Clear the NAPI's interrupt number */
ice_for_each_q_vector(vsi, v_idx) {
struct ice_q_vector *q_vector = vsi->q_vectors[v_idx];
 
-   netif_napi_set_irq(&q_vector->napi, -1);
+   netif_napi_set_irq_locked(&q_vector->napi, -1);
}
 
ice_for_each_txq(vsi, q_idx)
@@ -2830,6 +2849,25 @@ void ice_vsi_clear_napi_queues(struct ice_vsi *vsi)
netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_RX, NULL);
 }
 
+/**
+ * ice_vsi_clear_napi_queues - diss