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

[Waiman Long <longman@redhat.com>]

On 4/21/26 4:50 AM, Thomas Gleixner wrote:
> 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.
Sure, I will break out the context tracking change into its own patch.
>> +/* 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.

I will work on that.

Cheers,
Longman

>
> Thanks,
>
>          tglx
>
>