Re: [PATCH] timers: Exclude isolated cpus from timer migation

[Waiman Long <llong@redhat.com>]

On 4/10/25 2:54 AM, Gabriele Monaco wrote:
> The timer migration mechanism allows timers to move from idle CPUs to
> active 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 isolated and offline;
>
> Keep a cpumap to easily track unavailable cores and change the concept
> of online/offline tmigr to available/unavailable in code and
> tracepoints.
>
> A core is considered unavailable as idle if:
>   * is in the isolcpus list
>   * is in the nohz_full list
>   * is in an isolated cpuset
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>   include/linux/timer.h                  |  6 +++
>   include/trace/events/timer_migration.h |  4 +-
>   kernel/cgroup/cpuset.c                 | 15 +++---
>   kernel/time/timer_migration.c          | 72 +++++++++++++++++++++-----
>   kernel/time/timer_migration.h          |  2 +-
>   5 files changed, 76 insertions(+), 23 deletions(-)
>
> diff --git a/include/linux/timer.h b/include/linux/timer.h
> index 10596d7c3a346..27fb02aa3d780 100644
> --- a/include/linux/timer.h
> +++ b/include/linux/timer.h
> @@ -190,4 +190,10 @@ int timers_dead_cpu(unsigned int cpu);
>   #define timers_dead_cpu		NULL
>   #endif
>   
> +#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)
> +extern int tmigr_isolated_exclude_cpumask(cpumask_var_t exclude_cpumask);
> +#else
> +static inline int tmigr_isolated_exclude_cpumask(cpumask_var_t exclude_cpumask) { }
> +#endif
> +
>   #endif
> diff --git a/include/trace/events/timer_migration.h b/include/trace/events/timer_migration.h
> index 47db5eaf2f9ab..61171b13c687c 100644
> --- a/include/trace/events/timer_migration.h
> +++ b/include/trace/events/timer_migration.h
> @@ -173,14 +173,14 @@ DEFINE_EVENT(tmigr_cpugroup, tmigr_cpu_active,
>   	TP_ARGS(tmc)
>   );
>   
> -DEFINE_EVENT(tmigr_cpugroup, tmigr_cpu_online,
> +DEFINE_EVENT(tmigr_cpugroup, tmigr_cpu_available,
>   
>   	TP_PROTO(struct tmigr_cpu *tmc),
>   
>   	TP_ARGS(tmc)
>   );
>   
> -DEFINE_EVENT(tmigr_cpugroup, tmigr_cpu_offline,
> +DEFINE_EVENT(tmigr_cpugroup, tmigr_cpu_unavailable,
>   
>   	TP_PROTO(struct tmigr_cpu *tmc),
>   
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 306b604300914..47495ba4012b5 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1323,7 +1323,7 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
>   	return isolcpus_updated;
>   }
>   
> -static void update_unbound_workqueue_cpumask(bool isolcpus_updated)
> +static void update_unbound_interference_cpumask(bool isolcpus_updated)

Unbound workqueue is a special workqueue. The name 
"update_unbound_interference_cpumask" doesn't make sense. I would prefer 
you to use names like "update_exclusion_cpumasks" or 
"update_isolated_cpumasks".

Cheers,
Longman