[PATCH 0/3] rcu: synchronize_rcu[_expedited]() related fixes

[PATCH 0/3] rcu: synchronize_rcu[_expedited]() related fixes

[Frederic Weisbecker]

A few fixes especially for expedited GP polling causing a stall on TREE07,
as reported by Paul.

We may still want to optimize start_poll_synchronize_rcu_expedited() on
UP-no-preempt but I think Paul may be implying this while doing other
fixes.

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	rcu/dev

HEAD: 6e5fd7e614fd5c8f0fffeaa140b7ea697bfeb096

Thanks,
	Frederic
---

Frederic Weisbecker (2):
      rcu: Fix expedited GP polling against UP/no-preempt environment
      rcu: Fix preemption mode check on synchronize_rcu[_expedited]()

Valentin Schneider (1):
      preempt/dynamic: Introduce preempt mode accessors


 include/linux/sched.h | 16 +++++++++++++++
 kernel/rcu/tree.c     |  2 +-
 kernel/rcu/tree_exp.h | 57 +++++++++++++++++++++++++++++++--------------------
 kernel/sched/core.c   | 11 ++++++++++
 4 files changed, 63 insertions(+), 23 deletions(-)

[PATCH 1/3] rcu: Fix expedited GP polling against UP/no-preempt environment

[Frederic Weisbecker]

synchronize_rcu_expedited() has an early return condition: if the
current CPU is the only one online and the kernel doesn't run in
preemption mode, the current assumed quiescent state is worth a grace
period.

However the expedited grace period polling caller of
synchronize_rcu_expedited() takes a GP sequence snapshot and expects it
to complete by the end of the synchronize_rcu_expedited() call. Yet if
synchronize_rcu_expedited() relies on the above described UP/no-preempt
early return, the grace period sequence won't move and may cause
an expedited grace period polling stall.

Fix this with treating polling differently while calling
synchronize_rcu_expedited() and ignore the UP-no-preempt optimization
in this case.

Reported-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Uladzislau Rezki <uladzislau.rezki@sony.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/rcu/tree_exp.h | 57 ++++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
index d5f30085b0cf..3d8216ced93e 100644
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -794,27 +794,14 @@ static int rcu_print_task_exp_stall(struct rcu_node *rnp)
 
 #endif /* #else #ifdef CONFIG_PREEMPT_RCU */
 
-/**
- * synchronize_rcu_expedited - Brute-force RCU grace period
- *
- * Wait for an RCU grace period, but expedite it.  The basic idea is to
- * IPI all non-idle non-nohz online CPUs.  The IPI handler checks whether
- * the CPU is in an RCU critical section, and if so, it sets a flag that
- * causes the outermost rcu_read_unlock() to report the quiescent state
- * for RCU-preempt or asks the scheduler for help for RCU-sched.  On the
- * other hand, if the CPU is not in an RCU read-side critical section,
- * the IPI handler reports the quiescent state immediately.
- *
- * Although this is a great improvement over previous expedited
- * implementations, it is still unfriendly to real-time workloads, so is
- * thus not recommended for any sort of common-case code.  In fact, if
- * you are using synchronize_rcu_expedited() in a loop, please restructure
- * your code to batch your updates, and then use a single synchronize_rcu()
- * instead.
- *
- * This has the same semantics as (but is more brutal than) synchronize_rcu().
+/*
+ * Start and wait for an expedited grace period completion.
+ * If it happens to be called by polling functions (@polling = true),
+ * there is no possible early return in UP no-preempt mode because
+ * the callers are waiting for an actual given sequence snapshot to start
+ * and end.
  */
-void synchronize_rcu_expedited(void)
+static void __synchronize_rcu_expedited(bool polling)
 {
 	bool boottime = (rcu_scheduler_active == RCU_SCHEDULER_INIT);
 	struct rcu_exp_work rew;
@@ -827,7 +814,7 @@ void synchronize_rcu_expedited(void)
 			 "Illegal synchronize_rcu_expedited() in RCU read-side critical section");
 
 	/* Is the state is such that the call is a grace period? */
-	if (rcu_blocking_is_gp())
+	if (rcu_blocking_is_gp() && !polling)
 		return;
 
 	/* If expedited grace periods are prohibited, fall back to normal. */
@@ -863,6 +850,32 @@ void synchronize_rcu_expedited(void)
 
 	if (likely(!boottime))
 		destroy_work_on_stack(&rew.rew_work);
