2022-03-15 07:20:01

by Marco Elver

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

On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <[email protected]> wrote:
>
> From: Valentin Schneider <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Valentin Schneider <[email protected]>
> Signed-off-by: Frederic Weisbecker <[email protected]>
> Cc: Uladzislau Rezki <[email protected]>
> Cc: Joel Fernandes <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Neeraj Upadhyay <[email protected]>
> ---
> 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/[email protected]/

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

Thanks,
-- Marco


2022-03-15 20:19:04

by Paul E. McKenney

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

On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <[email protected]> wrote:
> >
> > From: Valentin Schneider <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Valentin Schneider <[email protected]>
> > Signed-off-by: Frederic Weisbecker <[email protected]>
> > Cc: Uladzislau Rezki <[email protected]>
> > Cc: Joel Fernandes <[email protected]>
> > Cc: Boqun Feng <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Neeraj Upadhyay <[email protected]>
> > ---
> > 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/[email protected]/
>
> 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

2022-03-17 05:02:32

by Peter Zijlstra

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

On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> https://lore.kernel.org/lkml/[email protected]/
>
> 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.

2022-03-17 06:05:58

by Marco Elver

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

On Mon, 14 Mar 2022 at 21:06, Paul E. McKenney <[email protected]> wrote:
>
> On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> > On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <[email protected]> wrote:
> > >
> > > From: Valentin Schneider <[email protected]>
> > >
> > > 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 <[email protected]>
> > > Signed-off-by: Valentin Schneider <[email protected]>
> > > Signed-off-by: Frederic Weisbecker <[email protected]>
> > > Cc: Uladzislau Rezki <[email protected]>
> > > Cc: Joel Fernandes <[email protected]>
> > > Cc: Boqun Feng <[email protected]>
> > > Cc: Peter Zijlstra <[email protected]>
> > > Cc: Neeraj Upadhyay <[email protected]>
> > > ---
> > > 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/[email protected]/
> >
> > 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

2022-03-17 06:07:59

by Frederic Weisbecker

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

On Mon, Mar 14, 2022 at 09:34:26PM +0100, Marco Elver wrote:
> On Mon, 14 Mar 2022 at 21:06, Paul E. McKenney <[email protected]> wrote:
> >
> > On Mon, Mar 14, 2022 at 03:44:39PM +0100, Marco Elver wrote:
> > > On Mon, 14 Mar 2022 at 14:37, Frederic Weisbecker <[email protected]> wrote:
> > > >
> > > > From: Valentin Schneider <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > Signed-off-by: Valentin Schneider <[email protected]>
> > > > Signed-off-by: Frederic Weisbecker <[email protected]>
> > > > Cc: Uladzislau Rezki <[email protected]>
> > > > Cc: Joel Fernandes <[email protected]>
> > > > Cc: Boqun Feng <[email protected]>
> > > > Cc: Peter Zijlstra <[email protected]>
> > > > Cc: Neeraj Upadhyay <[email protected]>
> > > > ---
> > > > 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/[email protected]/
> > >
> > > 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.