Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] cleanup: mixing guard(mutex) with goto in aie4_lookup_cert_comp() --
commit 3a88611fdb628b3897caa0f38eb9cc37baf71f5b Author: David Zhang <[email protected]> accel/amdxdna: Implement AIE4 kernel queue lifecycle and memory layout This commit initializes the kernel-mode submission required buffers, workqueue, and hardware contexts for AIE4. It updates the queue definition, adds a job workqueue, configures direct and indirect packets, and establishes mutex protection. > diff --git a/drivers/accel/amdxdna/aie4_ctx.c > b/drivers/accel/amdxdna/aie4_ctx.c > index 5a2fc19bad206..226570367f71f 100644 > --- a/drivers/accel/amdxdna/aie4_ctx.c > +++ b/drivers/accel/amdxdna/aie4_ctx.c [ ... ] > @@ -65,7 +72,7 @@ static struct cert_comp *aie4_lookup_cert_comp(struct > amdxdna_dev_hdl *ndev, u32 > aie4_free_notification(cert_comp); > free_cert_comp: > kfree(cert_comp); > - return NULL; > + return ERR_PTR(ret); [Severity: Low] Does this code mix goto-based error handling and scope-based cleanups in the same function? The kernel cleanup subsystem documentation in include/linux/cleanup.h strictly dictates that goto-based error handling and scope-based cleanups (like guard() and __free()) must never be mixed in the same function. Looking at aie4_lookup_cert_comp(): drivers/accel/amdxdna/aie4_ctx.c:aie4_lookup_cert_comp() { ... guard(mutex)(&ndev->cert_comp_lock); ... ret = aie4_request_notification(cert_comp); if (ret) { ... goto free_cert_comp; } ... } Mixing these creates confusing ownership semantics and increases the risk of double-frees or unlocking errors during refactoring. Could we convert this function entirely to scope-based cleanup or convert none of it? > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=12