+
+}
+
+/**
+ * synchronize_rcu_expedited - Brute-force RCU grace period
+ *
+ * Wait for an RCU grace period, but expedite it.  The basic idea is to
+ * IPI all non-idle non-nohz online CPUs.  The IPI handler checks whether
+ * the CPU is in an RCU critical section, and if so, it sets a flag that
+ * causes the outermost rcu_read_unlock() to report the quiescent state
+ * for RCU-preempt or asks the scheduler for help for RCU-sched.  On the
+ * other hand, if the CPU is not in an RCU read-side critical section,
+ * the IPI handler reports the quiescent state immediately.
+ *
+ * Although this is a great improvement over previous expedited
+ * implementations, it is still unfriendly to real-time workloads, so is
+ * thus not recommended for any sort of common-case code.  In fact, if
+ * you are using synchronize_rcu_expedited() in a loop, please restructure
+ * your code to batch your updates, and then use a single synchronize_rcu()
+ * instead.
+ *
+ * This has the same semantics as (but is more brutal than) synchronize_rcu().
+ */
+void synchronize_rcu_expedited(void)
+{
+	__synchronize_rcu_expedited(false);
 }
 EXPORT_SYMBOL_GPL(synchronize_rcu_expedited);
 
@@ -903,7 +916,7 @@ static void sync_rcu_do_polled_gp(struct work_struct *wp)
 	if (s & 0x1)
 		return;
 	while (!sync_exp_work_done(s))
-		synchronize_rcu_expedited();
+		__synchronize_rcu_expedited(true);
 	raw_spin_lock_irqsave(&rnp->exp_poll_lock, flags);
 	s = rnp->exp_seq_poll_rq;
 	if (!(s & 0x1) && !sync_exp_work_done(s))
-- 
2.25.1

[PATCH 2/3] preempt/dynamic: Introduce preempt mode accessors

[Frederic Weisbecker]

From: Valentin Schneider <valentin.schneider@arm.com>

CONFIG_PREEMPT{_NONE, _VOLUNTARY} designate either:
o The build-time preemption model when !PREEMPT_DYNAMIC
o The default boot-time preemption model when PREEMPT_DYNAMIC

IOW, using those on PREEMPT_DYNAMIC kernels is meaningless - the actual
model could have been set to something else by the "preempt=foo" cmdline
parameter.

Introduce a set of helpers to determine the actual preemption mode used by
the live kernel.

Suggested-by: Marco Elver <elver@google.com>
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Uladzislau Rezki <uladzislau.rezki@sony.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com>
---
 include/linux/sched.h | 16 ++++++++++++++++
 kernel/sched/core.c   | 11 +++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 508b91d57470..d348e886e4d0 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2096,6 +2096,22 @@ static inline void cond_resched_rcu(void)
 #endif
 }
 
+#ifdef CONFIG_PREEMPT_DYNAMIC
+
+extern bool preempt_mode_none(void);
+extern bool preempt_mode_voluntary(void);
+extern bool preempt_mode_full(void);
+
+#else
+
+#define preempt_mode_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
+#define preempt_mode_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
+#define preempt_mode_full() IS_ENABLED(CONFIG_PREEMPT)
+
+#endif
+
+#define preempt_mode_rt() IS_ENABLED(CONFIG_PREEMPT_RT)
+
 /*
  * Does a critical section need to be broken due to another
  * task waiting?: (technically does not depend on CONFIG_PREEMPTION,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2e4ae00e52d1..6e5c8f0f874a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6684,6 +6684,17 @@ static void __init preempt_dynamic_init(void)
 	}
 }
 
+#define PREEMPT_MODE_ACCESSOR(mode) \
+	bool preempt_mode_##mode(void)						 \
+	{									 \
+		WARN_ON_ONCE(preempt_dynamic_mode == preempt_dynamic_undefined); \
+		return preempt_dynamic_mode == preempt_dynamic_##mode;		 \
+	}
+
+PREEMPT_MODE_ACCESSOR(none)
+PREEMPT_MODE_ACCESSOR(voluntary)
+PREEMPT_MODE_ACCESSOR(full)
+
 #else /* !CONFIG_PREEMPT_DYNAMIC */
 
 static inline void preempt_dynamic_init(void) { }
-- 
2.25.1

[PATCH 3/3] rcu: Fix preemption mode check on synchronize_rcu[_expedited]()

[Frederic Weisbecker]

An early check on synchronize_rcu[_expedited]() tries to determine if
the current CPU is in UP mode on an SMP no-preempt kernel, in which case
there is no need to start a grace period since the current assumed
quiescent state is all we need.

