Re: [PATCH 25/48] rcu: Mark writes to rcu_sync ->gp_count field

[Alan Huang]

> 2024年5月10日 19:31，Oleg Nesterov <oleg@xxxxxxxxxx> wrote：
> 
> On 05/09, Paul E. McKenney wrote:
>> 
>> On Thu, May 09, 2024 at 05:13:12PM +0200, Oleg Nesterov wrote:
>>> 
>>> We can move these WARN_ON()'s into the ->rss_lock protected section.
>>> 
>>> Or perhaps we can use data_race(rsp->gp_count) ? To be honest I thought
>>> that READ_ONCE() should be enough...
>>> 
>>> Or we can simply kill these WARN_ON_ONCE()'s.
>> 
>> Or we could move those WARN_ON_ONCE() under the lock.
> 
> Sure, see above.
> 
> But could you help me to understand this magic? I naively thought
> that READ_ONCE() is always "safe"...
> 
> So, unless I am totally confused it turns out that, say,
> 
> CPU 0 CPU 1
> ----- -----
> 
> spin_lock(LOCK);
> ++X; READ_ONCE(X); // data race
> spin_unlock(LOCK);
> 
> is data-racy accoring to KCSAN, while
> 
> CPU 0 CPU 1
> ----- -----
> 
> spin_lock(LOCK);
> WRITE_ONCE(X, X+1); READ_ONCE(X); // no data race
> spin_unlock(LOCK);
> 
> is not.
> 
> Why is that?
> 
> Trying to read Documentation/dev-tools/kcsan.rst... it says
> 
> KCSAN is aware of *marked atomic operations* (``READ_ONCE``, WRITE_ONCE``,
> 
> ...
> 
> if all accesses to a variable that is accessed concurrently are properly
> marked, KCSAN will never trigger a watchpoint
> 
> 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().
> 
> 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?

X + 1 and READ_ONCE(X) are two read.

> 
> Thanks,
> 
> Oleg.
> 
>