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.0On 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
© 2016 - 2025 Red Hat, Inc.