[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f816b9cd-7797-4b70-9899-d2f56a0b785e@redhat.com>
Date: Thu, 10 Apr 2025 10:32:04 -0400
From: Waiman Long <llong@...hat.com>
To: Gabriele Monaco <gmonaco@...hat.com>, linux-kernel@...r.kernel.org,
Anna-Maria Behnsen <anna-maria@...utronix.de>,
Frederic Weisbecker <frederic@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] timers: Exclude isolated cpus from timer migation
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@...hat.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
Powered by blists - more mailing lists