Re: [PATCH v9 8/8] timers: Exclude isolated cpus from timer migration

[Waiman Long <llong@redhat.com>]

On 8/1/25 9:07 AM, Frederic Weisbecker wrote:
> Le Thu, Jul 31, 2025 at 02:25:30PM -0400, Waiman Long a écrit :
>> On 7/30/25 9:11 AM, Gabriele Monaco wrote:
>>> The timer migration mechanism allows active CPUs to pull timers from
>>> idle ones to improve the overall idle time. This is however undesired
>>> when CPU intensive workloads run on isolated cores, as the algorithm
>>> would move the timers from housekeeping to isolated cores, negatively
>>> affecting the isolation.
>>>
>>> Exclude isolated cores from the timer migration algorithm, extend the
>>> concept of unavailable cores, currently used for offline ones, to
>>> isolated ones:
>>> * A core is unavailable if isolated or offline;
>>> * A core is available if non isolated and online;
>>>
>>> A core is considered unavailable as isolated if it belongs to:
>>> * the isolcpus (domain) list
>>> * an isolated cpuset
>>> Except if it is:
>>> * in the nohz_full list (already idle for the hierarchy)
>> For the nohz_full list here, do you mean nohz_full housekeeping or
>> non-housekeeping list?
> nohz_full.
>
>>> @@ -436,6 +437,20 @@ static inline bool tmigr_is_not_available(struct tmigr_cpu *tmc)
>>>    	return !(tmc->tmgroup && tmc->available);
>>>    }
>>> +/*
>>> + * Returns true if @cpu should be excluded from the hierarchy as isolated.
>>> + * Domain isolated CPUs don't participate in timer migration, nohz_full
>>> + * CPUs are still part of the hierarchy but are always considered idle.
>>> + * This check is necessary, for instance, to prevent offline isolated CPU from
>>> + * being incorrectly marked as available once getting back online.
>>> + */
>>> +static inline bool tmigr_is_isolated(int cpu)
>>> +{
>>> +	return (!housekeeping_cpu(cpu, HK_TYPE_DOMAIN) ||
>>> +		cpuset_cpu_is_isolated(cpu)) &&
>>> +	       housekeeping_cpu(cpu, HK_TYPE_KERNEL_NOISE);
>>> +}
>> Does that mean a CPU in the nohz_full non-housekeeping list is always
>> considered not isolated WRT timer migration and hence will be made available
>> for timer migration purpose?
> Exactly, because nohz_full CPUs become idle (from a tick and timer migration
> POV) when they stop their tick. And since they are idle, their global timer
> are handled by the timekeeping CPU.
>
> This is much better than making the CPU unavailable like is done in this
> patchset for domain isolated CPUs, because unavailable CPUs must still handle
> their own global timers. Unfortunately we can't just fake them as well as idle,
> like we do with nohz_full CPUs, because that would mean walking the whole timer
> migration tree everytime a timer is queued or modified. This would be too
> costly.
>
> Indeed that should be commented somewhere in this function.

Thanks for the clarification. Yes, I agree that we should better 
document it in the code as it is not clear why we are doing that.

Cheers,
Longman