Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing error check for xa_store() leads to resource leak and potential kernel oops on allocation failure. -- commit 853688f50dc4ca56dd74698a3ecfdfeaf857e716 Author: Shradha Gupta <[email protected]> net: mana: Optimize irq affinity for low vcpu configs This patch optimizes irq affinity in the mana driver for low vCPU environments. When the number of allocated irqs is greater than the vCPU count, it ensures that queue irqs are spread across all available vCPUs to prevent softirq handling imbalances. > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c > b/drivers/net/ethernet/microsoft/mana/gdma_main.c > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c [ ... ] > @@ -1767,13 +1784,42 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev > *pdev, int nvec) > * first CPU sibling group since they are already affinitized to HWC IRQ > */ > cpus_read_lock(); > - if (gc->num_msix_usable <= num_online_cpus()) > - skip_first_cpu = true; > + if (gc->num_msix_usable <= num_online_cpus()) { > + err = irq_setup(irqs, nvec, gc->numa_node, true); > + if (err) { > + cpus_read_unlock(); > + goto free_irq; [Severity: High] This isn't a bug introduced by this patch, but looking at the error handling path for goto free_irq, is there a potential resource leak if xa_store() fails earlier in the function? When setting up the irqs in mana_gd_setup_dyn_irqs(), we call xa_store() with GFP_KERNEL: xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL); However, the return value is not checked. If this allocation fails, the gic pointer is not saved. If a subsequent operation like irq_setup() fails and we jump to the free_irq label, the cleanup loop attempts to retrieve it: gic = xa_load(&gc->irq_contexts, i); if (WARN_ON(!gic)) continue; Because the store failed, xa_load() will return NULL, causing the loop to continue without calling free_irq() or kfree(gic). Could this result in an un-freed irq handler that might trigger a panic later if the module is unloaded and the hardware raises an interrupt? > + } > + } else { [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
