Re: [Intel-wired-lan] [PATCH iwl-next] ice: fix system hang on `ethtool -L`

2026-01-21 Thread Alexander Lobakin
From: Loktionov, Aleksandr 
Date: Wed, 21 Jan 2026 08:18:47 +0100

> 
> 
>> -Original Message-
>> From: Lobakin, Aleksander 
>> Sent: Tuesday, January 20, 2026 6:34 PM
>> To: Nguyen, Anthony L ; intel-wired-
>> [email protected]
>> Cc: Lobakin, Aleksander ; 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];
>> [email protected]
>> Subject: [PATCH iwl-next] ice: fix system hang on `ethtool -L`
>>
>> ice_set_channels() calls ice_vsi_rebuild() under the netdev lock
>> taken, but ice_vsi_rebuild() calls netif_napi_{add,del}() which take
>> the same lock.
>> Add ice_vsi_rebuild_locked() which uses the _locked counterparts of
>> these functions and use it in ice_set_channels().
>>
>> Signed-off-by: Alexander Lobakin 
>> ---
>> Hey Tony, please amend to the patch I replied to.
>> ---
>>  drivers/net/ethernet/intel/ice/ice_base.h |  2 +
>> drivers/net/ethernet/intel/ice/ice_lib.h  |  1 +
>> drivers/net/ethernet/intel/ice/ice_base.c | 63 ---
>> drivers/net/ethernet/intel/ice/ice_lib.c  | 94 ---
>> drivers/net/ethernet/intel/ice/ice_main.c |  5 +-
>>  5 files changed, 143 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_base.h
>> b/drivers/net/ethernet/intel/ice/ice_base.h
>> index d28294247599..99b2c7232829 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_base.h
>> +++ b/drivers/net/ethernet/intel/ice/ice_base.h
>> @@ -12,8 +12,10 @@ int __ice_vsi_get_qs(struct ice_qs_cfg *qs_cfg);
>> int  ice_vsi_ctrl_one_rx_ring(struct ice_vsi *vsi, bool ena, u16
>> rxq_idx, bool wait);  int ice_vsi_wait_one_rx_ring(struct ice_vsi
>> *vsi, bool ena, u16 rxq_idx);
>> +int ice_vsi_alloc_q_vectors_locked(struct ice_vsi *vsi);
>>  int ice_vsi_alloc_q_vectors(struct ice_vsi *vsi);  void
>> ice_vsi_map_rings_to_vectors(struct ice_vsi *vsi);
>> +void ice_vsi_free_q_vectors_locked(struct ice_vsi *vsi);
>>  void ice_vsi_free_q_vectors(struct ice_vsi *vsi);  int
>> ice_vsi_cfg_single_txq(struct ice_vsi *vsi, struct ice_tx_ring
>> **tx_rings,
>> u16 q_idx);
>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h
>> b/drivers/net/ethernet/intel/ice/ice_lib.h
>> index 347b63e497e7..e55b72db72c4 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_lib.h
>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.h
>> @@ -68,6 +68,7 @@ int ice_ena_vsi(struct ice_vsi *vsi, bool locked);
>> void ice_vsi_decfg(struct ice_vsi *vsi);  void ice_dis_vsi(struct
>> ice_vsi *vsi, bool locked);
>>
>> +int ice_vsi_rebuild_locked(struct ice_vsi *vsi, u32 vsi_flags);
>>  int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags);  int
>> ice_vsi_cfg(struct ice_vsi *vsi);  struct ice_vsi
>> *ice_vsi_alloc(struct ice_pf *pf); diff --git
>> a/drivers/net/ethernet/intel/ice/ice_base.c
>> b/drivers/net/ethernet/intel/ice/ice_base.c
>> index 7097324c38f3..65e19815bec5 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_base.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_base.c
>> @@ -153,8 +153,8 @@ static int ice_vsi_alloc_q_vector(struct ice_vsi
>> *vsi, u16 v_idx)
>>   * handler here (i.e. resume, reset/rebuild, etc.)
>>   */
>>  if (vsi->netdev)
>> -netif_napi_add_config(vsi->netdev, &q_vector->napi,
>> -  ice_napi_poll, v_idx);
>> +netif_napi_add_config_locked(vsi->netdev, &q_vector-
>>> napi,
>> + ice_napi_poll, v_idx);
> If you converted ice_vsi_alloc_q_vector() into _locked, should it be 
> lockdep_assert_held(&vsi->netdev->lock); then?

