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;
+EXPORT_SYMBOL_GPL(housekeeping_flags);
 
 bool housekeeping_enabled(enum hk_type type)
 {
-	return !!(housekeeping.flags & BIT(type));
+	return !!(housekeeping_flags & BIT(type));
 }
 EXPORT_SYMBOL_GPL(housekeeping_enabled);
 
@@ -36,50 +30,46 @@ int housekeeping_any_cpu(enum hk_type type)
 {
 	int cpu;
 
-	if (static_branch_unlikely(&housekeeping_overridden)) {
-		if (housekeeping.flags & BIT(type)) {
-			cpu = sched_numa_find_closest(housekeeping.cpumasks[type], smp_processor_id());
-			if (cpu < nr_cpu_ids)
-				return cpu;
+	if (housekeeping_flags & BIT(type)) {
+		cpu = sched_numa_find_closest(housekeeping_cpumasks[type], smp_processor_id());
+		if (cpu < nr_cpu_ids)
+			return cpu;
 
-			cpu = cpumask_any_and_distribute(housekeeping.cpumasks[type], cpu_online_mask);
-			if (likely(cpu < nr_cpu_ids))
-				return cpu;
-			/*
-			 * Unless we have another problem this can only happen
-			 * at boot time before start_secondary() brings the 1st
-			 * housekeeping CPU up.
-			 */
-			WARN_ON_ONCE(system_state == SYSTEM_RUNNING ||
-				     type != HK_TYPE_TIMER);
-		}
+		cpu = cpumask_any_and_distribute(housekeeping_cpumasks[type], cpu_online_mask);
+		if (likely(cpu < nr_cpu_ids))
+			return cpu;
+		/*
+		 * Unless we have another problem this can only happen
+		 * at boot time before start_secondary() brings the 1st
+		 * housekeeping CPU up.
+		 */
+		WARN_ON_ONCE(system_state == SYSTEM_RUNNING ||
+			     type != HK_TYPE_TIMER);
 	}
+
 	return smp_processor_id();
 }
 EXPORT_SYMBOL_GPL(housekeeping_any_cpu);
 
 const struct cpumask *housekeeping_cpumask(enum hk_type type)
 {
-	if (static_branch_unlikely(&housekeeping_overridden))
-		if (housekeeping.flags & BIT(type))
-			return housekeeping.cpumasks[type];
+	if (housekeeping_flags & BIT(type))
+		return housekeeping_cpumasks[type];
 	return cpu_possible_mask;
 }
 EXPORT_SYMBOL_GPL(housekeeping_cpumask);
 
 void housekeeping_affine(struct task_struct *t, enum hk_type type)
 {
-	if (static_branch_unlikely(&housekeeping_overridden))
-		if (housekeeping.flags & BIT(type))
-			set_cpus_allowed_ptr(t, housekeeping.cpumasks[type]);
+	if (housekeeping_flags & BIT(type))
+		set_cpus_allowed_ptr(t, housekeeping_cpumasks[type]);
 }
 EXPORT_SYMBOL_GPL(housekeeping_affine);
 
 bool housekeeping_test_cpu(int cpu, enum hk_type type)
 {
-	if (static_branch_unlikely(&housekeeping_overridden))
-		if (housekeeping.flags & BIT(type))
-			return cpumask_test_cpu(cpu, housekeeping.cpumasks[type]);
+	if (housekeeping_flags & BIT(type))
+		return cpumask_test_cpu(cpu, housekeeping_cpumasks[type]);
 	return true;
 }
 EXPORT_SYMBOL_GPL(housekeeping_test_cpu);
@@ -88,17 +78,15 @@ void __init housekeeping_init(void)
 {
 	enum hk_type type;
 
-	if (!housekeeping.flags)
+	if (!housekeeping_flags)
 		return;
 
-	static_branch_enable(&housekeeping_overridden);
-
-	if (housekeeping.flags & HK_FLAG_KERNEL_NOISE)
+	if (housekeeping_flags & HK_FLAG_KERNEL_NOISE)
 		sched_tick_offload_init();
 
-	for_each_set_bit(type, &housekeeping.flags, HK_TYPE_MAX) {
+	for_each_set_bit(type, &housekeeping_flags, HK_TYPE_MAX) {
 		/* We need at least one CPU to handle housekeeping work */
-		WARN_ON_ONCE(cpumask_empty(housekeeping.cpumasks[type]));
+		WARN_ON_ONCE(cpumask_empty(housekeeping_cpumasks[type]));
 	}
 }
 
@@ -106,8 +94,8 @@ static void __init housekeeping_setup_type(enum hk_type type,
 					   cpumask_var_t housekeeping_staging)
 {
 
-	alloc_bootmem_cpumask_var(&housekeeping.cpumasks[type]);
-	cpumask_copy(housekeeping.cpumasks[type],
+	alloc_bootmem_cpumask_var(&housekeeping_cpumasks[type]);
+	cpumask_copy(housekeeping_cpumasks[type],
 		     housekeeping_staging);
 }
 
@@ -117,7 +105,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
 	unsigned int first_cpu;
 	int err = 0;
 
-	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping.flags & HK_FLAG_KERNEL_NOISE)) {
+	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping_flags & HK_FLAG_KERNEL_NOISE)) {
 		if (!IS_ENABLED(CONFIG_NO_HZ_FULL)) {
 			pr_warn("Housekeeping: nohz unsupported."
 				" Build with CONFIG_NO_HZ_FULL\n");
@@ -139,7 +127,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
 	if (first_cpu >= nr_cpu_ids || first_cpu >= setup_max_cpus) {
 		__cpumask_set_cpu(smp_processor_id(), housekeeping_staging);
 		__cpumask_clear_cpu(smp_processor_id(), non_housekeeping_mask);
-		if (!housekeeping.flags) {
+		if (!housekeeping_flags) {
 			pr_warn("Housekeeping: must include one present CPU, "
 				"using boot CPU:%d\n", smp_processor_id());
 		}
@@ -148,7 +136,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
 	if (cpumask_empty(non_housekeeping_mask))
 		goto free_housekeeping_staging;
 
-	if (!housekeeping.flags) {
+	if (!housekeeping_flags) {
 		/* First setup call ("nohz_full=" or "isolcpus=") */
 		enum hk_type type;
 
@@ -157,26 +145,26 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
 	} else {
 		/* Second setup call ("nohz_full=" after "isolcpus=" or the reverse) */
 		enum hk_type type;
-		unsigned long iter_flags = flags & housekeeping.flags;
+		unsigned long iter_flags = flags & housekeeping_flags;
 
 		for_each_set_bit(type, &iter_flags, HK_TYPE_MAX) {
 			if (!cpumask_equal(housekeeping_staging,
-					   housekeeping.cpumasks[type])) {
+					   housekeeping_cpumasks[type])) {
 				pr_warn("Housekeeping: nohz_full= must match isolcpus=\n");
 				goto free_housekeeping_staging;
 			}
 		}
 
-		iter_flags = flags & ~housekeeping.flags;
+		iter_flags = flags & ~housekeeping_flags;
 
 		for_each_set_bit(type, &iter_flags, HK_TYPE_MAX)
 			housekeeping_setup_type(type, housekeeping_staging);
 	}
 
-	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping.flags & HK_FLAG_KERNEL_NOISE))
+	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping_flags & HK_FLAG_KERNEL_NOISE))
 		tick_nohz_full_setup(non_housekeeping_mask);
 
