-----Original Message-----
From: Suman Ghosh <redacted>
Sent: Tuesday, December 12, 2023 11:48 PM
To: Souradeep Chakrabarti <redacted>; KY Srinivasan
[off-list ref]; Haiyang Zhang [off-list ref];
wei.liu@kernel.org; Dexuan Cui [off-list ref]; davem@davemloft.net;
edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Long Li
[off-list ref]; yury.norov@gmail.com; leon@kernel.org;
cai.huoqing@linux.dev; ssengar@linux.microsoft.com; vkuznets@redhat.com;
tglx@linutronix.de; linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-
kernel@vger.kernel.org; linux-rdma@vger.kernel.org
Cc: Souradeep Chakrabarti <redacted>; Paul Rosswurm
[off-list ref]
Subject: [EXTERNAL] RE: [EXT] [PATCH V5 net-next] net: mana: Assigning IRQ
affinity on HT cores

[Some people who received this message don't often get email from
sumang@marvell.com. Learn why this is important at
https://aka.ms/LearnAboutSenderIdentification ]

Hi Souradeep,

Please find inline for couple of comments.

quoted

+
+      if (!zalloc_cpumask_var(&curr, GFP_KERNEL)) {
+              err = -ENOMEM;
+              return err;
+      }
+      if (!zalloc_cpumask_var(&cpus, GFP_KERNEL)) {

[Suman] memory leak here, should free 'curr'.

This will be taken care in next version.

quoted

+              err = -ENOMEM;
+              return err;
+      }
+
+      rcu_read_lock();
+      for_each_numa_hop_mask(next, next_node) {
+              cpumask_andnot(curr, next, prev);
+              for (w = cpumask_weight(curr), cnt = 0; cnt < w; ) {
+                      cpumask_copy(cpus, curr);
+                      for_each_cpu(cpu, cpus) {
+                              irq_set_affinity_and_hint(irqs[i],
topology_sibling_cpumask(cpu));
+                              if (++i == nvec)
+                                      goto done;
+                              cpumask_andnot(cpus, cpus,
topology_sibling_cpumask(cpu));
+                              ++cnt;
+                      }
+              }
+              prev = next;
+      }
+done:
+      rcu_read_unlock();
+      free_cpumask_var(curr);
+      free_cpumask_var(cpus);
+      return err;
+}
+
static int mana_gd_setup_irqs(struct pci_dev *pdev)  {
-      unsigned int max_queues_per_port = num_online_cpus();
      struct gdma_context *gc = pci_get_drvdata(pdev);
+      unsigned int max_queues_per_port;
      struct gdma_irq_context *gic;
      unsigned int max_irqs, cpu;
-      int nvec, irq;
+      int start_irq_index = 1;
+      int nvec, *irqs, irq;
      int err, i = 0, j;

+      cpus_read_lock();
+      max_queues_per_port = num_online_cpus();
      if (max_queues_per_port > MANA_MAX_NUM_QUEUES)
              max_queues_per_port = MANA_MAX_NUM_QUEUES;

@@ -1261,6 +1302,14 @@ static int mana_gd_setup_irqs(struct pci_dev

*pdev)
      nvec = pci_alloc_irq_vectors(pdev, 2, max_irqs, PCI_IRQ_MSIX);
      if (nvec < 0)

[Suman] cpus_read_unlock()?

Thanks for pointing, it will be taken care off in the V6.

quoted

              return nvec;
+      if (nvec <= num_online_cpus())
+              start_irq_index = 0;
+
+      irqs = kmalloc_array((nvec - start_irq_index), sizeof(int),
GFP_KERNEL);
+      if (!irqs) {
+              err = -ENOMEM;
+              goto free_irq_vector;
+      }

      gc->irq_contexts = kcalloc(nvec, sizeof(struct gdma_irq_context),
                                 GFP_KERNEL); @@ -1287,21 +1336,44 @@
static int mana_gd_setup_irqs(struct pci_dev
*pdev)
                      goto free_irq;
              }

-              err = request_irq(irq, mana_gd_intr, 0, gic->name, gic);
-              if (err)
-                      goto free_irq;
-
-              cpu = cpumask_local_spread(i, gc->numa_node);
-              irq_set_affinity_and_hint(irq, cpumask_of(cpu));
+              if (!i) {
+                      err = request_irq(irq, mana_gd_intr, 0, gic->name, gic);
+                      if (err)
+                              goto free_irq;
+
+                      /* If number of IRQ is one extra than number of
+ online
CPUs,
+                       * then we need to assign IRQ0 (hwc irq) and IRQ1 to
+                       * same CPU.
+                       * Else we will use different CPUs for IRQ0 and IRQ1.
+                       * Also we are using cpumask_local_spread instead of
+                       * cpumask_first for the node, because the node can be
+                       * mem only.
+                       */
+                      if (start_irq_index) {
+                              cpu = cpumask_local_spread(i, gc->numa_node);
+                              irq_set_affinity_and_hint(irq, cpumask_of(cpu));
+                      } else {
+                              irqs[start_irq_index] = irq;
+                      }
+              } else {
+                      irqs[i - start_irq_index] = irq;
+                      err = request_irq(irqs[i - start_irq_index],
mana_gd_intr, 0,
+                                        gic->name, gic);
+                      if (err)
+                              goto free_irq;
+              }
      }

+      err = irq_setup(irqs, (nvec - start_irq_index), gc->numa_node);
+      if (err)
+              goto free_irq;
      err = mana_gd_alloc_res_map(nvec, &gc->msix_resource);
      if (err)
              goto free_irq;

      gc->max_num_msix = nvec;
      gc->num_msix_usable = nvec;
-
+      cpus_read_unlock();
      return 0;

free_irq:

@@ -1314,8 +1386,10 @@ static int mana_gd_setup_irqs(struct pci_dev

*pdev)
      }

      kfree(gc->irq_contexts);
+      kfree(irqs);
      gc->irq_contexts = NULL;
free_irq_vector:
+      cpus_read_unlock();
      pci_free_irq_vectors(pdev);
      return err;
}
--
2.34.1