From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0412A2698AF for ; Wed, 19 Nov 2025 20:43:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763585021; cv=none; b=VG6DwWDH0tlJ75wI5/dA2DNQ5hpw7WiJY7VinXL6AP4TaLBYAkMOVazT+Vd39tLsYethZkrOjFBZ5jMgYmtZlODYE5EjyQsxBQlIBFiNIImNqURHDazXT/H5PDjKs2HijhkibBGjnIvQYltX5eywoHPWiQhsGlG1eU1ZzdwinAc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763585021; c=relaxed/simple; bh=D2u9scS0jjDpX3X8pKHwCb8Pl4OCBEpqpljbZYJPhRU=; h=From:Message-ID:Date:MIME-Version:Subject:To:Cc:References: In-Reply-To:Content-Type; b=hTle4jsY4l/DJ1Xw1oW396/dKAd3psclShpcUr3/BGFbwxF+0NIedA8AJbv9VIKB55dAQL8KIBvzWdHXDZZStaMU5Fccev52dHnSYYLVdoj0/xZTDx5n7eJgIbDVPs3SB76gZOaPNXNHaHawOJR2lA/HUTxSIHIOb7FF0BoARGE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=RK168hm/; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=HBErS4Yg; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="RK168hm/"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="HBErS4Yg" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1763585018; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KBCrKnuu+3+9mT93IOVCPLlu0tAgg9wTYBOooqnzqN4=; b=RK168hm/5pi8j1N9rjJLgv3k24dSQzUqSRdvZV8MjRWCEVRNwOpTSPx8HY/6aUUtHdf3k8 MQikre/BpbxNkha/YudKvbE8gMRiwqFfNCo6q+IF8r2Pt7HhmfUskoCC6KZirAI8rHmN0N 8P/MRpf05gfxOHG+YbG4hNqg929li14= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-235-JaabPC4rPS-BzIG7Ey1SMw-1; Wed, 19 Nov 2025 15:43:36 -0500 X-MC-Unique: JaabPC4rPS-BzIG7Ey1SMw-1 X-Mimecast-MFC-AGG-ID: JaabPC4rPS-BzIG7Ey1SMw_1763585016 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-8b24b811fb1so40644785a.1 for ; Wed, 19 Nov 2025 12:43:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1763585016; x=1764189816; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:user-agent:mime-version:date:message-id:from:from:to :cc:subject:date:message-id:reply-to; bh=KBCrKnuu+3+9mT93IOVCPLlu0tAgg9wTYBOooqnzqN4=; b=HBErS4YgQDhv48gqjvLiw1vGnNV0nHRNLYqZNtf1fGZvp3V+E6jelpf/G9SYEcRC1l dEt6uPD1J2uj3+oil8gJLLkTEEpGOL11nbACch1lp16WiWvc4OySRiyjDy94I+5O0BNy KE2EpsoHkjPbhFjtPx2OYViR16XdaB02UMrtrPQBxlfX7CZL7YDYW+h1XIkff2yhxVy7 b7ohFP9BteZZeKdbDRIIcQow5Y/dxWOK08QfPmAYh40D3Yaa4sdaSTjn2+bZUkPfaZ1N gtk+RPKaEGMpxRzhE+IU0kD4RvBvsYezhSakiVLs359riGECPJs/m9LmBb/9Tkh9k+ux gslQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763585016; x=1764189816; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:user-agent:mime-version:date:message-id:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=KBCrKnuu+3+9mT93IOVCPLlu0tAgg9wTYBOooqnzqN4=; b=m4eQHvvXBRjbk4nJfVIhQ2uXdafPErhLdfkcpVF1pebhG/jscxe+Tnn++dojiaBZ/C 1Y0YAsCv6a/5eK6iwjuF1gItLISSfTiJpsJkN28v84lhxseahtQ0Hq+EGyE8/fo9oXnd mac8RmPmcbQFW6UsObAo6rmWoXWdJN2Vbv7PBjExJtfoL4oKjgHddcp3/MZlxnAC2yEW Bi4WQFBo6rW3DpAi0a/yK0HmPPPnmad2Oy14fCkPc0oUlLv22NBAifAPxgH2ajQWUxd7 8LvnclQwoZQKmI6daE151Px2Sz7A2MSOFpqteqarWEUDTviOSyFYoA2lwtjI/d+s53R5 ZJsQ== X-Forwarded-Encrypted: i=1; AJvYcCWE0qBN1EMCDLYyDjocbskMv4cp4D96bYH+KLuLbQXVDpAA73tFt+4WlsG1nvqO3tfKUZ1OOu0yQ7WPtmE=@vger.kernel.org X-Gm-Message-State: AOJu0YwYeld61hpirrTgM6gTZ323WUFSNYkZlNcUtpGs8UMQxHvO8J7u /g8RIp+2uFRWphJ8pD2IUh5cHWKGke7kGh1lbiR4QTPbtg17m/vF6g3iXhJM697NCwhEbmuqv9F Iab3POWoEsleGEiCbi4E4rK+8dYrVeBN3/0IZtGflWER9o+uTxSYFASHuGq/2KgAJmA== X-Gm-Gg: ASbGncutBpO7GVITEmnxnWLZHkQMPlSk5mT0KbB//i/l9CsbIFeawJE09x0BjSWl5Aq CcY4dow7lu60eBq7Jln/YL86Vx47RX62vvhmiDDLbbU/cp++5vXsVpzse5BXmHXyPkUFMMsafBU AojR/JW/4b4nWS9LQxoikUmHuE8gVrETcM9dvWuFdxBw/sDXS1admUuDD4lnrY/IupT25yz9LhC ceyPfDQjgzs60DB+50/5bHEI+OkMIJ7Rm/HceB5hDGvSdwtxFxFt2EvGZkIXYnCjycYT6L5GJKQ I5QhpKCw+v4Ox+FPdDGbgt/x+87OI1rfsN7StQOhLxuBG6zdFFIfwD4T4d780Jdx4bD1GjtLQPf FXkOHWgLkoAFijsSFVb/kHJSKcsoZdeR2FWNsQW7BtHU05G5bibWOmJQz X-Received: by 2002:a05:620a:4609:b0:8ac:70cd:8727 with SMTP id af79cd13be357-8b317da61camr549374285a.11.1763585015703; Wed, 19 Nov 2025 12:43:35 -0800 (PST) X-Google-Smtp-Source: AGHT+IGNXRGOjGwnmAtQ4JVhDbLPY3KXlKg/vbeg67kuLnjqH4LHNpCv5N2RpI5UH70CIDZzXe2WyA== X-Received: by 2002:a05:620a:4609:b0:8ac:70cd:8727 with SMTP id af79cd13be357-8b317da61camr549371785a.11.1763585015257; Wed, 19 Nov 2025 12:43:35 -0800 (PST) Received: from ?IPV6:2601:188:c102:b180:1f8b:71d0:77b1:1f6e? ([2601:188:c102:b180:1f8b:71d0:77b1:1f6e]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8b3294336besm22907585a.19.2025.11.19.12.43.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Nov 2025 12:43:34 -0800 (PST) From: Waiman Long X-Google-Original-From: Waiman Long Message-ID: <55c5b273-7b68-4636-824e-ddd5ee757f87@redhat.com> Date: Wed, 19 Nov 2025 15:43:33 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v15 7/7] timers: Exclude isolated cpus from timer migration To: Gabriele Monaco , linux-kernel@vger.kernel.org, Anna-Maria Behnsen , Frederic Weisbecker , Thomas Gleixner Cc: "John B. Wyatt IV" , "John B. Wyatt IV" References: <20251113083324.33490-1-gmonaco@redhat.com> <20251113083324.33490-8-gmonaco@redhat.com> Content-Language: en-US In-Reply-To: <20251113083324.33490-8-gmonaco@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 11/13/25 3:33 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) > * the nohz timekeeper core (must be available to handle global timers) > > CPUs are added to the hierarchy during late boot, excluding isolated > ones, the hierarchy is also adapted when the cpuset isolation changes. > > 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, 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). > > This effect was noticed on a 128 cores machine running oslat on the > isolated cores (1-31,33-63,65-95,97-127). The tool monopolises CPUs, > and the CPU with lowest count in a timer migration hierarchy (here 1 > and 65) appears as always active and continuously pulls global timers, > from the housekeeping CPUs. This ends up moving driver work (e.g. > delayed work) to isolated CPUs and causes latency spikes: > > before the change: > > # oslat -c 1-31,33-63,65-95,97-127 -D 62s > ... > Maximum: 1203 10 3 4 ... 5 (us) > > after the change: > > # oslat -c 1-31,33-63,65-95,97-127 -D 62s > ... > Maximum: 10 4 3 4 3 ... 5 (us) > > The same behaviour was observed on a machine with as few as 20 cores / > 40 threads with isocpus set to: 1-9,11-39 with rtla-osnoise-top. > > Tested-by: John B. Wyatt IV > Tested-by: John B. Wyatt IV > Reviewed-by: Frederic Weisbecker > Signed-off-by: Gabriele Monaco > --- > include/linux/timer.h | 9 ++ > kernel/cgroup/cpuset.c | 3 + > kernel/time/timer_migration.c | 156 ++++++++++++++++++++++++++++++++-- > 3 files changed, 163 insertions(+), 5 deletions(-) > > diff --git a/include/linux/timer.h b/include/linux/timer.h > index 0414d9e6b4fc..62e1cea71125 100644 > --- a/include/linux/timer.h > +++ b/include/linux/timer.h > @@ -188,4 +188,13 @@ 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(struct cpumask *exclude_cpumask); > +#else > +static inline int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask) > +{ > + return 0; > +} > +#endif > + > #endif > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c > index cf34623fe66f..bfc3b319e1c0 100644 > --- a/kernel/cgroup/cpuset.c > +++ b/kernel/cgroup/cpuset.c > @@ -1350,6 +1350,9 @@ static void update_isolation_cpumasks(bool isolcpus_updated) > > ret = workqueue_unbound_exclude_cpumask(isolated_cpus); > WARN_ON_ONCE(ret < 0); > + > + ret = tmigr_isolated_exclude_cpumask(isolated_cpus); > + WARN_ON_ONCE(ret < 0); > } > > /** > diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c > index d3eb9714e692..0e275d526d50 100644 > --- a/kernel/time/timer_migration.c > +++ b/kernel/time/timer_migration.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > > #include "timer_migration.h" > #include "tick-internal.h" > @@ -430,6 +431,9 @@ static DEFINE_PER_CPU(struct tmigr_cpu, tmigr_cpu); > */ > static cpumask_var_t tmigr_available_cpumask; > > +/* Enabled during late initcall */ > +static DEFINE_STATIC_KEY_FALSE(tmigr_exclude_isolated); > + > #define TMIGR_NONE 0xFF > #define BIT_CNT 8 > > @@ -438,6 +442,33 @@ 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 become idle (from a tick and timer > + * migration perspective) when they stop their tick. This lets the timekeeping > + * CPU handle their global timers. Marking also isolated CPUs as idle would be > + * too costly, hence they are completely excluded from the hierarchy. > + * This check is necessary, for instance, to prevent offline isolated CPUs from > + * being incorrectly marked as available once getting back online. > + * > + * This function returns false during early boot and the isolation logic is > + * enabled only after isolated CPUs are marked as unavailable at late boot. > + * The tick CPU can be isolated at boot, however we cannot mark it as > + * unavailable to avoid having no global migrator for the nohz_full CPUs. This > + * should be ensured by the callers of this function: implicitly from hotplug > + * callbacs and explicitly in tmigr_init_isolation and > + * tmigr_isolated_exclude_cpumask. > + */ > +static inline bool tmigr_is_isolated(int cpu) > +{ > + if (static_branch_unlikely(&tmigr_exclude_isolated)) > + return (!housekeeping_cpu(cpu, HK_TYPE_DOMAIN) || > + cpuset_cpu_is_isolated(cpu)) && > + housekeeping_cpu(cpu, HK_TYPE_KERNEL_NOISE); > + return false; > +} > + > /* > * Returns true, when @childmask corresponds to the group migrator or when the > * group is not active - so no migrator is set. > @@ -1439,8 +1470,9 @@ static int tmigr_clear_cpu_available(unsigned int cpu) > int migrator; > u64 firstexp; > > - cpumask_clear_cpu(cpu, tmigr_available_cpumask); > scoped_guard(raw_spinlock_irq, &tmc->lock) { > + if (!tmc->available) > + return 0; > tmc->available = false; > WRITE_ONCE(tmc->wakeup, KTIME_MAX); > > @@ -1453,11 +1485,11 @@ static int tmigr_clear_cpu_available(unsigned int cpu) > } > > if (firstexp != KTIME_MAX) { > - migrator = cpumask_any(tmigr_available_cpumask); > + migrator = cpumask_any_but(tmigr_available_cpumask, cpu); > work_on_cpu(migrator, tmigr_trigger_active, NULL); > } > > - return 0; > + return 1; > } > > static int tmigr_set_cpu_available(unsigned int cpu) > @@ -1468,17 +1500,130 @@ static int tmigr_set_cpu_available(unsigned int cpu) > if (WARN_ON_ONCE(!tmc->tmgroup)) > return -EINVAL; > > - cpumask_set_cpu(cpu, tmigr_available_cpumask); > + if (tmigr_is_isolated(cpu)) > + return 0; > + > scoped_guard(raw_spinlock_irq, &tmc->lock) { > + if (tmc->available) > + return 0; > trace_tmigr_cpu_available(tmc); > tmc->idle = timer_base_is_idle(); > if (!tmc->idle) > __tmigr_cpu_activate(tmc); > tmc->available = true; > } > + return 1; > +} > + > +static int tmigr_online_cpu(unsigned int cpu) > +{ > + if (tmigr_set_cpu_available(cpu) > 0) > + cpumask_set_cpu(cpu, tmigr_available_cpumask); > + return 0; > +} > + > +static int tmigr_offline_cpu(unsigned int cpu) > +{ > + if (tmigr_clear_cpu_available(cpu) > 0) > + cpumask_clear_cpu(cpu, tmigr_available_cpumask); > + return 0; > +} > + > +static void tmigr_cpu_isolate(struct work_struct *ignored) > +{ > + tmigr_clear_cpu_available(smp_processor_id()); > +} > + > +static void tmigr_cpu_unisolate(struct work_struct *ignored) > +{ > + tmigr_set_cpu_available(smp_processor_id()); > +} > + > +static int __tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask) > +{ > + struct work_struct __percpu *works __free(free_percpu) = > + alloc_percpu(struct work_struct); > + cpumask_var_t cpumask_unisol __free(free_cpumask_var) = CPUMASK_VAR_NULL; > + cpumask_var_t cpumask_isol __free(free_cpumask_var) = CPUMASK_VAR_NULL; > + int cpu; There are currently only 2 callers for this function - from late_init call and from cpuset. Concurrent call is not possible. Maybe we can just pre-allocate these cpumask_var_t and percpu work structures once and reuse it instead of doing an allocation and free each time it is called. The pre-allocation can be done in tmigr_init_isolation(). > + > + if (!alloc_cpumask_var(&cpumask_isol, GFP_KERNEL)) > + return -ENOMEM; > + if (!alloc_cpumask_var(&cpumask_unisol, GFP_KERNEL)) > + return -ENOMEM; > + if (!works) > + return -ENOMEM; > + > + cpumask_andnot(cpumask_unisol, cpu_online_mask, exclude_cpumask); > + cpumask_andnot(cpumask_unisol, cpumask_unisol, tmigr_available_cpumask); > + /* Set up the mask earlier to avoid races with the migrator CPU */ > + cpumask_or(tmigr_available_cpumask, tmigr_available_cpumask, cpumask_unisol); > + for_each_cpu(cpu, cpumask_unisol) { > + struct work_struct *work = per_cpu_ptr(works, cpu); > + > + INIT_WORK(work, tmigr_cpu_unisolate); > + schedule_work_on(cpu, work); > + } > + > + cpumask_and(cpumask_isol, exclude_cpumask, tmigr_available_cpumask); > + cpumask_and(cpumask_isol, cpumask_isol, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE)); > + /* > + * Handle this here and not in the cpuset code because exclude_cpumask > + * might include also the tick CPU if included in isolcpus. > + */ > + for_each_cpu(cpu, cpumask_isol) { > + if (!tick_nohz_cpu_hotpluggable(cpu)) { > + cpumask_clear_cpu(cpu, cpumask_isol); > + break; > + } > + } > + /* Set up the mask earlier to avoid races with the migrator CPU */ > + cpumask_andnot(tmigr_available_cpumask, tmigr_available_cpumask, cpumask_isol); > + for_each_cpu(cpu, cpumask_isol) { > + struct work_struct *work = per_cpu_ptr(works, cpu); > + > + INIT_WORK(work, tmigr_cpu_isolate); > + schedule_work_on(cpu, work); > + } > + > + for_each_cpu_or(cpu, cpumask_isol, cpumask_unisol) > + flush_work(per_cpu_ptr(works, cpu)); > + > return 0; > } > > +/** > + * tmigr_isolated_exclude_cpumask - Exclude given CPUs from hierarchy > + * @exclude_cpumask: the cpumask to be excluded from timer migration hierarchy > + * > + * This function can be called from cpuset code to provide the new set of > + * isolated CPUs that should be excluded from the hierarchy. > + * Online CPUs not present in exclude_cpumask but already excluded are brought > + * back to the hierarchy. > + * Functions to isolate/unisolate need to be called locally and can sleep. > + */ > +int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask) > +{ > + lockdep_assert_cpus_held(); > + return __tmigr_isolated_exclude_cpumask(exclude_cpumask); > +} > + > +static int __init tmigr_init_isolation(void) > +{ > + cpumask_var_t cpumask __free(free_cpumask_var) = CPUMASK_VAR_NULL; > + > + static_branch_enable(&tmigr_exclude_isolated); > + > + if (!housekeeping_enabled(HK_TYPE_DOMAIN)) > + return 0; > + if (!alloc_cpumask_var(&cpumask, GFP_KERNEL)) > + return -ENOMEM; > + > + cpumask_andnot(cpumask, cpu_possible_mask, housekeeping_cpumask(HK_TYPE_DOMAIN)); > + > + return __tmigr_isolated_exclude_cpumask(cpumask); > +} Should we put all these functions under "#if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON)" like in the timer.h header file? Cheers, Longman > + > static void tmigr_init_group(struct tmigr_group *group, unsigned int lvl, > int node) > { > @@ -1867,7 +2012,7 @@ static int __init tmigr_init(void) > goto err; > > ret = cpuhp_setup_state(CPUHP_AP_TMIGR_ONLINE, "tmigr:online", > - tmigr_set_cpu_available, tmigr_clear_cpu_available); > + tmigr_online_cpu, tmigr_offline_cpu); > if (ret) > goto err; > > @@ -1878,3 +2023,4 @@ static int __init tmigr_init(void) > return ret; > } > early_initcall(tmigr_init); > +late_initcall(tmigr_init_isolation);