However the preemption mode doesn't take into account the boot selected
preemption mode under CONFIG_PREEMPT_DYNAMIC=y, missing a possible
early return if the running flavour is "none" or "voluntary".

Use the shiny new preempt mode accessors to fix this.

Reported-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Uladzislau Rezki <uladzislau.rezki@sony.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/rcu/tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index d2db709f83d9..8fe2f9843139 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3769,7 +3769,7 @@ static int rcu_blocking_is_gp(void)
 {
 	int ret;
 
-	if (IS_ENABLED(CONFIG_PREEMPTION))
+	if (preempt_mode_full() || preempt_mode_rt())
 		return rcu_scheduler_active == RCU_SCHEDULER_INACTIVE;
 	might_sleep();  /* Check for RCU read-side critical section. */
 	preempt_disable();
-- 
2.25.1

Re: [PATCH 2/3] preempt/dynamic: Introduce preempt mode accessors

[Marco Elver]

On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <frederic@kernel.org> wrote:
>
> From: Valentin Schneider <valentin.schneider@arm.com>
>
> CONFIG_PREEMPT{_NONE, _VOLUNTARY} designate either:
> o The build-time preemption model when !PREEMPT_DYNAMIC
> o The default boot-time preemption model when PREEMPT_DYNAMIC
>
> IOW, using those on PREEMPT_DYNAMIC kernels is meaningless - the actual
> model could have been set to something else by the "preempt=foo" cmdline
> parameter.
>
> Introduce a set of helpers to determine the actual preemption mode used by
> the live kernel.
>
> Suggested-by: Marco Elver <elver@google.com>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> Cc: Uladzislau Rezki <uladzislau.rezki@sony.com>
> Cc: Joel Fernandes <joel@joelfernandes.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com>
> ---
>  include/linux/sched.h | 16 ++++++++++++++++
>  kernel/sched/core.c   | 11 +++++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 508b91d57470..d348e886e4d0 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2096,6 +2096,22 @@ static inline void cond_resched_rcu(void)
>  #endif
>  }
>
> +#ifdef CONFIG_PREEMPT_DYNAMIC
> +
> +extern bool preempt_mode_none(void);
> +extern bool preempt_mode_voluntary(void);
> +extern bool preempt_mode_full(void);
> +
> +#else
> +
> +#define preempt_mode_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> +#define preempt_mode_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> +#define preempt_mode_full() IS_ENABLED(CONFIG_PREEMPT)
> +

Shame this was somehow forgotten.
There was a v3 of this patch that fixed a bunch of things (e.g. making
these proper functions so all builds error if accidentally used in
#if).

https://lore.kernel.org/lkml/20211112185203.280040-3-valentin.schneider@arm.com/

Is it also possible to take all the rest of that series (all 4
patches) from Valentin?

Thanks,
-- Marco

Re: [PATCH 2/3] preempt/dynamic: Introduce preempt mode accessors

[Paul E. McKenney]

On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <frederic@kernel.org> wrote:
> >
> > From: Valentin Schneider <valentin.schneider@arm.com>
> >
> > CONFIG_PREEMPT{_NONE, _VOLUNTARY} designate either:
> > o The build-time preemption model when !PREEMPT_DYNAMIC
> > o The default boot-time preemption model when PREEMPT_DYNAMIC
> >
> > IOW, using those on PREEMPT_DYNAMIC kernels is meaningless - the actual
> > model could have been set to something else by the "preempt=foo" cmdline
> > parameter.
> >
> > Introduce a set of helpers to determine the actual preemption mode used by
> > the live kernel.
> >
> > Suggested-by: Marco Elver <elver@google.com>
> > Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > Cc: Uladzislau Rezki <uladzislau.rezki@sony.com>
> > Cc: Joel Fernandes <joel@joelfernandes.org>
> > Cc: Boqun Feng <boqun.feng@gmail.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com>
> > ---
> >  include/linux/sched.h | 16 ++++++++++++++++
> >  kernel/sched/core.c   | 11 +++++++++++
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 508b91d57470..d348e886e4d0 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -2096,6 +2096,22 @@ static inline void cond_resched_rcu(void)
> >  #endif
> >  }
> >
> > +#ifdef CONFIG_PREEMPT_DYNAMIC
> > +
> > +extern bool preempt_mode_none(void);
> > +extern bool preempt_mode_voluntary(void);
> > +extern bool preempt_mode_full(void);
> > +
> > +#else
> > +
> > +#define preempt_mode_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> > +#define preempt_mode_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> > +#define preempt_mode_full() IS_ENABLED(CONFIG_PREEMPT)
> > +
> 
> Shame this was somehow forgotten.
> There was a v3 of this patch that fixed a bunch of things (e.g. making
> these proper functions so all builds error if accidentally used in
> #if).
> 
> https://lore.kernel.org/lkml/20211112185203.280040-3-valentin.schneider@arm.com/
> 
> Is it also possible to take all the rest of that series (all 4
> patches) from Valentin?

Me, I am assuming that #2/3 is an experimental test so that I am able
to easily whack this series over the head with rcutorture.  ;-)

							Thanx, Paul

