Messages in this thread | ![/](/images/icornerl.gif) | | From | Souradeep Chakrabarti <> | Subject | RE: [EXT] [PATCH V5 net-next] net: mana: Assigning IRQ affinity on HT cores | Date | Tue, 12 Dec 2023 18:22:20 +0000 |
| |
>-----Original Message----- >From: Suman Ghosh <sumang@marvell.com> >Sent: Tuesday, December 12, 2023 11:48 PM >To: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>; KY Srinivasan ><kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; >wei.liu@kernel.org; Dexuan Cui <decui@microsoft.com>; davem@davemloft.net; >edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Long Li ><longli@microsoft.com>; 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 <schakrabarti@microsoft.com>; Paul Rosswurm ><paulros@microsoft.com> >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. > >>+ >>+ 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. >>+ 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. >> 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 >>
| ![\](/images/icornerr.gif) |