Re: [PATCH 04/23] tick/nohz: Allow runtime changes in full dynticks CPUs

[Thomas Gleixner <tglx@kernel.org>]

On Mon, Apr 20 2026 at 23:03, Waiman Long wrote:
> Full dynticks can only be enabled if "nohz_full" boot option has been
> been specified with or without parameter. Any change in the list of
> nohz_full CPUs have to be reflected in tick_nohz_full_mask. Introduce
> a new tick_nohz_full_update_cpus() helper that can be called to update
> the tick_nohz_full_mask at run time. The housekeeping_update() function
> is modified to call the new helper when the HK_TYPE_KERNEL_NOSIE cpumask
> is going to be changed.
>
> We also need to enable CPU context tracking for those CPUs that

We need nothing. Use passive voice for change logs as requested in
documentation.

> are in tick_nohz_full_mask. So remove __init from tick_nohz_init()
> and ct_cpu_track_user() so that they be called later when an isolated
> cpuset partition is being created. The __ro_after_init attribute is
> taken away from context_tracking_key as well.
>
> Also add a new ct_cpu_untrack_user() function to reverse the action of
> ct_cpu_track_user() in case we need to disable the nohz_full mode of
> a CPU.
>
> With nohz_full enabled, the boot CPU (typically CPU 0) will be the
> tick CPU which cannot be shut down easily. So the boot CPU should not
> be used in an isolated cpuset partition.
>
> With runtime modification of nohz_full CPUs, tick_do_timer_cpu can become
> TICK_DO_TIMER_NONE. So remove the two TICK_DO_TIMER_NONE WARN_ON_ONCE()
> checks in tick-sched.c to avoid unnecessary warnings.

in tick-sched.c? Describe the functions which contain that.

>  static inline void tick_nohz_task_switch(void)
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 925999de1a28..394e432630a3 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -411,7 +411,7 @@ static __always_inline void ct_kernel_enter(bool user, int offset) { }
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/context_tracking.h>
>  
> -DEFINE_STATIC_KEY_FALSE_RO(context_tracking_key);
> +DEFINE_STATIC_KEY_FALSE(context_tracking_key);
>  EXPORT_SYMBOL_GPL(context_tracking_key);
>  
>  static noinstr bool context_tracking_recursion_enter(void)
> @@ -674,9 +674,9 @@ void user_exit_callable(void)
>  }
>  NOKPROBE_SYMBOL(user_exit_callable);
>  
> -void __init ct_cpu_track_user(int cpu)
> +void ct_cpu_track_user(int cpu)
>  {
> -	static __initdata bool initialized = false;
> +	static bool initialized;
>  
>  	if (cpu == CONTEXT_TRACKING_FORCE_ENABLE) {
>  		static_branch_inc(&context_tracking_key);
> @@ -700,6 +700,15 @@ void __init ct_cpu_track_user(int cpu)
>  	initialized = true;
>  }
>  
> +void ct_cpu_untrack_user(int cpu)
> +{
> +	if (!per_cpu(context_tracking.active, cpu))
> +		return;
> +
> +	per_cpu(context_tracking.active, cpu) = false;
> +	static_branch_dec(&context_tracking_key);
> +}
> +

Why is this in a patch which makes tick/nohz related changes? This is a
preparatory change, so make it that way and do not bury it inside
something else.

> +/* Get the new set of run-time nohz CPU list & update accordingly */
> +void tick_nohz_full_update_cpus(struct cpumask *cpumask)
> +{
> +	int cpu;
> +
> +	if (!tick_nohz_full_running) {
> +		pr_warn_once("Full dynticks cannot be enabled without the nohz_full kernel boot parameter!\n");

That's the result of this enforced enable hackery. Make this work
properly.

> +		return;
> +	}
> +
> +	/*
> +	 * To properly enable/disable nohz_full dynticks for the affected CPUs,
> +	 * the new nohz_full CPUs have to be copied to tick_nohz_full_mask and
> +	 * ct_cpu_track_user/ct_cpu_untrack_user() will have to be called
> +	 * for those CPUs that have their states changed. Those CPUs should be
> +	 * in an offline state.
> +	 */
> +	for_each_cpu_andnot(cpu, cpumask, tick_nohz_full_mask) {
> +		WARN_ON_ONCE(cpu_online(cpu));
> +		ct_cpu_track_user(cpu);
> +		cpumask_set_cpu(cpu, tick_nohz_full_mask);
> +	}
> +
> +	for_each_cpu_andnot(cpu, tick_nohz_full_mask, cpumask) {
> +		WARN_ON_ONCE(cpu_online(cpu));
> +		ct_cpu_untrack_user(cpu);
> +		cpumask_clear_cpu(cpu, tick_nohz_full_mask);
> +	}
> +}

So this writes to tick_nohz_full_mask while other CPUs can access
it. That's just wrong and I'm not at all interested in the resulting
KCSAN warnings.

tick_nohz_full_mask needs to become a RCU protected pointer, which is
updated once the new mask is established in a separately allocated one.

Thanks,

        tglx