Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] amdxdna: monotonic regression in get_read_index() due to lockless update --
commit 556cf69a63f5758be4c65cd88bedd02f3117e2ca Author: David Zhang <[email protected]> accel/amdxdna: Prepare for AIE4 command submission This commit prepares data structures and completion wait helpers for AIE4 command submission. It extends the amdxdna_job_priv structure and implements an ordered retry mechanism in get_read_index() when a torn read is detected. > diff --git a/drivers/accel/amdxdna/aie4_ctx.c > b/drivers/accel/amdxdna/aie4_ctx.c > --- a/drivers/accel/amdxdna/aie4_ctx.c > +++ b/drivers/accel/amdxdna/aie4_ctx.c [ ... ] > static u64 get_read_index(struct amdxdna_hwctx *hwctx) > { [ ... ] > + /* > + * Cache valid sample as fallback for torn reads. Lockless on purpose: > + * a racing caller may store an older value, which only delays seeing > + * a completion until the next wakeup, never reports one early. > + */ > + WRITE_ONCE(priv->last_read_index, ri); [Severity: Medium] Can this lockless assignment allow concurrent get_read_index() callers to overwrite a newer completion index with an older one? For example, if Thread A reads ri=10, and Thread B reads ri=11 and writes last_read_index=11, could Thread A then execute this assignment and write last_read_index=10? This appears to cause last_read_index to go backward. > return ri; > } [ ... ] > int aie4_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq, u32 timeout) > { [ ... ] > - if (!ret) > + if (!ret) { > ret = -ETIME; > + } else if (ret > 0 && !check_cert_comp_linked(hwctx, cert_comp) && > + get_read_index(hwctx) <= seq) { [Severity: Medium] If last_read_index went backward as described above, could this check falsely evaluate to true? If a torn read falls back to the stale last_read_index while the device disconnects or suspends, it seems a successfully completed command might incorrectly fail and return -EAGAIN. > + /* Disconnect (suspend or TDR) before the command completed. */ > + ret = -EAGAIN; > + } > > aie4_put_cert_comp(cert_comp); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