IIRC the core kernel functions check for this and warn already.

> 
> Everything else looks fine.
> Reviewed-by: Aleksandr Loktionov 

Thanks,
Olek


Re: [Intel-wired-lan] [PATCH iwl-next] ice: fix system hang on `ethtool -L`

2026-01-20 Thread Loktionov, Aleksandr


> -Original Message-
> From: Lobakin, Aleksander 
> Sent: Tuesday, January 20, 2026 6:34 PM
> To: Nguyen, Anthony L ; intel-wired-
> [email protected]
> Cc: Lobakin, Aleksander ; 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];
> [email protected]
> Subject: [PATCH iwl-next] ice: fix system hang on `ethtool -L`
> 
> ice_set_channels() calls ice_vsi_rebuild() under the netdev lock
> taken, but ice_vsi_rebuild() calls netif_napi_{add,del}() which take
> the same lock.
> Add ice_vsi_rebuild_locked() which uses the _locked counterparts of
> these functions and use it in ice_set_channels().
> 
> Signed-off-by: Alexander Lobakin 
> ---
> Hey Tony, please amend to the patch I replied to.
> ---
>  drivers/net/ethernet/intel/ice/ice_base.h |  2 +
> drivers/net/ethernet/intel/ice/ice_lib.h  |  1 +
> drivers/net/ethernet/intel/ice/ice_base.c | 63 ---
> drivers/net/ethernet/intel/ice/ice_lib.c  | 94 ---
> drivers/net/ethernet/intel/ice/ice_main.c |  5 +-
>  5 files changed, 143 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_base.h
> b/drivers/net/ethernet/intel/ice/ice_base.h
> index d28294247599..99b2c7232829 100644
> --- a/drivers/net/ethernet/intel/ice/ice_base.h
> +++ b/drivers/net/ethernet/intel/ice/ice_base.h
> @@ -12,8 +12,10 @@ int __ice_vsi_get_qs(struct ice_qs_cfg *qs_cfg);
> int  ice_vsi_ctrl_one_rx_ring(struct ice_vsi *vsi, bool ena, u16
> rxq_idx, bool wait);  int ice_vsi_wait_one_rx_ring(struct ice_vsi
> *vsi, bool ena, u16 rxq_idx);
> +int ice_vsi_alloc_q_vectors_locked(struct ice_vsi *vsi);
>  int ice_vsi_alloc_q_vectors(struct ice_vsi *vsi);  void
> ice_vsi_map_rings_to_vectors(struct ice_vsi *vsi);
> +void ice_vsi_free_q_vectors_locked(struct ice_vsi *vsi);
>  void ice_vsi_free_q_vectors(struct ice_vsi *vsi);  int
> ice_vsi_cfg_single_txq(struct ice_vsi *vsi, struct ice_tx_ring
> **tx_rings,
>  u16 q_idx);
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h
> b/drivers/net/ethernet/intel/ice/ice_lib.h
> index 347b63e497e7..e55b72db72c4 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.h
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.h
> @@ -68,6 +68,7 @@ int ice_ena_vsi(struct ice_vsi *vsi, bool locked);
> void ice_vsi_decfg(struct ice_vsi *vsi);  void ice_dis_vsi(struct
> ice_vsi *vsi, bool locked);
> 
> +int ice_vsi_rebuild_locked(struct ice_vsi *vsi, u32 vsi_flags);
>  int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags);  int
> ice_vsi_cfg(struct ice_vsi *vsi);  struct ice_vsi
> *ice_vsi_alloc(struct ice_pf *pf); diff --git
> a/drivers/net/ethernet/intel/ice/ice_base.c
> b/drivers/net/ethernet/intel/ice/ice_base.c
> index 7097324c38f3..65e19815bec5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_base.c
> +++ b/drivers/net/ethernet/intel/ice/ice_base.c
> @@ -153,8 +153,8 @@ static int ice_vsi_alloc_q_vector(struct ice_vsi
> *vsi, u16 v_idx)
>* handler here (i.e. resume, reset/rebuild, etc.)
>*/
>   if (vsi->netdev)
> - netif_napi_add_config(vsi->netdev, &q_vector->napi,
> -   ice_napi_poll, v_idx);
> + netif_napi_add_config_locked(vsi->netdev, &q_vector-
> >napi,
> +  ice_napi_poll, v_idx);
If you converted ice_vsi_alloc_q_vector() into _locked, should it be 
lockdep_assert_held(&vsi->netdev->lock); then?

