Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] misc/fastrpc: missing memory barrier and data race on ctx->is_work_done allows stale error codes --
commit ec0bf5065c708ee8356d5c1feae24849edc520bd Author: Jianping Li <[email protected]> misc: fastrpc: wake poll-mode waiters on SSR This commit updates fastrpc_notify_users() to set ctx->is_work_done along with ctx->retval. This ensures that threads spinning in polling mode will observe the termination and exit, instead of waiting for a timeout when the DSP goes down. > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index 05b2e7e4ad3b7..c54c450cb5714 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -2663,6 +2663,7 @@ static void fastrpc_notify_users(struct fastrpc_user > *user) > spin_lock(&user->lock); > list_for_each_entry(ctx, &user->pending, node) { > ctx->retval = -EPIPE; > + ctx->is_work_done = true; [Severity: Medium] Is there a missing write memory barrier here? If the stores to ctx->retval and ctx->is_work_done are reordered by the compiler or the CPU, a concurrent thread executing poll_for_remote_response() might read ctx->is_work_done as true but read a stale value for ctx->retval, such as the initialization value -1 or -EPERM. In poll_for_remote_response(), a concurrent thread evaluates this locklessly: ... (val == FASTRPC_POLL_RESPONSE) || ctx->is_work_done Even if the polling thread uses dma_rmb() before reading ctx->retval, that only ensures read-read hardware ordering. It does not prevent this writer thread from reordering the stores. Would it be safer to use smp_store_release() for ctx->is_work_done here and smp_load_acquire() in the polling loop? Additionally, does this introduce a plain C data race? poll_for_remote_response() appears to locklessly write to ctx->is_work_done on DSP polling completion. This could cause undefined behavior when racing with the lockless reads and this new write in fastrpc_notify_users(). > complete(&ctx->work); > } > spin_unlock(&user->lock); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