Re: [PATCH 0/3] rcu: synchronize_rcu[_expedited]() related fixes

[Paul E. McKenney]

On Mon, Mar 14, 2022 at 02:37:35PM +0100, Frederic Weisbecker wrote:
> 
> A few fixes especially for expedited GP polling causing a stall on TREE07,
> as reported by Paul.
> 
> We may still want to optimize start_poll_synchronize_rcu_expedited() on
> UP-no-preempt but I think Paul may be implying this while doing other
> fixes.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> 	rcu/dev
> 
> HEAD: 6e5fd7e614fd5c8f0fffeaa140b7ea697bfeb096
> 
> Thanks,
> 	Frederic

I have pulled these in for review and testing, thank you!!!  I have
started a ~90-minute test and will let you know how that goes.

> ---
> 
> Frederic Weisbecker (2):
>       rcu: Fix expedited GP polling against UP/no-preempt environment

I have some concerns with this one due to the fact that it acquires locks
in cases where the old code would not.  (I did have problems with this
in both recent SRCU changes and the normal-grace-period counterpart to
this series.)

But let's see what rcutorture and kbuild test robot think about it.

>       rcu: Fix preemption mode check on synchronize_rcu[_expedited]()

This one looks good.

> Valentin Schneider (1):
>       preempt/dynamic: Introduce preempt mode accessors

I am guessing that this one is a compact placeholder for my convenience
(in which case thank you!).  I will be marking it "EXP" on my next rebase.

							Thanx, Paul

>  include/linux/sched.h | 16 +++++++++++++++
>  kernel/rcu/tree.c     |  2 +-
>  kernel/rcu/tree_exp.h | 57 +++++++++++++++++++++++++++++++--------------------
>  kernel/sched/core.c   | 11 ++++++++++
>  4 files changed, 63 insertions(+), 23 deletions(-)

Re: [PATCH 2/3] preempt/dynamic: Introduce preempt mode accessors

[Marco Elver]

On Mon, 14 Mar 2022 at 21:06, Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> > On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <frederic@kernel.org> wrote:
> > >
> > > From: Valentin Schneider <valentin.schneider@arm.com>
> > >
> > > CONFIG_PREEMPT{_NONE, _VOLUNTARY} designate either:
> > > o The build-time preemption model when !PREEMPT_DYNAMIC
> > > o The default boot-time preemption model when PREEMPT_DYNAMIC
> > >
> > > IOW, using those on PREEMPT_DYNAMIC kernels is meaningless - the actual
> > > model could have been set to something else by the "preempt=foo" cmdline
> > > parameter.
> > >
> > > Introduce a set of helpers to determine the actual preemption mode used by
> > > the live kernel.
> > >
> > > Suggested-by: Marco Elver <elver@google.com>
> > > Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > > Cc: Uladzislau Rezki <uladzislau.rezki@sony.com>
> > > Cc: Joel Fernandes <joel@joelfernandes.org>
> > > Cc: Boqun Feng <boqun.feng@gmail.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com>
> > > ---
> > >  include/linux/sched.h | 16 ++++++++++++++++
> > >  kernel/sched/core.c   | 11 +++++++++++
> > >  2 files changed, 27 insertions(+)
> > >
> > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > index 508b91d57470..d348e886e4d0 100644
> > > --- a/include/linux/sched.h
> > > +++ b/include/linux/sched.h
> > > @@ -2096,6 +2096,22 @@ static inline void cond_resched_rcu(void)
> > >  #endif
> > >  }
> > >
> > > +#ifdef CONFIG_PREEMPT_DYNAMIC
> > > +
> > > +extern bool preempt_mode_none(void);
> > > +extern bool preempt_mode_voluntary(void);
> > > +extern bool preempt_mode_full(void);
> > > +
> > > +#else
> > > +
> > > +#define preempt_mode_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> > > +#define preempt_mode_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> > > +#define preempt_mode_full() IS_ENABLED(CONFIG_PREEMPT)
> > > +
> >
> > Shame this was somehow forgotten.
> > There was a v3 of this patch that fixed a bunch of things (e.g. making
> > these proper functions so all builds error if accidentally used in
> > #if).
> >
> > https://lore.kernel.org/lkml/20211112185203.280040-3-valentin.schneider@arm.com/
> >
> > Is it also possible to take all the rest of that series (all 4
> > patches) from Valentin?
>
> Me, I am assuming that #2/3 is an experimental test so that I am able
> to easily whack this series over the head with rcutorture.  ;-)

