> -----Original Message-----
> From: Michal Schmidt <mschm...@redhat.com>
> Sent: Thursday, May 23, 2024 4:43 AM
> To: Jesse Brandeburg <jesse.brandeb...@intel.com>; Tony Nguyen
> <anthony.l.ngu...@intel.com>; intel-wired-...@lists.osuosl.org
> Cc: net...@vger.kernel.org; Nitesh Narayan Lal <nit...@redhat.com>;
> Thomas Gleixner <t...@linutronix.de>
> Subject: [EXTERNAL] [PATCH iwl-next] ice: use irq_update_affinity_hint()
> 
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
> 
> ----------------------------------------------------------------------
> irq_set_affinity_hint() is deprecated. Use irq_update_affinity_hint() instead.
> This removes the side-effect of actually applying the affinity.
> 
> The driver does not really need to worry about spreading its IRQs across CPUs.
> The core code already takes care of that.
> On the contrary, when the driver applies affinities by itself, it breaks the 
> users'
> expectations:
>  1. The user configures irqbalance with IRQBALANCE_BANNED_CPULIST in
>     order to prevent IRQs from being moved to certain CPUs that run a
>     real-time workload.
>  2. ice reconfigures VSIs at runtime due to a MIB change
>     (ice_dcb_process_lldp_set_mib_change). Reopening a VSI resets the
>     affinity in ice_vsi_req_irq_msix().
>  3. ice has no idea about irqbalance's config, so it may move an IRQ to
>     a banned CPU. The real-time workload suffers unacceptable latency.
> 
> I am not sure if updating the affinity hints is at all useful, because 
> irqbalance
> ignores them since 2016 ([1]), but at least it's harmless.
> 
> This ice change is similar to i40e commit d34c54d1739c ("i40e: Use
> irq_update_affinity_hint()").
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__github.com_Irqbalance_irqbalance_commit_dcc411e7bfdd&d=DwIDAg&
> c=nKjWec2b6R0mOyPaz7xtfQ&r=q3VKxXQKiboRw_F01ggTzHuhwawxR1P9_t
> MCN2FODU4&m=8_ejYsgRhUX90iDcRMQE2cUmtBDDCjGE8LVB-
> vhUlW0MMr-ZoqFeyCnI5qTQrKxf&s=r5rBcGd0wa-
> lQkDZdyzZwBeBviSd8DZpgYh028enkU8&e=
> 
> Signed-off-by: Michal Schmidt <mschm...@redhat.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_lib.c  | 4 ++--
> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 5371e91f6bbb..0f8b622db2b5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -2587,8 +2587,8 @@ void ice_vsi_free_irq(struct ice_vsi *vsi)
>               if (!IS_ENABLED(CONFIG_RFS_ACCEL))
>                       irq_set_affinity_notifier(irq_num, NULL);
> 
> -             /* clear the affinity_mask in the IRQ descriptor */
> -             irq_set_affinity_hint(irq_num, NULL);
> +             /* clear the affinity_hint in the IRQ descriptor */
> +             irq_update_affinity_hint(irq_num, NULL);
>               synchronize_irq(irq_num);
>               devm_free_irq(ice_pf_to_dev(pf), irq_num, vsi-
> >q_vectors[i]);
>       }
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index f60c022f7960..a5d369b8fed5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -2607,7 +2607,7 @@ static int ice_vsi_req_irq_msix(struct ice_vsi *vsi,
> char *basename)
>               }
> 
>               /* assign the mask for this irq */
> -             irq_set_affinity_hint(irq_num, &q_vector->affinity_mask);
> +             irq_update_affinity_hint(irq_num, &q_vector->affinity_mask);
>       }
> 
>       err = ice_set_cpu_rx_rmap(vsi);
> @@ -2625,7 +2625,7 @@ static int ice_vsi_req_irq_msix(struct ice_vsi *vsi,
> char *basename)
>               irq_num = vsi->q_vectors[vector]->irq.virq;
>               if (!IS_ENABLED(CONFIG_RFS_ACCEL))
>                       irq_set_affinity_notifier(irq_num, NULL);
> -             irq_set_affinity_hint(irq_num, NULL);
> +             irq_update_affinity_hint(irq_num, NULL);
>               devm_free_irq(dev, irq_num, &vsi->q_vectors[vector]);
>       }
>       return err;
> --
> 2.44.0
> 

LGTM
Reviewed-by: Sunil Goutham <sgout...@marvell.com>

Reply via email to