Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201015002301.101830-1-joel@joelfernandes.org>
 <20201015002301.101830-7-joel@joelfernandes.org> <20201015133511.GB127222@lothringen>
 <20201017012753.GB4015033@google.com> <20201017143144.GA835860@rowland.harvard.edu>
In-Reply-To: <20201017143144.GA835860@rowland.harvard.edu>
From:   Joel Fernandes <joel@joelfernandes.org>
Date:   Sun, 18 Oct 2020 13:16:57 -0700
Message-ID: <CAEXW_YSyshFLYMNia+TGfO11Pi_iojzke1+0SG7anGt2GNUXWg@mail.gmail.com>
Subject: Re: [PATCH v7 6/6] rcu/segcblist: Add additional comments to explain smp_mb()
To:     Alan Stern <stern@rowland.harvard.edu>
Cc:     Frederic Weisbecker <frederic@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@redhat.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Marco Elver <elver@google.com>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        "Paul E. McKenney" <paulmck@kernel.org>, rcu <rcu@vger.kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        "Uladzislau Rezki (Sony)" <urezki@gmail.com>,
        Frederic Weisbecker <fweisbec@gmail.com>,
        Neeraj upadhyay <neeraj.iitr10@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Oct 17, 2020 at 7:31 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> 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.

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