I might be out of the loop here. All I can add is that any issues that
are a consequence of the preempt mode accessors are only testable if
the preemption model is actually changed at runtime and AFAIK
rcutorture doesn't do that. But as noted, this patch wasn't the latest
version and there were issues with it fixed by Valentin's latest v3
(from November, but had never been picked up anywhere).

Thanks,
-- Marco

Re: [PATCH 2/3] preempt/dynamic: Introduce preempt mode accessors

[Paul E. McKenney]

On Mon, Mar 14, 2022 at 09:34:26PM +0100, Marco Elver wrote:
> On Mon, 14 Mar 2022 at 21:06, Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> > > On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <frederic@kernel.org> wrote:
> > > >
> > > > From: Valentin Schneider <valentin.schneider@arm.com>
> > > >
> > > > CONFIG_PREEMPT{_NONE, _VOLUNTARY} designate either:
> > > > o The build-time preemption model when !PREEMPT_DYNAMIC
> > > > o The default boot-time preemption model when PREEMPT_DYNAMIC
> > > >
> > > > IOW, using those on PREEMPT_DYNAMIC kernels is meaningless - the actual
> > > > model could have been set to something else by the "preempt=foo" cmdline
> > > > parameter.
> > > >
> > > > Introduce a set of helpers to determine the actual preemption mode used by
> > > > the live kernel.
> > > >
> > > > Suggested-by: Marco Elver <elver@google.com>
> > > > Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> > > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > > > Cc: Uladzislau Rezki <uladzislau.rezki@sony.com>
> > > > Cc: Joel Fernandes <joel@joelfernandes.org>
> > > > Cc: Boqun Feng <boqun.feng@gmail.com>
> > > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > > Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com>
> > > > ---
> > > >  include/linux/sched.h | 16 ++++++++++++++++
> > > >  kernel/sched/core.c   | 11 +++++++++++
> > > >  2 files changed, 27 insertions(+)
> > > >
> > > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > > index 508b91d57470..d348e886e4d0 100644
> > > > --- a/include/linux/sched.h
> > > > +++ b/include/linux/sched.h
> > > > @@ -2096,6 +2096,22 @@ static inline void cond_resched_rcu(void)
> > > >  #endif
> > > >  }
> > > >
> > > > +#ifdef CONFIG_PREEMPT_DYNAMIC
> > > > +
> > > > +extern bool preempt_mode_none(void);
> > > > +extern bool preempt_mode_voluntary(void);
> > > > +extern bool preempt_mode_full(void);
> > > > +
> > > > +#else
> > > > +
> > > > +#define preempt_mode_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> > > > +#define preempt_mode_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> > > > +#define preempt_mode_full() IS_ENABLED(CONFIG_PREEMPT)
> > > > +
> > >
> > > Shame this was somehow forgotten.
> > > There was a v3 of this patch that fixed a bunch of things (e.g. making
> > > these proper functions so all builds error if accidentally used in
> > > #if).
> > >
> > > https://lore.kernel.org/lkml/20211112185203.280040-3-valentin.schneider@arm.com/
> > >
> > > Is it also possible to take all the rest of that series (all 4
> > > patches) from Valentin?
> >
> > Me, I am assuming that #2/3 is an experimental test so that I am able
> > to easily whack this series over the head with rcutorture.  ;-)
> 
> I might be out of the loop here. All I can add is that any issues that
> are a consequence of the preempt mode accessors are only testable if
> the preemption model is actually changed at runtime and AFAIK
> rcutorture doesn't do that. But as noted, this patch wasn't the latest
> version and there were issues with it fixed by Valentin's latest v3
> (from November, but had never been picked up anywhere).

I will be marking 2/3 "EXP" to prevent myself from accidentally sending
it upstream.