-	housekeeping.flags |= flags;
+	housekeeping_flags |= flags;
 	err = 1;
 
 free_housekeeping_staging:
-- 
2.51.0

On Fri, Aug 29, 2025 at 05:47:42PM +0200 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>

This makes sense and LGTM,

Reviewed-by: Phil Auld <pauld@redhat.com>


Cheers,
Phil

> ---
>  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;
> +EXPORT_SYMBOL_GPL(housekeeping_flags);
>  
>  bool housekeeping_enabled(enum hk_type type)
>  {
> -	return !!(housekeeping.flags & BIT(type));
> +	return !!(housekeeping_flags & BIT(type));
>  }
>  EXPORT_SYMBOL_GPL(housekeeping_enabled);
>  
> @@ -36,50 +30,46 @@ int housekeeping_any_cpu(enum hk_type type)
>  {
>  	int cpu;
>  
> -	if (static_branch_unlikely(&housekeeping_overridden)) {
> -		if (housekeeping.flags & BIT(type)) {
> -			cpu = sched_numa_find_closest(housekeeping.cpumasks[type], smp_processor_id());
> -			if (cpu < nr_cpu_ids)
> -				return cpu;
> +	if (housekeeping_flags & BIT(type)) {
> +		cpu = sched_numa_find_closest(housekeeping_cpumasks[type], smp_processor_id());
> +		if (cpu < nr_cpu_ids)
> +			return cpu;
>  
> -			cpu = cpumask_any_and_distribute(housekeeping.cpumasks[type], cpu_online_mask);
> -			if (likely(cpu < nr_cpu_ids))
> -				return cpu;
> -			/*
> -			 * Unless we have another problem this can only happen
> -			 * at boot time before start_secondary() brings the 1st
> -			 * housekeeping CPU up.
> -			 */
> -			WARN_ON_ONCE(system_state == SYSTEM_RUNNING ||
> -				     type != HK_TYPE_TIMER);
> -		}
> +		cpu = cpumask_any_and_distribute(housekeeping_cpumasks[type], cpu_online_mask);
> +		if (likely(cpu < nr_cpu_ids))
> +			return cpu;
> +		/*
> +		 * Unless we have another problem this can only happen
> +		 * at boot time before start_secondary() brings the 1st
> +		 * housekeeping CPU up.
> +		 */
> +		WARN_ON_ONCE(system_state == SYSTEM_RUNNING ||
> +			     type != HK_TYPE_TIMER);
>  	}
> +
>  	return smp_processor_id();
>  }
>  EXPORT_SYMBOL_GPL(housekeeping_any_cpu);
>  
>  const struct cpumask *housekeeping_cpumask(enum hk_type type)
>  {
> -	if (static_branch_unlikely(&housekeeping_overridden))
> -		if (housekeeping.flags & BIT(type))
> -			return housekeeping.cpumasks[type];
> +	if (housekeeping_flags & BIT(type))
> +		return housekeeping_cpumasks[type];
>  	return cpu_possible_mask;
>  }
>  EXPORT_SYMBOL_GPL(housekeeping_cpumask);
>  
>  void housekeeping_affine(struct task_struct *t, enum hk_type type)
>  {
> -	if (static_branch_unlikely(&housekeeping_overridden))
> -		if (housekeeping.flags & BIT(type))
> -			set_cpus_allowed_ptr(t, housekeeping.cpumasks[type]);
> +	if (housekeeping_flags & BIT(type))
> +		set_cpus_allowed_ptr(t, housekeeping_cpumasks[type]);
>  }
>  EXPORT_SYMBOL_GPL(housekeeping_affine);
>  
>  bool housekeeping_test_cpu(int cpu, enum hk_type type)
>  {
> -	if (static_branch_unlikely(&housekeeping_overridden))
> -		if (housekeeping.flags & BIT(type))
> -			return cpumask_test_cpu(cpu, housekeeping.cpumasks[type]);
> +	if (housekeeping_flags & BIT(type))
> +		return cpumask_test_cpu(cpu, housekeeping_cpumasks[type]);
>  	return true;
>  }
>  EXPORT_SYMBOL_GPL(housekeeping_test_cpu);
> @@ -88,17 +78,15 @@ void __init housekeeping_init(void)
>  {
>  	enum hk_type type;
>  
> -	if (!housekeeping.flags)
> +	if (!housekeeping_flags)
>  		return;
>  
> -	static_branch_enable(&housekeeping_overridden);
> -
> -	if (housekeeping.flags & HK_FLAG_KERNEL_NOISE)
> +	if (housekeeping_flags & HK_FLAG_KERNEL_NOISE)
>  		sched_tick_offload_init();
>  
> -	for_each_set_bit(type, &housekeeping.flags, HK_TYPE_MAX) {
> +	for_each_set_bit(type, &housekeeping_flags, HK_TYPE_MAX) {
>  		/* We need at least one CPU to handle housekeeping work */
> -		WARN_ON_ONCE(cpumask_empty(housekeeping.cpumasks[type]));
> +		WARN_ON_ONCE(cpumask_empty(housekeeping_cpumasks[type]));
>  	}
>  }
>  
> @@ -106,8 +94,8 @@ static void __init housekeeping_setup_type(enum hk_type type,
>  					   cpumask_var_t housekeeping_staging)
>  {
>  
> -	alloc_bootmem_cpumask_var(&housekeeping.cpumasks[type]);
> -	cpumask_copy(housekeeping.cpumasks[type],
> +	alloc_bootmem_cpumask_var(&housekeeping_cpumasks[type]);
> +	cpumask_copy(housekeeping_cpumasks[type],
>  		     housekeeping_staging);
>  }
>  
> @@ -117,7 +105,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
>  	unsigned int first_cpu;
>  	int err = 0;
>  
> -	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping.flags & HK_FLAG_KERNEL_NOISE)) {
> +	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping_flags & HK_FLAG_KERNEL_NOISE)) {
>  		if (!IS_ENABLED(CONFIG_NO_HZ_FULL)) {
>  			pr_warn("Housekeeping: nohz unsupported."
>  				" Build with CONFIG_NO_HZ_FULL\n");
> @@ -139,7 +127,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
>  	if (first_cpu >= nr_cpu_ids || first_cpu >= setup_max_cpus) {
>  		__cpumask_set_cpu(smp_processor_id(), housekeeping_staging);
>  		__cpumask_clear_cpu(smp_processor_id(), non_housekeeping_mask);
> -		if (!housekeeping.flags) {
> +		if (!housekeeping_flags) {
>  			pr_warn("Housekeeping: must include one present CPU, "
>  				"using boot CPU:%d\n", smp_processor_id());
>  		}
> @@ -148,7 +136,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
>  	if (cpumask_empty(non_housekeeping_mask))
>  		goto free_housekeeping_staging;
>  
> -	if (!housekeeping.flags) {
> +	if (!housekeeping_flags) {
>  		/* First setup call ("nohz_full=" or "isolcpus=") */
>  		enum hk_type type;
>  
> @@ -157,26 +145,26 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
>  	} else {
>  		/* Second setup call ("nohz_full=" after "isolcpus=" or the reverse) */
>  		enum hk_type type;
> -		unsigned long iter_flags = flags & housekeeping.flags;
> +		unsigned long iter_flags = flags & housekeeping_flags;
>  
>  		for_each_set_bit(type, &iter_flags, HK_TYPE_MAX) {
>  			if (!cpumask_equal(housekeeping_staging,
> -					   housekeeping.cpumasks[type])) {
> +					   housekeeping_cpumasks[type])) {
>  				pr_warn("Housekeeping: nohz_full= must match isolcpus=\n");
>  				goto free_housekeeping_staging;
>  			}
>  		}
>  
> -		iter_flags = flags & ~housekeeping.flags;
> +		iter_flags = flags & ~housekeeping_flags;
>  
>  		for_each_set_bit(type, &iter_flags, HK_TYPE_MAX)
>  			housekeeping_setup_type(type, housekeeping_staging);
>  	}
>  
> -	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping.flags & HK_FLAG_KERNEL_NOISE))
> +	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping_flags & HK_FLAG_KERNEL_NOISE))
>  		tick_nohz_full_setup(non_housekeeping_mask);
>  
> -	housekeeping.flags |= flags;
> +	housekeeping_flags |= flags;
>  	err = 1;
>  
>  free_housekeeping_staging:
> -- 
> 2.51.0
> 
> 

