Re: [PATCH 01/33] sched/isolation: Remove housekeeping static key

[Frederic Weisbecker <frederic@kernel.org>]

Le Fri, Aug 29, 2025 at 05:34:55PM -0400, Waiman Long a écrit :
> 
> On 8/29/25 11:47 AM, Frederic Weisbecker wrote:
> > The housekeeping static key in its current use is mostly irrelevant.
> > Most of the time, a housekeeping function call had already been issued
> > before the static call got a chance to be evaluated, defeating the
> > initial call optimization purpose.
> > 
> > housekeeping_cpu() is the sole correct user performing the static call
> > before the actual slow-path function call. But it's seldom used in
> > fast-path.
> > 
> > Finally the static call prevents from synchronizing correctly against
> > dynamic updates of the housekeeping cpumasks through cpusets.
> > 
> > Get away with a simple flag test instead.
> > 
> > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > ---
> >   include/linux/sched/isolation.h | 25 +++++----
> >   kernel/sched/isolation.c        | 90 ++++++++++++++-------------------
> >   2 files changed, 55 insertions(+), 60 deletions(-)
> > 
> > diff --git a/include/linux/sched/isolation.h b/include/linux/sched/isolation.h
> > index d8501f4709b5..f98ba0d71c52 100644
> > --- a/include/linux/sched/isolation.h
> > +++ b/include/linux/sched/isolation.h
> > @@ -25,12 +25,22 @@ enum hk_type {
> >   };
> >   #ifdef CONFIG_CPU_ISOLATION
> > -DECLARE_STATIC_KEY_FALSE(housekeeping_overridden);
> > +extern unsigned long housekeeping_flags;
> > +
> >   extern int housekeeping_any_cpu(enum hk_type type);
> >   extern const struct cpumask *housekeeping_cpumask(enum hk_type type);
> >   extern bool housekeeping_enabled(enum hk_type type);
> >   extern void housekeeping_affine(struct task_struct *t, enum hk_type type);
> >   extern bool housekeeping_test_cpu(int cpu, enum hk_type type);
> > +
> > +static inline bool housekeeping_cpu(int cpu, enum hk_type type)
> > +{
> > +	if (housekeeping_flags & BIT(type))
> > +		return housekeeping_test_cpu(cpu, type);
> > +	else
> > +		return true;
> > +}
> > +
> >   extern void __init housekeeping_init(void);
> >   #else
> > @@ -58,17 +68,14 @@ static inline bool housekeeping_test_cpu(int cpu, enum hk_type type)
> >   	return true;
> >   }
> > +static inline bool housekeeping_cpu(int cpu, enum hk_type type)
> > +{
> > +	return true;
> > +}
> > +
> >   static inline void housekeeping_init(void) { }
> >   #endif /* CONFIG_CPU_ISOLATION */
> > -static inline bool housekeeping_cpu(int cpu, enum hk_type type)
> > -{
> > -#ifdef CONFIG_CPU_ISOLATION
> > -	if (static_branch_unlikely(&housekeeping_overridden))
> > -		return housekeeping_test_cpu(cpu, type);
> > -#endif
> > -	return true;
> > -}
> >   static inline bool cpu_is_isolated(int cpu)
> >   {
> > diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> > index a4cf17b1fab0..2a6fc6fc46fb 100644
> > --- a/kernel/sched/isolation.c
> > +++ b/kernel/sched/isolation.c
> > @@ -16,19 +16,13 @@ enum hk_flags {
> >   	HK_FLAG_KERNEL_NOISE	= BIT(HK_TYPE_KERNEL_NOISE),
> >   };
> > -DEFINE_STATIC_KEY_FALSE(housekeeping_overridden);
> > -EXPORT_SYMBOL_GPL(housekeeping_overridden);
> > -
> > -struct housekeeping {
> > -	cpumask_var_t cpumasks[HK_TYPE_MAX];
> > -	unsigned long flags;
> > -};
> > -
> > -static struct housekeeping housekeeping;
> > +static cpumask_var_t housekeeping_cpumasks[HK_TYPE_MAX];
> > +unsigned long housekeeping_flags;
> 
> Should we add the__read_mostly attribute to housekeeping_flags to prevent
> possible false cacheline sharing problem?
> 
> Other than that, LGTM

Makes sense.

Thanks.

-- 
Frederic Weisbecker
SUSE Labs