But longer term, maybe I should pick up Valentin's series.

This stuff is v5.19 at the earliest, so not (yet) urgent.

							Thanx, Paul

Re: [PATCH 0/3] rcu: synchronize_rcu[_expedited]() related fixes

[Paul E. McKenney]

On Mon, Mar 14, 2022 at 01:26:10PM -0700, Paul E. McKenney wrote:
> On Mon, Mar 14, 2022 at 02:37:35PM +0100, Frederic Weisbecker wrote:
> > 
> > A few fixes especially for expedited GP polling causing a stall on TREE07,
> > as reported by Paul.
> > 
> > We may still want to optimize start_poll_synchronize_rcu_expedited() on
> > UP-no-preempt but I think Paul may be implying this while doing other
> > fixes.
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> > 	rcu/dev
> > 
> > HEAD: 6e5fd7e614fd5c8f0fffeaa140b7ea697bfeb096
> > 
> > Thanks,
> > 	Frederic
> 
> I have pulled these in for review and testing, thank you!!!  I have
> started a ~90-minute test and will let you know how that goes.

And TREE05 doesn't like this much.  I get too-short grace periods.
TREE05 is unusual in being the one with the kernel build with
CONFIG_PREEMPT_NONE=y but also with CONFIG_PREEMPT_DYNAMIC=n.

Running tests of the commits individually.

							Thanx, Paul

> > ---
> > 
> > Frederic Weisbecker (2):
> >       rcu: Fix expedited GP polling against UP/no-preempt environment
> 
> I have some concerns with this one due to the fact that it acquires locks
> in cases where the old code would not.  (I did have problems with this
> in both recent SRCU changes and the normal-grace-period counterpart to
> this series.)
> 
> But let's see what rcutorture and kbuild test robot think about it.
> 
> >       rcu: Fix preemption mode check on synchronize_rcu[_expedited]()
> 
> This one looks good.
> 
> > Valentin Schneider (1):
> >       preempt/dynamic: Introduce preempt mode accessors
> 
> I am guessing that this one is a compact placeholder for my convenience
> (in which case thank you!).  I will be marking it "EXP" on my next rebase.
> 
> 							Thanx, Paul
> 
> >  include/linux/sched.h | 16 +++++++++++++++
> >  kernel/rcu/tree.c     |  2 +-
> >  kernel/rcu/tree_exp.h | 57 +++++++++++++++++++++++++++++++--------------------
> >  kernel/sched/core.c   | 11 ++++++++++
> >  4 files changed, 63 insertions(+), 23 deletions(-)

Re: [PATCH 2/3] preempt/dynamic: Introduce preempt mode accessors

[Frederic Weisbecker]

On Mon, Mar 14, 2022 at 09:34:26PM +0100, Marco Elver wrote:
> On Mon, 14 Mar 2022 at 21:06, Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> > > On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <frederic@kernel.org> wrote:
> > > >
> > > > From: Valentin Schneider <valentin.schneider@arm.com>
> > > >
> > > > CONFIG_PREEMPT{_NONE, _VOLUNTARY} designate either:
> > > > o The build-time preemption model when !PREEMPT_DYNAMIC
> > > > o The default boot-time preemption model when PREEMPT_DYNAMIC
> > > >
> > > > IOW, using those on PREEMPT_DYNAMIC kernels is meaningless - the actual
> > > > model could have been set to something else by the "preempt=foo" cmdline
> > > > parameter.
> > > >
> > > > Introduce a set of helpers to determine the actual preemption mode used by
> > > > the live kernel.
> > > >
> > > > Suggested-by: Marco Elver <elver@google.com>
> > > > Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> > > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > > > Cc: Uladzislau Rezki <uladzislau.rezki@sony.com>
> > > > Cc: Joel Fernandes <joel@joelfernandes.org>
> > > > Cc: Boqun Feng <boqun.feng@gmail.com>
> > > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > > Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com>
> > > > ---
> > > >  include/linux/sched.h | 16 ++++++++++++++++
> > > >  kernel/sched/core.c   | 11 +++++++++++
> > > >  2 files changed, 27 insertions(+)
> > > >
> > > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > > index 508b91d57470..d348e886e4d0 100644
> > > > --- a/include/linux/sched.h
> > > > +++ b/include/linux/sched.h
> > > > @@ -2096,6 +2096,22 @@ static inline void cond_resched_rcu(void)
> > > >  #endif
> > > >  }
> > > >
> > > > +#ifdef CONFIG_PREEMPT_DYNAMIC
> > > > +
> > > > +extern bool preempt_mode_none(void);
> > > > +extern bool preempt_mode_voluntary(void);
> > > > +extern bool preempt_mode_full(void);
> > > > +
> > > > +#else
> > > > +
> > > > +#define preempt_mode_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> > > > +#define preempt_mode_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> > > > +#define preempt_mode_full() IS_ENABLED(CONFIG_PREEMPT)
> > > > +
> > >
> > > Shame this was somehow forgotten.
> > > There was a v3 of this patch that fixed a bunch of things (e.g. making
> > > these proper functions so all builds error if accidentally used in
> > > #if).
> > >
> > > https://lore.kernel.org/lkml/20211112185203.280040-3-valentin.schneider@arm.com/
> > >
> > > Is it also possible to take all the rest of that series (all 4
> > > patches) from Valentin?
> >
> > Me, I am assuming that #2/3 is an experimental test so that I am able
> > to easily whack this series over the head with rcutorture.  ;-)
> 
> I might be out of the loop here. All I can add is that any issues that
> are a consequence of the preempt mode accessors are only testable if
> the preemption model is actually changed at runtime and AFAIK
> rcutorture doesn't do that. But as noted, this patch wasn't the latest
> version and there were issues with it fixed by Valentin's latest v3
> (from November, but had never been picked up anywhere).