--

On Fri, Aug 29, 2025 at 05:47:42PM +0200, Frederic Weisbecker wrote:

> +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;
> +}

That 'else' is superfluous.

> -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;
> +EXPORT_SYMBOL_GPL(housekeeping_flags);

I don't particularly like exporting variables much. It means modules can
actually change the value and things like that.

And while an exported static_key can be changed by modules, that's
fixable.

Le Mon, Sep 01, 2025 at 12:26:42PM +0200, Peter Zijlstra a écrit :
> On Fri, Aug 29, 2025 at 05:47:42PM +0200, Frederic Weisbecker wrote:
> 
> > +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;
> > +}
> 
> That 'else' is superfluous.
> 
> > -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;
> > +EXPORT_SYMBOL_GPL(housekeeping_flags);
> 
> I don't particularly like exporting variables much. It means modules can
> actually change the value and things like that.

Can't say I'm confortable myself.

> 
> And while an exported static_key can be changed by modules, that's
> fixable.

Hm, ok I think I can keep it.

My only worry was that in such a situation:

    CPU 0                              CPU 1
    -----                              -----
    rcu_read_lock()                   static_key_enable(&key)
    if (static_key_unlikely(&key))    synchronize_rcu()
        do_something()
    rcu_read_unlock()

The static_key_unlikely evaluation is really part of that RCU
reader sequence block. I must count on the fact that the (un-)patched
instruction is well contained within the rcu_read_lock() / rcu_read_unlock().

