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. -- Dominique