Thanks for the reminder, the patchset indeed got forgotten somewhow.
If necessary I'll resend it.

Re: [PATCH 2/3] preempt/dynamic: Introduce preempt mode accessors

[Peter Zijlstra]

On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> https://lore.kernel.org/lkml/20211112185203.280040-3-valentin.schneider@arm.com/
> 
> Is it also possible to take all the rest of that series (all 4
> patches) from Valentin?

I'll go stick the remaining 3 patches from Valentin in sched/core, they
seem to still apply without issue.

Re: [PATCH 2/3] preempt/dynamic: Introduce preempt mode accessors

[Paul E. McKenney]

On Tue, Mar 15, 2022 at 11:48:17AM +0100, Peter Zijlstra wrote:
> On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> > https://lore.kernel.org/lkml/20211112185203.280040-3-valentin.schneider@arm.com/
> > 
> > Is it also possible to take all the rest of that series (all 4
> > patches) from Valentin?
> 
> I'll go stick the remaining 3 patches from Valentin in sched/core, they
> seem to still apply without issue.

If testing goes well, are you planning to push into the upcoming
merge window?  (I won't be pushing until the v5.19 merge window at
the earliest.)

Either way, just please let me know!

							Thanx, Paul

Re: [PATCH 0/3] rcu: synchronize_rcu[_expedited]() related fixes

[Frederic Weisbecker]

On Mon, Mar 14, 2022 at 03:35:04PM -0700, Paul E. McKenney wrote:
> On Mon, Mar 14, 2022 at 01:26:10PM -0700, Paul E. McKenney wrote:
> > On Mon, Mar 14, 2022 at 02:37:35PM +0100, Frederic Weisbecker wrote:
> > > 
> > > A few fixes especially for expedited GP polling causing a stall on TREE07,
> > > as reported by Paul.
> > > 
> > > We may still want to optimize start_poll_synchronize_rcu_expedited() on
> > > UP-no-preempt but I think Paul may be implying this while doing other
> > > fixes.
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> > > 	rcu/dev
> > > 
> > > HEAD: 6e5fd7e614fd5c8f0fffeaa140b7ea697bfeb096
> > > 
> > > Thanks,
> > > 	Frederic
> > 
> > I have pulled these in for review and testing, thank you!!!  I have
> > started a ~90-minute test and will let you know how that goes.
> 
> And TREE05 doesn't like this much.  I get too-short grace periods.
> TREE05 is unusual in being the one with the kernel build with
> CONFIG_PREEMPT_NONE=y but also with CONFIG_PREEMPT_DYNAMIC=n.
> 
> Running tests of the commits individually.

Ouch, please test the following:

---
From f180dd5809d2c3a6343cbd13f244b7b7f110a506 Mon Sep 17 00:00:00 2001
From: Frederic Weisbecker <frederic@kernel.org>
Date: Tue, 15 Mar 2022 16:33:38 +0100
Subject: [PATCH] rcutorture: Call preempt_schedule() through static call/key

