From: Joel Fernandes <joel@joelfernandes.org>
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>
Subject: Re: [PATCH rcu 3/3] srcu: Explain why callbacks invocations can't run concurrently
Date: Wed, 13 Dec 2023 14:01:24 -0500 [thread overview]
Message-ID: <CAEXW_YRhf71h7_jwYAYLFXN=B6_cK7AMoGgWFXAUQfkUjkmiXA@mail.gmail.com> (raw)
In-Reply-To: <9109c700-a353-4b12-a7c5-2f67e9ab4e86@paulmck-laptop>
On Wed, Dec 13, 2023 at 1:55 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Wed, Dec 13, 2023 at 01:35:22PM -0500, Joel Fernandes wrote:
> > On Wed, Dec 13, 2023 at 12:52 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Wed, Dec 13, 2023 at 09:27:09AM -0500, Joel Fernandes wrote:
> > > > On Tue, Dec 12, 2023 at 12:48 PM 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-obvious,
> > > > > 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_NEXT_TAIL));
> > > > > rcu_segcblist_advance(&sdp->srcu_cblist,
> > > > > rcu_seq_current(&ssp->srcu_sup->srcu_gp_seq));
> > > > > + /*
> > > > > + * Although this function is theoretically re-entrant, concurrent
> > > > > + * callbacks invocation is disallowed to avoid executing an SRCU 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).
>
> You are quite correct! But even before this, I was saying that
> lack of same-CPU callback concurrency was an accident of the current
> implementation rather than a guarantee. For example, there might come
> a time when RCU needs to respond to callback flooding with concurrent
> execution of the flooded CPU's callbacks. Or not, but we do need to
> keep this option open.
Got it, reminds me to focus on requirements as well along with implementation.
> > 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.
>
> That is still one motivation for ordered execution of callbacks. For the
> dynamic nocb switch, we could have chosen to make rcu_barrier() place
> a callback on both lists, but we instead chose to exclude rcu_barrier()
> calls during the switch.
Right!
> > I should just shut up and hide in shame now.
>
> No need for that! After all, one motivation for Requirements.rst was
> to help me keep track of all this stuff.
Thanks!
- Joel
next prev parent reply other threads:[~2023-12-13 19:01 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-12 17:47 [PATCH rcu 0/3] SRCU updates for v6.8 Neeraj Upadhyay (AMD)
2023-12-12 17:48 ` [PATCH rcu 1/3] srcu: Remove superfluous callbacks advancing from srcu_gp_start() Neeraj Upadhyay (AMD)
2023-12-12 17:48 ` [PATCH rcu 2/3] srcu: No need to advance/accelerate if no callback enqueued Neeraj Upadhyay (AMD)
2023-12-12 17:48 ` [PATCH rcu 3/3] srcu: Explain why callbacks invocations can't run concurrently Neeraj Upadhyay (AMD)
2023-12-13 14:27 ` Joel Fernandes
2023-12-13 17:52 ` Paul E. McKenney
2023-12-13 18:35 ` Joel Fernandes
2023-12-13 18:55 ` Paul E. McKenney
2023-12-13 19:01 ` Joel Fernandes [this message]
2023-12-13 18:18 ` [PATCH rcu 4/3] srcu: Use try-lock lockdep annotation for NMI-safe access Neeraj Upadhyay (AMD)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEXW_YRhf71h7_jwYAYLFXN=B6_cK7AMoGgWFXAUQfkUjkmiXA@mail.gmail.com' \
--to=joel@joelfernandes.org \
--cc=Neeraj.Upadhyay@amd.com \
--cc=frederic@kernel.org \
--cc=kernel-team@meta.com \
--cc=linux-kernel@vger.kernel.org \
--cc=neeraj.iitr10@gmail.com \
--cc=paulmck@kernel.org \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).