Marco Elver wrote on Mon, Dec 05, 2022 at 08:00:00AM +0100:
Should I just update the wrapped condition, as below?
err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD);
err = wait_event_killable(req->wq,READ_ONCE(req->status) >= REQ_STATUS_RCVD);Yes, this looks good!
The writes all are straightforward, there's all the error paths to convert to WRITE_ONCE too but that's not difficult (leaving only the init without such a marker); I'll send a patch when you've confirmed the read looks good. (the other reads are a bit less obvious as some are protected by a lock in trans_fd, which should cover all cases of possible concurrent updates there as far as I can see, but this mixed model is definitely hard to reason with... Well, that's how it was written and I won't ever have time to rewrite any of this. Enough ranting.)
If the lock-protected accesses indeed are non-racy, they should be left unmarked. If some assumption here turns out to be wrong, KCSAN would (hopefully) tell us one way or another.
Great, that makes sense.
I've left the commit at home, will submit it tonight -- you and Naresh will be in Cc from suggested/reported-by tags.