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

[Joel Fernandes]

On Sat, Oct 17, 2020 at 7:31 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Oct 16, 2020 at 09:27:53PM -0400, joel@xxxxxxxxxxxxxxxxx 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.

Right, that works as well. The main point I was trying to hit was the
control-dependency hardware ordering in P1 (due to rcu_barrier()
checking for a condition before doing whatever is after the
rcu_barrier()).

thanks,

 - Joel