Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <20231212174750.GA11886@neeraj.linux> <20231212174817.11919-3-neeraj.iitr10@gmail.com>
 <CAEXW_YRHjdM+NA3CqNuwaRNXkRWbtypmt5Ov=YXnrpn3Eo-==Q@mail.gmail.com> <2b2c1573-337d-409b-a8ee-daeff096c7f4@paulmck-laptop>
In-Reply-To: <2b2c1573-337d-409b-a8ee-daeff096c7f4@paulmck-laptop>
From:   Joel Fernandes <joel@joelfernandes.org>
Date:   Wed, 13 Dec 2023 13:35:22 -0500
Message-ID: <CAEXW_YQnR51F9xnODZd3iE+S5Jpd2NHRBTk6Jt2WHTSdB9H8kA@mail.gmail.com>
Subject: Re: [PATCH rcu 3/3] srcu: Explain why callbacks invocations can't run concurrently
To:     paulmck@kernel.org
Cc:     "Neeraj Upadhyay (AMD)" <neeraj.iitr10@gmail.com>,
        rcu@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel-team@meta.com, rostedt@goodmis.org, Neeraj.Upadhyay@amd.com,
        Frederic Weisbecker <frederic@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Dec 13, 2023 at 12:52=E2=80=AFPM Paul E. McKenney <paulmck@kernel.o=
rg> wrote:
>
> On Wed, Dec 13, 2023 at 09:27:09AM -0500, Joel Fernandes wrote:
> > On Tue, Dec 12, 2023 at 12:48=E2=80=AFPM Neeraj Upadhyay (AMD)
> > <neeraj.iitr10@gmail.com> wrote:
> > >
> > > From: Frederic Weisbecker <frederic@kernel.org>
> > >
> > > If an SRCU barrier is queued while callbacks are running and a new
> > > callbacks invocator for the same sdp were to run concurrently, the
> > > RCU barrier might execute too early. As this requirement is non-obvio=
us,
> > > make sure to keep a record.
> > >
> > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > > Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > Signed-off-by: Neeraj Upadhyay (AMD) <neeraj.iitr10@gmail.com>
> > > ---
> > >  kernel/rcu/srcutree.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > > index 2bfc8ed1eed2..0351a4e83529 100644
> > > --- a/kernel/rcu/srcutree.c
> > > +++ b/kernel/rcu/srcutree.c
> > > @@ -1715,6 +1715,11 @@ static void srcu_invoke_callbacks(struct work_=
struct *work)
> > >         WARN_ON_ONCE(!rcu_segcblist_segempty(&sdp->srcu_cblist, RCU_N=
EXT_TAIL));
> > >         rcu_segcblist_advance(&sdp->srcu_cblist,
> > >                               rcu_seq_current(&ssp->srcu_sup->srcu_gp=
_seq));
> > > +       /*
> > > +        * Although this function is theoretically re-entrant, concur=
rent
> > > +        * callbacks invocation is disallowed to avoid executing an S=
RCU barrier
> > > +        * too early.
> > > +        */
> >
> > Side comment:
> > I guess even without the barrier reasoning, it is best not to allow
> > concurrent CB execution anyway since it diverges from the behavior of
> > straight RCU :)
>
> Good point!
>
> But please do not forget item 12 on the list in checklist.rst.  ;-)
> (Which I just updated to include the other call_rcu*() functions.)

I think this is more so now with recent kernels (with the dynamic nocb
switch) than with older kernels right? I haven't kept up with the
checklist recently (which is my bad).

My understanding comes from the fact that the RCU barrier depends on
callbacks on the same CPU executing in order with straight RCU
otherwise it breaks. Hence my comment. But as you pointed out, that's
outdated knowledge.

I should just shut up and hide in shame now.

:-/

 - Joel