On Sun, May 12, 2024 at 12:53:06PM +0200, Oleg Nesterov wrote: > On 05/10, Paul E. McKenney wrote: > > > > On Fri, May 10, 2024 at 01:31:49PM +0200, Oleg Nesterov wrote: > > > > > Why is that? > > > > Because I run KCSAN on RCU using Kconfig options that cause KCSAN > > to be more strict. > > Yes, I see now. > > > > but how can KCSAN detect that all accesses to X are properly marked? I see nothing > > > KCSAN-related in the definition of WRITE_ONCE() or READ_ONCE(). > > > > The trick is that KCSAN sees the volatile cast that both READ_ONCE() > > and WRITE_ONCE() use. > > Hmm. grep-grep-grep... I seem to understand, DEFINE_TSAN_VOLATILE_READ_WRITE. > So __tsan_volatile_readX() will use KCSAN_ACCESS_ATOMIC. > > Thanks! You got it!!! > > > And what does the "all accesses" above actually mean? The 2nd version does > > > > > > WRITE_ONCE(X, X+1); > > > > > > but "X + 1" is the plain/unmarked access? > > > > ... > > > > In that case, the "X+1" cannot be involved in a data race, so KCSAN > > won't complain. > > Yes, yes, I understand now. > > Paul, thanks for your explanations! and sorry for wasting your time by > provoking the unnecessarily long discussion. Not a problem and absolutely no need to apologize! Of course, please do pass this information on to anyone else needing it. > I am going to send the trivial patch which moves these WARN_ON()'s under > spin_lock(), this looks more clean to me. But I won't argue if you prefer > your original patch. Actually, I like your patch quite a bit better than I do my original. In fact, I feel quite foolish that I did not think of this to begin with. With your way, we have strict locking for that field and can therefore just use plain C-language accesses for all accesses to it. KCSAN will then warn us of any buggy lockless access to that field, even if that buggy access uses READ_ONCE(). Much much better your way!!! So thank you for persisting on this! Thanx, Paul
