Re: [PATCH v7 6/6] rcu/segcblist: Add additional comments to explain smp_mb()

[Alan Stern <stern@rowland.harvard.edu>]

On Fri, Oct 16, 2020 at 09:27:53PM -0400, joel@joelfernandes.org wrote:
> Adding Alan as well as its memory barrier discussion ;-)

I don't know the internals of how RCU works, so I'll just speak to the 
litmus test itself, ignoring issues of whether the litmus test is 
appropriate or expresses what you really want.

> The following litmus test would confirm it:
> 
> C rcubarrier+ctrldep
> 
> (*
>  * Result: Never
>  *
>  * This litmus test shows that rcu_barrier (P1) prematurely
>  * returning by reading len 0 can cause issues if P0 does
>  * NOT have a smb_mb() before WRITE_ONCE().
>  *
>  * mod_data == 2 means garbage which the callback should never see.
>  *)
> 
> { int len = 1; }
> 
> P0(int *len, int *mod_data)
> {
>         int r0;
> 
>         // accessed by say RCU callback in rcu_do_batch();
>         r0 = READ_ONCE(*mod_data);
>         smp_mb(); // Remove this and the "exists" will become true.
>         WRITE_ONCE(*len, 0);
> }
> 
> P1(int *len, int *mod_data)
> {
>         int r0;
> 
>         r0 = READ_ONCE(*len);
> 
>         // rcu_barrier will return early if len is 0
>         if (r0 == 0)
>                 WRITE_ONCE(*mod_data, 2);
> }
> 
> // Is it possible?
> exists (0:r0=2 /\ 1:r0=0)

This result is indeed not possible.  And yes, some sort of memory 
barrier is needed in P0.  But it doesn't have to be smp_mb(); you could 
use a weaker barrier instead.  For example, you could replace the 
READ_ONCE in P0 with smp_load_acquire(), or you could replace the 
WRITE_ONCE with smp_store_release().  Either of those changes would 
suffice to prevent this outcome.

Alan