Re: [PATCH v6 0/6] timers: Exclude isolated cpus from timer migation

Gabriele Monaco
2025-06-18T12:17:34Z

On Fri, 2025-05-30 at 16:20 +0200, 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 isolated and offline;
>
> 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)
> * the nohz timekeeper core (must be available to handle global
> timers)

Frederic, Thomas, Waiman, would you have time to review this series?
Thanks,
Gabriele

>
> Due to how the timer migration algorithm works, any CPU part of the
> hierarchy can have their global timers pulled by remote CPUs and have
> to
> pull remote timers, only skipping pulling remote timers would break
> the
> logic.
> For this reason, we prevent isolated CPUs from pulling remote global
> timers, but also the other way around: any global timer started on an
> isolated CPU will run there. This does not break the concept of
> isolation (global timers don't come from outside the CPU) and, if
> considered inappropriate, can usually be mitigated with other
> isolation
> techniques (e.g. IRQ pinning).
>
> The first 3 patches are preparatory work to change the concept of
> online/offline to available/unavailable, keep track of those in a
> separate cpumask and change a function name in cpuset code.
>
> Patch 4 and 5 adapt isolation and cpuset to prevent domain isolated
> and
> nohz_full from covering all CPUs not leaving any housekeeping one.
> This
> can lead to problems with the changes introduced in this series
> because
> no CPU would remain to handle global timers.
>
> Patch 6 extends the unavailable status to domain isolated CPUs, which
> is the main contribution of the series.
>
> Changes since v5:
> * Remove fallback if no housekeeping is left by isolcpus and
> nohz_full
> * Adjust condition not to activate CPUs in the migration hierarchy
> * Always force the nohz tick CPU active in the hierarchy
>
> Changes since v4 [1]:
> * use on_each_cpu_mask() with changes on isolated CPUs to avoid races
> * keep nohz_full CPUs included in the timer migration hierarchy
> * prevent domain isolated and nohz_full to cover all CPUs
>
> Changes since v3:
> * add parameter to function documentation
> * split into multiple straightforward patches
>
> Changes since v2:
> * improve comments about handling CPUs isolated at boot
> * minor cleanup
>
> Changes since v1 [2]:
> * split into smaller patches
> * use available mask instead of unavailable
> * simplification and cleanup
>
> [1] -
> https://lore.kernel.org/lkml/20250506091534.42117-7-gmonaco@redhat.com
> [2] -
> https://lore.kernel.org/lkml/20250410065446.57304-2-gmonaco@redhat.com
>
> Gabriele Monaco (6):
>   timers: Rename tmigr 'online' bit to 'available'
>   timers: Add the available mask in timer migration
>   cgroup/cpuset: Rename update_unbound_workqueue_cpumask() to
>     update_exclusion_cpumasks()
>   sched/isolation: Force housekeeping if isolcpus and nohz_full don't
>     leave any
>   cgroup/cpuset: Fail if isolated and nohz_full don't leave any
>     housekeeping
>   timers: Exclude isolated cpus from timer migation
>
>  include/linux/timer.h                  |  9 +++
>  include/trace/events/timer_migration.h |  4 +-
>  kernel/cgroup/cpuset.c                 | 71 +++++++++++++++++++--
>  kernel/sched/isolation.c               | 12 ++++
>  kernel/time/timer_migration.c          | 88 ++++++++++++++++++++++--
> --
>  kernel/time/timer_migration.h          |  2 +-
>  6 files changed, 165 insertions(+), 21 deletions(-)
>
>
> base-commit: 0ff41df1cb268fc69e703a08a57ee14ae967d0ca