From: Tim Chen <tim.c.chen@linux.intel.com>
To: Luo Gengkun <luogengkun2@huawei.com>,
peterz@infradead.org, mingo@redhat.com, juri.lelli@redhat.com,
vincent.guittot@linaro.org, yu.c.chen@intel.com
Cc: dietmar.eggemann@arm.com, rostedt@goodmis.org,
bsegall@google.com, mgorman@suse.de, vschneid@redhat.com,
kprateek.nayak@amd.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 linux 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus
Date: Mon, 20 Jul 2026 15:09:30 -0700 [thread overview]
Message-ID: <ee643d72b410087c1aa316d58de685e8a12d122c.camel@linux.intel.com> (raw)
In-Reply-To: <20260720122214.3977092-2-luogengkun2@huawei.com>
On Mon, 2026-07-20 at 12:22 +0000, Luo Gengkun wrote:
> The overhead of task_cache_work() is high, especially in multi-NUMA systems.
> Currently, task_cache_work() tries to find the pref_llc by scanning all CPUs
> in the system. However, most of these scans are meaningless, such as those
> for CPUs that have never been visited or were accessed a long time ago.
>
> To address this problem, introduce visited_cpus to track the visited CPUs
> and evict them once they have not been accessed for a duration exceeding
> llc_epoch_affinity_timeout.
>
> Signed-off-by: Luo Gengkun <luogengkun2@huawei.com>
> ---
> include/linux/mm_types.h | 6 +++++
> include/linux/sched.h | 2 ++
> kernel/sched/fair.c | 48 +++++++++++++++++++++++++++++++---------
> 3 files changed, 46 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index b18c2b2e7d2c..35559079e4d4 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1620,6 +1620,11 @@ static inline int mm_alloc_sched_noprof(struct mm_struct *mm)
> if (!pcpu_sched)
> return -ENOMEM;
>
> + if (!zalloc_cpumask_var(&mm->sc_stat.visited_cpus, GFP_KERNEL)) {
> + free_percpu(pcpu_sched);
> + return -ENOMEM;
> + }
> +
> mm_init_sched(mm, pcpu_sched);
> return 0;
> }
> @@ -1630,6 +1635,7 @@ static inline void mm_destroy_sched(struct mm_struct *mm)
> {
> free_percpu(mm->sc_stat.pcpu_sched);
> mm->sc_stat.pcpu_sched = NULL;
> + free_cpumask_var(mm->sc_stat.visited_cpus);
> }
> #else /* !CONFIG_SCHED_CACHE */
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 373bcc0598d1..b461a71a65da 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2388,6 +2388,7 @@ static __always_inline int task_mm_cid(struct task_struct *t)
> struct sched_cache_time {
> u64 runtime;
> unsigned long epoch;
> + unsigned long epoch_last_visit;
> };
>
> struct sched_cache_stat {
> @@ -2398,6 +2399,7 @@ struct sched_cache_stat {
> unsigned long next_scan;
> unsigned long footprint;
> int cpu;
> + cpumask_var_t visited_cpus;
> } ____cacheline_aligned_in_smp;
>
> #else
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d78467ec6ee1..c56b2df66de8 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1585,6 +1585,7 @@ void mm_init_sched(struct mm_struct *mm,
> pcpu_sched->runtime = 0;
> /* a slightly stale cpu epoch is acceptible */
> pcpu_sched->epoch = rq->cpu_epoch;
> + pcpu_sched->epoch_last_visit = rq->cpu_epoch;
> epoch = rq->cpu_epoch;
> }
>
> @@ -1635,13 +1636,23 @@ static inline void __update_mm_sched(struct rq *rq,
> }
> }
>
> -static unsigned long fraction_mm_sched(struct rq *rq,
> - struct sched_cache_time *pcpu_sched)
> +static unsigned long fraction_mm_sched(int cpu,
> + struct mm_struct *mm)
> {
> + struct sched_cache_time *pcpu_sched =
> + per_cpu_ptr(mm->sc_stat.pcpu_sched, cpu);
> + struct rq *rq = cpu_rq(cpu);
> +
> guard(raw_spinlock_irqsave)(&rq->cpu_epoch_lock);
>
> __update_mm_sched(rq, pcpu_sched);
>
> + /* Skip the rq that has not been hit for a long time */
> + if ((rq->cpu_epoch - pcpu_sched->epoch_last_visit) > llc_epoch_affinity_timeout) {
> + cpumask_clear_cpu(cpu, mm->sc_stat.visited_cpus);
> + return 0;
> + }
> +
> /*
> * Runtime is a geometric series (r=0.5) and as such will sum to twice
> * the accumulation period, this means the multiplcation here should
> @@ -1711,6 +1722,9 @@ void account_mm_sched(struct rq *rq, struct task_struct *p, s64 delta_exec)
> pcpu_sched->runtime += delta_exec;
> rq->cpu_runtime += delta_exec;
> epoch = rq->cpu_epoch;
> + pcpu_sched->epoch_last_visit = epoch;
We need to make sure that the epoch_last_visit update is seen by the cpu
running task_cache_work(), before it attempts to do the epoch comparison
and clear the cpu, and causing inconsistency in the visited_cpus.
For example
CPU A (e.g. doing LLC/affinity selection, reading remote pcpu_sched) CPU B (= `cpu`, running account_mm_sched() locally)
-------------------------------------------------------------------- -----------------------------------------------------
read pcpu_sched->epoch_last_visit (stale, old value)
-> looks like it timed out
pcpu_sched->epoch_last_visit = epoch (fresh visit!)
cpumask_set_cpu(cpu, visited_cpus) (correctly marks it visited)
cpumask_clear_cpu(cpu, visited_cpus) <-- wipes out the fresh set!
Perhaps we will need a memory barrier and double check for expiration before we clear
the visited mask.
if (rq->cpu_epoch - READ_ONCE(pcpu_sched->epoch_last_visit > llc_epoch_affinity_timeout &&
cpumask_test_cpu(cpu, mm-sc_stat.visited_cpus)) {
/*
* account_mm_sched() may have raced in right here and refreshed
* epoch_last_visit + set the bit again after we read it as stale.
* Re-check timeout after applying memory barrier.
*/
smp_mb();
if ((rq->cpu_epoch - READ_ONCE(pcpu_sched->epoch_last_visit)) > llc_epoch_affinity_timeout))
cpumask_clear_cpu(cpu, mm->sc_stat.visited_cpus);
}
This fix is ugly and there's probably a better way.
> + if (!cpumask_test_cpu(cpu_of(rq), mm->sc_stat.visited_cpus))
> + cpumask_set_cpu(cpu_of(rq), mm->sc_stat.visited_cpus);
> }
>
> /*
> @@ -1867,6 +1881,18 @@ static void task_cache_work(struct callback_head *work)
> guard(rcu)();
>
> get_scan_cpumasks(cpus, p);
Now that we know exactly which cpus to scan from visited_cpus,
we can get rid of get_scan_cpumasks(), which attempts to limit
the scope of cpu scan.
Thanks.
Tim
> + /*
> + * Data race: While evaluating the visited_cpus without
> + * a lock, a CPU could be concurrently set by
> + * account_mm_sched(), meaning the scan might skip the newly
> + * visited CPU if the bit changes during the scan. This is
> + * a deliberate trade-off between accuracy and efficiency:
> + * locking would prevent this race but incur extra overhead.
> + * The missed runtime contribution is negligible because it
> + * implies this process hasn't run on that CPU for a long
> + * time, and will be captured in the next cycle.
> + */
> + cpumask_and(cpus, cpus, mm->sc_stat.visited_cpus);
>
> for_each_cpu(cpu, cpus) {
> /* XXX sched_cluster_active */
> @@ -1877,19 +1903,21 @@ static void task_cache_work(struct callback_head *work)
> if (!sd)
> continue;
>
> - for_each_cpu(i, sched_domain_span(sd)) {
> - occ = fraction_mm_sched(cpu_rq(i),
> - per_cpu_ptr(mm->sc_stat.pcpu_sched, i));
> + for_each_cpu_and(i, sched_domain_span(sd), mm->sc_stat.visited_cpus) {
> + cur = rcu_dereference_all(cpu_rq(i)->curr);
> + if (cur && !(cur->flags & (PF_EXITING | PF_KTHREAD)) &&
> + cur->mm == mm)
> + nr_running++;
> +
> + occ = fraction_mm_sched(i, mm);
> + if (occ == 0)
> + continue;
> +
> a_occ += occ;
> if (occ > m_occ) {
> m_occ = occ;
> m_cpu = i;
> }
> -
> - cur = rcu_dereference_all(cpu_rq(i)->curr);
> - if (cur && !(cur->flags & (PF_EXITING | PF_KTHREAD)) &&
> - cur->mm == mm)
> - nr_running++;
> }
>
> /*
next prev parent reply other threads:[~2026-07-20 22:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-20 12:22 [PATCH v7 linux 0/2] Cache aware scheduling: Reduce the overhead of task_cache_work Luo Gengkun
2026-07-20 12:22 ` [PATCH v7 linux 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus Luo Gengkun
2026-07-20 22:09 ` Tim Chen [this message]
2026-07-21 2:53 ` Luo Gengkun
2026-07-21 14:59 ` Tim Chen
2026-07-20 12:22 ` [PATCH v7 linux 2/2] -- DO NOT APPLY!!! -- sched/cache/debug: Add trace event and sched feature to track scan cost Luo Gengkun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ee643d72b410087c1aa316d58de685e8a12d122c.camel@linux.intel.com \
--to=tim.c.chen@linux.intel.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luogengkun2@huawei.com \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
--cc=yu.c.chen@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®