Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] cleanup: mixing goto-based error handling with guard() --
commit 24bec1792936c12968d8086392604ffa71c7cce1 Author: David Zhang <[email protected]> accel/amdxdna: Decouple AIE4 doorbell and MSI-X notify transport hooks This commit separates PCI-specific doorbell and interrupt notification handling from the transport-neutral context code. > diff --git a/drivers/accel/amdxdna/aie4_ctx.c > b/drivers/accel/amdxdna/aie4_ctx.c > index 5eb918e1d58c7..5a2fc19bad206 100644 > --- a/drivers/accel/amdxdna/aie4_ctx.c > +++ b/drivers/accel/amdxdna/aie4_ctx.c [ ... ] > @@ -51,32 +42,27 @@ static struct cert_comp *aie4_lookup_cert_comp(struct > amdxdna_dev_hdl *ndev, u32 [ ... ] > - ret = request_irq(cert_comp->irq, cert_comp_isr, 0, "xdna_hsa", > cert_comp); > + /* Transport-specific: PCI wires an MSI-X irq, platform an IPI > callback. */ > + ret = aie4_request_notification(cert_comp); > if (ret) { > - XDNA_ERR(xdna, "request irq %d failed %d", cert_comp->irq, ret); > + XDNA_ERR(xdna, "request notification for msix idx %u failed > %d", msix_idx, ret); > goto free_cert_comp; > } > > ret = xa_err(xa_store(&ndev->cert_comp_xa, msix_idx, cert_comp, > GFP_KERNEL)); > if (ret) { > - XDNA_ERR(xdna, "store cert_comp for msix idx %d failed %d", > msix_idx, ret); > + XDNA_ERR(xdna, "store cert_comp for msix idx %u failed %d", > msix_idx, ret); > goto free_irq; > } > > return cert_comp; > > free_irq: > - free_irq(cert_comp->irq, cert_comp); > + aie4_free_notification(cert_comp); > free_cert_comp: > kfree(cert_comp); > return NULL; [Severity: Low] Is it safe to continue using goto-based error handling here? Looking earlier in aie4_lookup_cert_comp(), a scope-based lock is declared: guard(mutex)(&ndev->cert_comp_lock); The kernel cleanup subsystem guidelines recommend that goto statements and scope-based cleanup helpers like guard() should never be mixed in the same function. Mixing them creates confusing ownership semantics and increases the risk of double-free bugs or resource leaks during future modifications. Could we either convert all resources in aie4_lookup_cert_comp() to scope-based cleanup, or avoid using guard() in this function entirely? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
