On Thu, 2021-11-11 at 10:36 +0100, Marco Elver wrote: > On Thu, 11 Nov 2021 at 04:47, Mike Galbraith <efault@xxxxxx> wrote: > > > > On Thu, 2021-11-11 at 04:35 +0100, Mike Galbraith wrote: > > > On Thu, 2021-11-11 at 04:16 +0100, Mike Galbraith wrote: > > > > On Wed, 2021-11-10 at 20:24 +0000, Valentin Schneider wrote: > > > > > > > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > > > > index 5f8db54226af..0640d5622496 100644 > > > > > --- a/include/linux/sched.h > > > > > +++ b/include/linux/sched.h > > > > > @@ -2073,6 +2073,22 @@ static inline void cond_resched_rcu(void) > > > > > #endif > > > > > } > > > > > > > > > > +#ifdef CONFIG_PREEMPT_DYNAMIC > > > > > + > > > > > +extern bool is_preempt_none(void); > > > > > +extern bool is_preempt_voluntary(void); > > > > > +extern bool is_preempt_full(void); > > > > > + > > > > > +#else > > > > > + > > > > > +#define is_preempt_none() IS_ENABLED(CONFIG_PREEMPT_NONE) > > > > > +#define is_preempt_voluntary() > > > > > IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY) > > > > > +#define is_preempt_full() IS_ENABLED(CONFIG_PREEMPT) > > > > > > > > I think that should be IS_ENABLED(CONFIG_PREEMPTION), see > > > > c1a280b68d4e. > > > > > > > > Noticed while applying the series to an RT tree, where tglx > > > > has done that replacement to the powerpc spot your next patch > > > > diddles. > > > > > > Damn, then comes patch 5 properly differentiating PREEMPT/PREEMPT_RT. > > > > So I suppose the powerpc spot should remain CONFIG_PREEMPT and become > > CONFIG_PREEMPTION when the RT change gets merged, because that spot is > > about full preemptibility, not a distinct preemption model. > > > > That's rather annoying :-/ > > I guess the question is if is_preempt_full() should be true also if > is_preempt_rt() is true? That's what CONFIG_PREEMPTION is. More could follow, but it was added to allow multiple models to say "preemptible". > Not sure all cases are happy with that, e.g. the kernel/trace/trace.c > case, which wants to print the precise preemption level. Yeah, that's the "annoying" bit, needing one oddball model accessor that isn't about a particular model. > To avoid confusion, I'd introduce another helper that says true if the > preemption level is "at least full", currently that'd be "full or rt". > Something like is_preempt_full_or_rt() (but might as well write > "is_preempt_full() || is_preempt_rt()"), or is_preemption() (to match > that Kconfig variable, although it's slightly confusing). The > implementation of that helper can just be a static inline function > returning "is_preempt_full() || is_preempt_rt()". > > Would that help? Yeah, as it sits two accessors are needed, one that says PREEMPT the other PREEMPTION, spelling optional. -Mike