Everything else looks fine.
Reviewed-by: Aleksandr Loktionov 


...
 
> --
> 2.52.0



[Intel-wired-lan] [PATCH iwl-next] ice: fix system hang on `ethtool -L`

2026-01-20 Thread Alexander Lobakin
ice_set_channels() calls ice_vsi_rebuild() under the netdev lock
taken, but ice_vsi_rebuild() calls netif_napi_{add,del}() which
take the same lock.
Add ice_vsi_rebuild_locked() which uses the _locked counterparts
of these functions and use it in ice_set_channels().

Signed-off-by: Alexander Lobakin 
---
Hey Tony, please amend to the patch I replied to.
---
 drivers/net/ethernet/intel/ice/ice_base.h |  2 +
 drivers/net/ethernet/intel/ice/ice_lib.h  |  1 +
 drivers/net/ethernet/intel/ice/ice_base.c | 63 ---
 drivers/net/ethernet/intel/ice/ice_lib.c  | 94 ---
 drivers/net/ethernet/intel/ice/ice_main.c |  5 +-
 5 files changed, 143 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_base.h 
b/drivers/net/ethernet/intel/ice/ice_base.h
index d28294247599..99b2c7232829 100644
--- a/drivers/net/ethernet/intel/ice/ice_base.h
+++ b/drivers/net/ethernet/intel/ice/ice_base.h
@@ -12,8 +12,10 @@ int __ice_vsi_get_qs(struct ice_qs_cfg *qs_cfg);
 int
 ice_vsi_ctrl_one_rx_ring(struct ice_vsi *vsi, bool ena, u16 rxq_idx, bool 
wait);
 int ice_vsi_wait_one_rx_ring(struct ice_vsi *vsi, bool ena, u16 rxq_idx);
+int ice_vsi_alloc_q_vectors_locked(struct ice_vsi *vsi);
 int ice_vsi_alloc_q_vectors(struct ice_vsi *vsi);
 void ice_vsi_map_rings_to_vectors(struct ice_vsi *vsi);
+void ice_vsi_free_q_vectors_locked(struct ice_vsi *vsi);
 void ice_vsi_free_q_vectors(struct ice_vsi *vsi);
 int ice_vsi_cfg_single_txq(struct ice_vsi *vsi, struct ice_tx_ring **tx_rings,
   u16 q_idx);
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h 
b/drivers/net/ethernet/intel/ice/ice_lib.h
index 347b63e497e7..e55b72db72c4 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -68,6 +68,7 @@ int ice_ena_vsi(struct ice_vsi *vsi, bool locked);
 void ice_vsi_decfg(struct ice_vsi *vsi);
 void ice_dis_vsi(struct ice_vsi *vsi, bool locked);
 