rcutorture sometimes want to trigger a random scheduler preemption call
while simulating a read delay. However a direct call to
preempt_schedule() is not desirable because it bypasses the static
call/key filter used by CONFIG_PREEMPT_DYNAMIC. This breaks the
no-preempt assumption in case the dynamic preemption mode is "none".

For example rcu_blocking_is_gp() is fooled and abbreviates grace periods
when the CPU runs in no-preempt UP mode.

Fix this with making torture_preempt_schedule() to call through
preempt dynamic static call/key.

Reported-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 include/linux/torture.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/torture.h b/include/linux/torture.h
index 63fa4196e51c..7038104463e4 100644
--- a/include/linux/torture.h
+++ b/include/linux/torture.h
@@ -118,7 +118,7 @@ void _torture_stop_kthread(char *m, struct task_struct **tp);
 	_torture_stop_kthread("Stopping " #n " task", &(tp))
 
 #ifdef CONFIG_PREEMPTION
-#define torture_preempt_schedule() preempt_schedule()
+#define torture_preempt_schedule() __preempt_schedule()
 #else
 #define torture_preempt_schedule()	do { } while (0)
 #endif
-- 
2.25.1

Re: [PATCH 0/3] rcu: synchronize_rcu[_expedited]() related fixes

[Paul E. McKenney]

On Tue, Mar 15, 2022 at 04:52:26PM +0100, Frederic Weisbecker wrote:
> On Mon, Mar 14, 2022 at 03:35:04PM -0700, Paul E. McKenney wrote:
> > On Mon, Mar 14, 2022 at 01:26:10PM -0700, Paul E. McKenney wrote:
> > > On Mon, Mar 14, 2022 at 02:37:35PM +0100, Frederic Weisbecker wrote:
> > > > 
> > > > A few fixes especially for expedited GP polling causing a stall on TREE07,
> > > > as reported by Paul.
> > > > 
> > > > We may still want to optimize start_poll_synchronize_rcu_expedited() on
> > > > UP-no-preempt but I think Paul may be implying this while doing other
> > > > fixes.
> > > > 
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> > > > 	rcu/dev
> > > > 
> > > > HEAD: 6e5fd7e614fd5c8f0fffeaa140b7ea697bfeb096
> > > > 
> > > > Thanks,
> > > > 	Frederic
> > > 
> > > I have pulled these in for review and testing, thank you!!!  I have
> > > started a ~90-minute test and will let you know how that goes.
> > 
> > And TREE05 doesn't like this much.  I get too-short grace periods.
> > TREE05 is unusual in being the one with the kernel build with
> > CONFIG_PREEMPT_NONE=y but also with CONFIG_PREEMPT_DYNAMIC=n.
> > 
> > Running tests of the commits individually.
> 
> Ouch, please test the following:
> 
> ---
> >From f180dd5809d2c3a6343cbd13f244b7b7f110a506 Mon Sep 17 00:00:00 2001
> From: Frederic Weisbecker <frederic@kernel.org>
> Date: Tue, 15 Mar 2022 16:33:38 +0100
> Subject: [PATCH] rcutorture: Call preempt_schedule() through static call/key

You know, it would have been quite some time before I would have thought
to suspect that code, so thank you very much for chasing this down!

I have pulled it in, reverted my reversion of your patch 3/3, and started
a moderate set of tests.

							Thanx, Paul

> rcutorture sometimes want to trigger a random scheduler preemption call
> while simulating a read delay. However a direct call to
> preempt_schedule() is not desirable because it bypasses the static
> call/key filter used by CONFIG_PREEMPT_DYNAMIC. This breaks the
> no-preempt assumption in case the dynamic preemption mode is "none".
> 
> For example rcu_blocking_is_gp() is fooled and abbreviates grace periods
> when the CPU runs in no-preempt UP mode.
> 
> Fix this with making torture_preempt_schedule() to call through
> preempt dynamic static call/key.
> 
> Reported-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>  include/linux/torture.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/torture.h b/include/linux/torture.h
> index 63fa4196e51c..7038104463e4 100644
> --- a/include/linux/torture.h
> +++ b/include/linux/torture.h
> @@ -118,7 +118,7 @@ void _torture_stop_kthread(char *m, struct task_struct **tp);
>  	_torture_stop_kthread("Stopping " #n " task", &(tp))
>  
>  #ifdef CONFIG_PREEMPTION
> -#define torture_preempt_schedule() preempt_schedule()
> +#define torture_preempt_schedule() __preempt_schedule()
>  #else
>  #define torture_preempt_schedule()	do { } while (0)
>  #endif
> -- 
> 2.25.1
>