Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Mixing `goto` and `guard()` causes undefined behavior/build error --
commit e82723d9f7a656583eeaed6e8880da6e23ecb6df Author: Lizhi Hou <[email protected]> accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo() This commit reorders amdxdna_pm_resume_get() to execute before acquiring xdna->dev_lock. This prevents a potential deadlock that occurs if the device is suspended during the callback. > diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c > b/drivers/accel/amdxdna/amdxdna_ctx.c > index 855da8c79a1cd..20e2fd096c913 100644 > --- a/drivers/accel/amdxdna/amdxdna_ctx.c > +++ b/drivers/accel/amdxdna/amdxdna_ctx.c > @@ -412,16 +412,25 @@ int amdxdna_hwctx_sync_debug_bo(struct amdxdna_client > *client, u32 debug_bo_hdl) > if (!gobj) > return -EINVAL; > > + ret = amdxdna_pm_resume_get(xdna); > + if (ret) { > + XDNA_ERR(xdna, "Resume failed, ret %d", ret); > + goto put_obj; > + } > + > abo = to_xdna_obj(gobj); > guard(mutex)(&xdna->dev_lock); [Severity: High] Does jumping forward over the guard(mutex) declaration with the goto put_obj statement above trigger a compilation error? Jumping over a variable initialized with __attribute__((cleanup)) is usually rejected by modern compilers. If a compiler allowed it, the destructor would execute at the end of the function using uninitialized stack memory for the lock pointer, causing a kernel panic. Additionally, include/linux/cleanup.h notes that mixing goto-based error handling and scope-based cleanups in the same function should be avoided to prevent issues like this. > hwctx = xa_load(&client->hwctx_xa, abo->assigned_hwctx); > if (!hwctx) { > ret = -EINVAL; > - goto put_obj; > + goto suspend_put; > } > > ret = xdna->dev_info->ops->hwctx_sync_debug_bo(hwctx, debug_bo_hdl); > > +suspend_put: > + amdxdna_pm_suspend_put(xdna); > + > put_obj: > drm_gem_object_put(gobj); > return ret; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