+int ice_vsi_rebuild_locked(struct ice_vsi *vsi, u32 vsi_flags);
 int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags);
 int ice_vsi_cfg(struct ice_vsi *vsi);
 struct ice_vsi *ice_vsi_alloc(struct ice_pf *pf);
diff --git a/drivers/net/ethernet/intel/ice/ice_base.c 
b/drivers/net/ethernet/intel/ice/ice_base.c
index 7097324c38f3..65e19815bec5 100644
--- a/drivers/net/ethernet/intel/ice/ice_base.c
+++ b/drivers/net/ethernet/intel/ice/ice_base.c
@@ -153,8 +153,8 @@ static int ice_vsi_alloc_q_vector(struct ice_vsi *vsi, u16 
v_idx)
 * handler here (i.e. resume, reset/rebuild, etc.)
 */
if (vsi->netdev)
-   netif_napi_add_config(vsi->netdev, &q_vector->napi,
- ice_napi_poll, v_idx);
+   netif_napi_add_config_locked(vsi->netdev, &q_vector->napi,
+ice_napi_poll, v_idx);
 
 out:
/* tie q_vector and VSI together */
@@ -196,7 +196,7 @@ static void ice_free_q_vector(struct ice_vsi *vsi, int 
v_idx)
 
/* only VSI with an associated netdev is set up with NAPI */
if (vsi->netdev)
-   netif_napi_del(&q_vector->napi);
+   netif_napi_del_locked(&q_vector->napi);
 
/* release MSIX interrupt if q_vector had interrupt allocated */
if (q_vector->irq.index < 0)
@@ -896,13 +896,15 @@ int ice_vsi_wait_one_rx_ring(struct ice_vsi *vsi, bool 
ena, u16 rxq_idx)
 }
 
 /**
- * ice_vsi_alloc_q_vectors - Allocate memory for interrupt vectors
+ * ice_vsi_alloc_q_vectors_locked - Allocate memory for interrupt vectors
  * @vsi: the VSI being configured
  *
- * We allocate one q_vector per queue interrupt. If allocation fails we
- * return -ENOMEM.
+ * Should be called only under the netdev lock.
+ * We allocate one q_vector per queue interrupt.
+ *
+ * Return: 0 on success, -ENOMEM if allocation fails.
  */
-int ice_vsi_alloc_q_vectors(struct ice_vsi *vsi)
+int ice_vsi_alloc_q_vectors_locked(struct ice_vsi *vsi)
 {
struct device *dev = ice_pf_to_dev(vsi->back);
u16 v_idx;
@@ -929,6 +931,30 @@ int ice_vsi_alloc_q_vectors(struct ice_vsi *vsi)
return v_idx ? 0 : err;
 }
 
+/**
+ * ice_vsi_alloc_q_vectors - Allocate memory for interrupt vectors
+ * @vsi: the VSI being configured
+ *
+ * We allocate one q_vector per queue interrupt.
+ *
+ * Return: 0 on success, -ENOMEM if allocation fails.
+ */
+int ice_vsi_alloc_q_vectors(struct ice_vsi *vsi)
+{
+   struct net_device *dev = vsi->netdev;
+   int ret;
+
+   if (dev)
+   netdev_lock(dev);
+
+   ret = ice_vsi_alloc_q_vectors_locked(vsi);
+
+   if (dev)
+   netdev_unlock(dev);
+
+   return ret;
+}
+
 /**
  * ice_vsi_map_rings_to_vectors - Map VSI rings to interrupt vectors
  * @vsi: the VSI being configured
@@ -992,10 +1018,12 @@ void ice_vsi_map_rings_to_vectors(struct ice_vsi *vsi)
 }
 
 /**
- * ice_vsi_free_q_vectors - Free memory allocated for interrupt vectors
+ * ice_vsi_free_q_vectors_locked - Free memory all