Re: [Intel-wired-lan] [PATCH net-next v8 3/6] net: napi: add CPU affinity to napi_config

[Ahmed Zaki]

On 2025-02-13 5:26 a.m., Paolo Abeni wrote:
On 2/11/25 10:06 PM, Ahmed Zaki wrote:
@@ -394,10 +395,8 @@ struct napi_struct {
        struct list_head        dev_list;
        struct hlist_node       napi_hash_node;
        int                     irq;
-#ifdef CONFIG_RFS_ACCEL
        struct irq_affinity_notify notify;
        int                     napi_rmap_idx;
-#endif

I'm sorry for the late doubt, but it's not clear to me why you need to
add the #ifdef in the previous patch ?!?

It was there to make the code consistent, since the rmap and the 
notifier were only needed for ARFS.

It can be removed, although I am not sure if there would be any warnings 
since on !CONFIG_ARFS_ACCEL the fields would never be used.


diff --git a/net/core/dev.c b/net/core/dev.c
index 209296cef3cd..d2c942bbd5e6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6871,28 +6871,39 @@ void netif_queue_set_napi(struct net_device *dev, 
unsigned int queue_index,
  }
  EXPORT_SYMBOL(netif_queue_set_napi);
  
-#ifdef CONFIG_RFS_ACCEL
  static void
-netif_irq_cpu_rmap_notify(struct irq_affinity_notify *notify,
-                         const cpumask_t *mask)
+netif_napi_irq_notify(struct irq_affinity_notify *notify,
+                     const cpumask_t *mask)
  {
        struct napi_struct *napi =
                container_of(notify, struct napi_struct, notify);
+#ifdef CONFIG_RFS_ACCEL
        struct cpu_rmap *rmap = napi->dev->rx_cpu_rmap;
        int err;
+#endif
  
-	err = cpu_rmap_update(rmap, napi->napi_rmap_idx, mask);
-       if (err)
-               netdev_warn(napi->dev, "RMAP update failed (%d)\n",
-                           err);
+       if (napi->config && napi->dev->irq_affinity_auto)
+               cpumask_copy(&napi->config->affinity_mask, mask);
+
+#ifdef CONFIG_RFS_ACCEL
+       if (napi->dev->rx_cpu_rmap_auto) {
+               err = cpu_rmap_update(rmap, napi->napi_rmap_idx, mask);
+               if (err)
+                       netdev_warn(napi->dev, "RMAP update failed (%d)\n",
+                                   err);
+       }
+#endif

Minor nit: if you provide a netif_rx_cpu_rmap() helper returning
dev->rx_cpu_rmap or NULL for !CONFIG_RFS_ACCEL build, you can avoid the
above 2 ifdefs and possibly more below.


Thanks, I will add this if there is a new version.


@@ -6915,7 +6926,6 @@ static int napi_irq_cpu_rmap_add(struct napi_struct 
*napi, int irq)
        if (rc)
                goto err_set;
  
-	set_bit(NAPI_STATE_HAS_NOTIFIER, &napi->state);

Minor nit: I think it would be better if the previous patch would add
directly this line in netif_napi_set_irq_locked() (avoding the removal
here).


yes, it just made more sense for that patch.