I think it should be the case.

Also I see that, at least on x86, static_key_enable() eventually ends up
into sync_core() broadcast IPIs. And that followed by synchronize_rcu()
should make sure that the whole block sequence from past readers should be done.

So yes I think I can try to keep that static key.

Thanks.

-- 
Frederic Weisbecker
SUSE Labs

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

Cheers,
Longman

> +EXPORT_SYMBOL_GPL(housekeeping_flags);
>   
>   bool housekeeping_enabled(enum hk_type type)
>   {
> -	return !!(housekeeping.flags & BIT(type));
> +	return !!(housekeeping_flags & BIT(type));
>   }
>   EXPORT_SYMBOL_GPL(housekeeping_enabled);
>   
> @@ -36,50 +30,46 @@ int housekeeping_any_cpu(enum hk_type type)
>   {
>   	int cpu;
>   
> -	if (static_branch_unlikely(&housekeeping_overridden)) {
> -		if (housekeeping.flags & BIT(type)) {
> -			cpu = sched_numa_find_closest(housekeeping.cpumasks[type], smp_processor_id());
> -			if (cpu < nr_cpu_ids)
> -				return cpu;
> +	if (housekeeping_flags & BIT(type)) {
> +		cpu = sched_numa_find_closest(housekeeping_cpumasks[type], smp_processor_id());
> +		if (cpu < nr_cpu_ids)
> +			return cpu;
>   
> -			cpu = cpumask_any_and_distribute(housekeeping.cpumasks[type], cpu_online_mask);
> -			if (likely(cpu < nr_cpu_ids))
> -				return cpu;
> -			/*
> -			 * Unless we have another problem this can only happen
> -			 * at boot time before start_secondary() brings the 1st
> -			 * housekeeping CPU up.
> -			 */
> -			WARN_ON_ONCE(system_state == SYSTEM_RUNNING ||
> -				     type != HK_TYPE_TIMER);
> -		}
> +		cpu = cpumask_any_and_distribute(housekeeping_cpumasks[type], cpu_online_mask);
> +		if (likely(cpu < nr_cpu_ids))
> +			return cpu;
> +		/*
> +		 * Unless we have another problem this can only happen
> +		 * at boot time before start_secondary() brings the 1st
> +		 * housekeeping CPU up.
> +		 */
> +		WARN_ON_ONCE(system_state == SYSTEM_RUNNING ||
> +			     type != HK_TYPE_TIMER);
>   	}
> +
>   	return smp_processor_id();
>   }
>   EXPORT_SYMBOL_GPL(housekeeping_any_cpu);
>   
>   const struct cpumask *housekeeping_cpumask(enum hk_type type)
>   {
> -	if (static_branch_unlikely(&housekeeping_overridden))
> -		if (housekeeping.flags & BIT(type))
> -			return housekeeping.cpumasks[type];
> +	if (housekeeping_flags & BIT(type))
> +		return housekeeping_cpumasks[type];
>   	return cpu_possible_mask;
>   }
>   EXPORT_SYMBOL_GPL(housekeeping_cpumask);
>   
>   void housekeeping_affine(struct task_struct *t, enum hk_type type)
>   {
> -	if (static_branch_unlikely(&housekeeping_overridden))
> -		if (housekeeping.flags & BIT(type))
> -			set_cpus_allowed_ptr(t, housekeeping.cpumasks[type]);
> +	if (housekeeping_flags & BIT(type))
> +		set_cpus_allowed_ptr(t, housekeeping_cpumasks[type]);
>   }
>   EXPORT_SYMBOL_GPL(housekeeping_affine);
>   
>   bool housekeeping_test_cpu(int cpu, enum hk_type type)
>   {
> -	if (static_branch_unlikely(&housekeeping_overridden))
> -		if (housekeeping.flags & BIT(type))
> -			return cpumask_test_cpu(cpu, housekeeping.cpumasks[type]);
> +	if (housekeeping_flags & BIT(type))
> +		return cpumask_test_cpu(cpu, housekeeping_cpumasks[type]);
>   	return true;
>   }
>   EXPORT_SYMBOL_GPL(housekeeping_test_cpu);
> @@ -88,17 +78,15 @@ void __init housekeeping_init(void)
>   {
>   	enum hk_type type;
>   
> -	if (!housekeeping.flags)
> +	if (!housekeeping_flags)
>   		return;
>   
> -	static_branch_enable(&housekeeping_overridden);
> -
> -	if (housekeeping.flags & HK_FLAG_KERNEL_NOISE)
> +	if (housekeeping_flags & HK_FLAG_KERNEL_NOISE)
>   		sched_tick_offload_init();
>   
> -	for_each_set_bit(type, &housekeeping.flags, HK_TYPE_MAX) {
> +	for_each_set_bit(type, &housekeeping_flags, HK_TYPE_MAX) {
>   		/* We need at least one CPU to handle housekeeping work */
> -		WARN_ON_ONCE(cpumask_empty(housekeeping.cpumasks[type]));
> +		WARN_ON_ONCE(cpumask_empty(housekeeping_cpumasks[type]));
>   	}
>   }
>   
> @@ -106,8 +94,8 @@ static void __init housekeeping_setup_type(enum hk_type type,
>   					   cpumask_var_t housekeeping_staging)
>   {
>   
> -	alloc_bootmem_cpumask_var(&housekeeping.cpumasks[type]);
> -	cpumask_copy(housekeeping.cpumasks[type],
> +	alloc_bootmem_cpumask_var(&housekeeping_cpumasks[type]);
> +	cpumask_copy(housekeeping_cpumasks[type],
>   		     housekeeping_staging);
>   }
>   
> @@ -117,7 +105,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
>   	unsigned int first_cpu;
>   	int err = 0;
>   
> -	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping.flags & HK_FLAG_KERNEL_NOISE)) {
> +	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping_flags & HK_FLAG_KERNEL_NOISE)) {
>   		if (!IS_ENABLED(CONFIG_NO_HZ_FULL)) {
>   			pr_warn("Housekeeping: nohz unsupported."
>   				" Build with CONFIG_NO_HZ_FULL\n");
> @@ -139,7 +127,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
>   	if (first_cpu >= nr_cpu_ids || first_cpu >= setup_max_cpus) {
>   		__cpumask_set_cpu(smp_processor_id(), housekeeping_staging);
>   		__cpumask_clear_cpu(smp_processor_id(), non_housekeeping_mask);
> -		if (!housekeeping.flags) {
> +		if (!housekeeping_flags) {
>   			pr_warn("Housekeeping: must include one present CPU, "
>   				"using boot CPU:%d\n", smp_processor_id());
>   		}
> @@ -148,7 +136,7 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
>   	if (cpumask_empty(non_housekeeping_mask))
>   		goto free_housekeeping_staging;
>   
> -	if (!housekeeping.flags) {
> +	if (!housekeeping_flags) {
>   		/* First setup call ("nohz_full=" or "isolcpus=") */
>   		enum hk_type type;
>   
> @@ -157,26 +145,26 @@ static int __init housekeeping_setup(char *str, unsigned long flags)
>   	} else {
>   		/* Second setup call ("nohz_full=" after "isolcpus=" or the reverse) */
>   		enum hk_type type;
> -		unsigned long iter_flags = flags & housekeeping.flags;
> +		unsigned long iter_flags = flags & housekeeping_flags;
>   
>   		for_each_set_bit(type, &iter_flags, HK_TYPE_MAX) {
>   			if (!cpumask_equal(housekeeping_staging,
> -					   housekeeping.cpumasks[type])) {
> +					   housekeeping_cpumasks[type])) {
>   				pr_warn("Housekeeping: nohz_full= must match isolcpus=\n");
>   				goto free_housekeeping_staging;
>   			}
>   		}
>   
> -		iter_flags = flags & ~housekeeping.flags;
> +		iter_flags = flags & ~housekeeping_flags;
>   
>   		for_each_set_bit(type, &iter_flags, HK_TYPE_MAX)
>   			housekeeping_setup_type(type, housekeeping_staging);
>   	}
>   
> -	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping.flags & HK_FLAG_KERNEL_NOISE))
> +	if ((flags & HK_FLAG_KERNEL_NOISE) && !(housekeeping_flags & HK_FLAG_KERNEL_NOISE))
>   		tick_nohz_full_setup(non_housekeeping_mask);
>   
> -	housekeeping.flags |= flags;
> +	housekeeping_flags |= flags;
>   	err = 1;
>   
>   free_housekeeping_staging:

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