* [PATCH v8 0/2] Cache aware scheduling: Reduce the overhead of task_cache_work
@ 2026-07-23 4:04 Luo Gengkun
2026-07-23 4:04 ` [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus Luo Gengkun
2026-07-23 4:04 ` [PATCH v8 2/2] -- DO NOT APPLY!!! -- sched/cache/debug: Add trace event and sched feature to track scan cost Luo Gengkun
0 siblings, 2 replies; 15+ messages in thread
From: Luo Gengkun @ 2026-07-23 4:04 UTC (permalink / raw)
To: peterz, mingo, juri.lelli, vincent.guittot, tim.c.chen, yu.c.chen
Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
kprateek.nayak, linux-kernel
Hi everyone,
The cache-aware scheduling patches have now been merged into the mainline.
The goal of this series is to reduce the overhead of task_cache_work() by
minimizing the number of scanned CPUs, which yields significant
performance gains in multi-instance scenarios like Redis. To facilitate
testing, a debug patch is appended.
Below are the Redis test results conducted on AMD server.
When NUMA balancing is disabled:
valkey-benchmark rps | baseline | schedcache | schedcache_visit
| (no cache aware) | (without this series) | (with this series)
----------------------+---------------------+---------------------+-----------+-------------------+--------
| p99 latency(ms) | p99 latency(ms) | DIFF(%) | p99 latency(ms) | DIFF(%)
400000 | 0.436 | 0.554 | -25.68% | 0.441 | -1.14%
Additionally, the output of perf top -e cycles:k highlights the overhead
incurred by task_cache_work():
valkey-benchmark rps | schedcache | schedcache_visit
| (without this series) | (with this series)
----------------------+-------------------------------------------+---------------------------------------
400000 | 0.81% [kernel] [k] task_cache_work | 0.02% [kernel] [k] task_cache_work
The trace data is shown below:
echo SC_NODE > /sys/kernel/debug/sched/features
echo NO_SC_VISIT > /sys/kernel/debug/sched/features
redis-server-24660 [195] ..... 2916.005989: sched_cache_scan: comm=redis-server pid=24660 scan=384
valkey-benchmar-48836 [317] ..... 2916.005993: sched_cache_scan: comm=valkey-benchmar pid=48836 scan=384
valkey-benchmar-48487 [310] ..... 2916.006002: sched_cache_scan: comm=valkey-benchmar pid=48487 scan=384
echo NO_SC_NODE > /sys/kernel/debug/sched/features
echo SC_VISIT > /sys/kernel/debug/sched/features
redis-server-29080 [225] ..... 2876.257909: sched_cache_scan: comm=redis-server pid=29080 scan=14
redis-server-26694 [018] ..... 2876.257909: sched_cache_scan: comm=redis-server pid=26694 scan=13
valkey-benchmar-48198 [103] ..... 2876.257909: sched_cache_scan: comm=valkey-benchmar pid=48198 scan=16
The above results show that this series significantly reduces the overhead
of task_cache_work() by decreasing the number of CPUs that need to be
scanned, from 384 to 16, when NUMA balancing is disabled.
Even with NUMA balancing enabled, this patch provides better performance
benefits. The following tests were conducted with NUMA balancing enabled:
valkey-benchmark rps | baseline | schedcache | schedcache_visit
| (no cache aware) | (without this series) | (with this series)
----------------------+---------------------+---------------------+-----------+-------------------+--------
| p99 latency(ms) | p99 latency(ms) | DIFF(%) | p99 latency(ms) | DIFF(%)
400000 | 0.437 | 0.454 | -3.89% | 0.442 | -1.14%
The overhead of task_cache_work() is as follows:
valkey-benchmark rps | schedcache | schedcache_visit
| (without this series) | (with this series)
----------------------+-------------------------------------------+---------------------------------------
400000 | 0.13% [kernel] [k] task_cache_work | 0.03% [kernel] [k] task_cache_work
Hackbench testing was also conducted. The results below show that this
series does not affect the accuracy of cache-aware scheduling.
=========================================
Hackbench Comparison: baseline vs schedcache
=========================================
MODE GROUPS FDS | baseline(std) | schedcache(std) | DIFF(%) | VERDICT
---------- ------ -----+--------------------+--------------------+------------+-----------
threads 1 10 | 113.200 (4.22%) | 67.300 (1.32%) | 40.55% | IMPROVED
threads 1 2 | 16.555 (4.11%) | 11.020 (1.66%) | 33.43% | IMPROVED
threads 1 20 | 250.774 (1.26%) | 265.026 (5.44%) | -5.68% | REGRESSED
threads 1 4 | 42.117 (1.44%) | 27.758 (1.64%) | 34.09% | IMPROVED
threads 1 6 | 65.140 (4.31%) | 39.182 (1.38%) | 39.85% | IMPROVED
threads 1 8 | 84.286 (1.29%) | 53.721 (1.58%) | 36.26% | IMPROVED
threads 2 10 | 122.592 (0.44%) | 113.365 (4.93%) | 7.53% | IMPROVED
threads 2 2 | 17.702 (4.09%) | 10.473 (0.42%) | 40.84% | IMPROVED
threads 2 20 | 336.457 (1.77%) | 314.108 (1.51%) | 6.64% | IMPROVED
threads 2 4 | 43.989 (0.88%) | 27.067 (3.38%) | 38.47% | IMPROVED
threads 2 6 | 69.322 (0.85%) | 41.707 (4.19%) | 39.84% | IMPROVED
threads 2 8 | 103.767 (1.81%) | 58.518 (3.00%) | 43.61% | IMPROVED
threads 4 10 | 148.882 (3.56%) | 149.449 (1.06%) | -0.38% | REGRESSED
threads 4 2 | 18.909 (2.96%) | 11.063 (2.08%) | 41.49% | IMPROVED
threads 4 20 | 724.943 (2.14%) | 631.222 (3.92%) | 12.93% | IMPROVED
threads 4 4 | 48.191 (1.91%) | 27.352 (5.35%) | 43.24% | IMPROVED
threads 4 6 | 79.725 (3.84%) | 78.732 (4.10%) | 1.25% | IMPROVED
threads 4 8 | 108.768 (1.36%) | 105.928 (1.65%) | 2.61% | IMPROVED
Hackbench Comparison: schedcache vs schedcache_visit
=========================================
MODE GROUPS FDS | schedcache(std) |schedcache_visit(std) | DIFF(%) | VERDICT
---------- ------ -----+--------------------+----------------------+---------+-----------
threads 1 10 | 67.300 (1.32%) | 67.014 (0.96%) | 0.42% | IMPROVED
threads 1 2 | 11.020 (1.66%) | 10.557 (1.46%) | 4.20% | IMPROVED
threads 1 20 | 265.026 (5.44%) | 212.366 (16.32%) | 19.87% | IMPROVED
threads 1 4 | 27.758 (1.64%) | 25.711 (1.32%) | 7.37% | IMPROVED
threads 1 6 | 39.182 (1.38%) | 38.914 (0.34%) | 0.68% | IMPROVED
threads 1 8 | 53.721 (1.58%) | 52.889 (0.27%) | 1.55% | IMPROVED
threads 2 10 | 121.203 (6.99%) | 124.254 (1.38%) | -2.52% | REGRESSED
threads 2 2 | 10.473 (0.42%) | 11.206 (5.91%) | -7.00% | REGRESSED
threads 2 20 | 314.108 (1.51%) | 301.754 (1.95%) | 3.93% | IMPROVED
threads 2 4 | 27.067 (3.38%) | 28.028 (2.01%) | -3.55% | REGRESSED
threads 2 6 | 41.707 (4.19%) | 42.149 (3.35%) | -1.06% | REGRESSED
threads 2 8 | 58.518 (3.00%) | 57.133 (4.39%) | 2.37% | IMPROVED
threads 4 10 | 149.449 (1.06%) | 141.407 (0.08%) | 5.38% | IMPROVED
threads 4 2 | 11.063 (2.08%) | 11.360 (5.85%) | -2.68% | REGRESSED
threads 4 20 | 631.222 (3.92%) | 622.780 (2.49%) | 1.34% | IMPROVED
threads 4 4 | 27.352 (5.35%) | 27.947 (5.37%) | -2.18% | REGRESSED
threads 4 6 | 78.732 (4.10%) | 73.911 (0.70%) | 6.12% | IMPROVED
threads 4 8 | 105.928 (1.65%) | 107.535 (3.29%) | -1.52% | REGRESSED
---
Changes history
**v8 Changes:**
1. Remove get_scan_cpumasks() since visited_cpus provides the exact CPUs to
scan.
Link to v7: https://lore.kernel.org/all/20260720122214.3977092-1-luogengkun2@huawei.com/
**v7 Changes:**
1. Add code comments in task_cache_work() to clarify the race condition
between setting and reading visited_cpus.
Link to v6: https://lore.kernel.org/all/20260717041231.3324851-1-luogengkun2@huawei.com/
**v6 Changes:**
1. Switched to cpumask_var_t to dynamically allocate visited_cpus inside
mm_struct, preventing per-process memory bloat when NR_CPUS is configured
large.
2. Intersected the LLC scheduling domain span directly with visited_cpus
to prevent skipping CPU is cross-node LLC topolgies.
3. Renamed epoch_timeout to epoch_last_visit to better reflect its purpose.
4. Moved __update_mm_sched to execute before the timeout check in
fraction_mm_sched()
Link to v5: https://lore.kernel.org/all/20260709130053.2749834-1-luogengkun2@huawei.com/
**v5 Changes:**
1. Restore get_scan_cpumasks() to avoid violating NUMA_BALANCING constraints.
2. Use for_each_cpu_and() to filter CPUs within the LLC domain.
Link to v4: https://lore.kernel.org/all/20260618064300.80072-1-luogengkun2@huawei.com/
**v4 Changes:**
1. Rebase to the master.
2. epoch_timeout is introduced to evict expired CPUs instead of relying on
epoch, because epoch is refreshed periodically due to invocations of
fraction_mm_sched().
3. Move the increasement of nr_running before fraction_mm_sched().
4. Remove the redundant 'work->next' reset at the end of task_cache_work().
5. Add a debug patch to show the number of CPUs scanned to show the
benefit of this optimization.
Link to v3: https://lore.kernel.org/all/20260423085414.1389749-1-luogengkun2@huawei.com/
**v3 Changes:**
1. Remove the static key and enable this feature by default.
2. Reuse llc_epoch_affinity_timeout instead of introducing
llc_epoch_visited_timeout.
3. Move the calculation of rq->cpu_epoch - pcpu_sched->epoch into
fraction_mm_sched() to avoid race between task_cache_work() and
__update_mm_sched().
4. Reset work->next at the end of task_cache_work() to prevent concurrent
executions by multiple threads within the same process.
Link to v2: https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/
**v2 Changes:**
1. Added a pre-check before set/clear visited_cpus to avoid C2C overhead.
2. Optimized llc_epoch_visited_timeout by using a static key to minimize overhead.
Link to v1: https://lore.kernel.org/all/f2488085-4b52-491d-84be-d30d43954381@huawei.com/
Luo Gengkun (2):
sched/cache: Reduce the overhead of task_cache_work by only scan the
visisted cpus
-- DO NOT APPLY!!! -- sched/cache/debug: Add trace event and sched
feature to track scan cost
include/linux/mm_types.h | 6 ++++
include/linux/sched.h | 2 ++
include/trace/events/sched.h | 21 +++++++++++
kernel/sched/fair.c | 69 +++++++++++++++++++++++++++++-------
kernel/sched/features.h | 2 ++
5 files changed, 88 insertions(+), 12 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-23 4:04 [PATCH v8 0/2] Cache aware scheduling: Reduce the overhead of task_cache_work Luo Gengkun @ 2026-07-23 4:04 ` Luo Gengkun 2026-07-27 1:09 ` Chen, Yu C 2026-07-28 20:16 ` Tim Chen 2026-07-23 4:04 ` [PATCH v8 2/2] -- DO NOT APPLY!!! -- sched/cache/debug: Add trace event and sched feature to track scan cost Luo Gengkun 1 sibling, 2 replies; 15+ messages in thread From: Luo Gengkun @ 2026-07-23 4:04 UTC (permalink / raw) To: peterz, mingo, juri.lelli, vincent.guittot, tim.c.chen, yu.c.chen Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel 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. Now that we know exactly which CPUs to scan from visited_cpus, we can remove get_scan_cpumasks(). Signed-off-by: Luo Gengkun <luogengkun2@huawei.com> --- include/linux/mm_types.h | 6 +++ include/linux/sched.h | 2 + kernel/sched/fair.c | 94 ++++++++++++++++------------------------ 3 files changed, 46 insertions(+), 56 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..10d442074f21 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; + if (!cpumask_test_cpu(cpu_of(rq), mm->sc_stat.visited_cpus)) + cpumask_set_cpu(cpu_of(rq), mm->sc_stat.visited_cpus); } /* @@ -1761,51 +1775,6 @@ static void task_tick_cache(struct rq *rq, struct task_struct *p) } } -static void get_scan_cpumasks(cpumask_var_t cpus, struct task_struct *p) -{ -#ifdef CONFIG_NUMA_BALANCING - int cpu, curr_cpu, nid, pref_nid; - - if (!static_branch_likely(&sched_numa_balancing)) - goto out; - - cpu = READ_ONCE(p->mm->sc_stat.cpu); - if (cpu != -1) - nid = cpu_to_node(cpu); - curr_cpu = task_cpu(p); - - /* - * Scanning in the preferred NUMA node is ideal. However, the NUMA - * preferred node is per-task rather than per-process. It is possible - * for different threads of the process to have distinct preferred - * nodes; consequently, the process-wide preferred LLC may bounce - * between different nodes. As a workaround, maintain the scan - * CPU mask to also cover the process's current preferred LLC and the - * current running node to mitigate the bouncing risk. - * TBD: numa_group should be considered during task aggregation. - */ - pref_nid = p->numa_preferred_nid; - /* honor the task's preferred node */ - if (pref_nid == NUMA_NO_NODE) - goto out; - - cpumask_or(cpus, cpus, cpumask_of_node(pref_nid)); - - /* honor the task's preferred LLC CPU */ - if (cpu != -1 && !cpumask_test_cpu(cpu, cpus) && nid != NUMA_NO_NODE) - cpumask_or(cpus, cpus, cpumask_of_node(nid)); - - /* make sure the task's current running node is included */ - if (!cpumask_test_cpu(curr_cpu, cpus)) - cpumask_or(cpus, cpus, cpumask_of_node(cpu_to_node(curr_cpu))); - - return; - -out: -#endif - cpumask_copy(cpus, cpu_online_mask); -} - static inline void update_avg_scale(u64 *avg, u64 sample) { int factor = per_cpu(sd_llc_size, raw_smp_processor_id()); @@ -1866,7 +1835,18 @@ static void task_cache_work(struct callback_head *work) scoped_guard (cpus_read_lock) { guard(rcu)(); - get_scan_cpumasks(cpus, p); + /* + * 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, cpu_online_mask, mm->sc_stat.visited_cpus); for_each_cpu(cpu, cpus) { /* XXX sched_cluster_active */ @@ -1877,19 +1857,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++; } /* -- 2.34.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-23 4:04 ` [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus Luo Gengkun @ 2026-07-27 1:09 ` Chen, Yu C 2026-07-28 8:53 ` Luo Gengkun 2026-07-28 20:16 ` Tim Chen 1 sibling, 1 reply; 15+ messages in thread From: Chen, Yu C @ 2026-07-27 1:09 UTC (permalink / raw) To: Luo Gengkun Cc: tim.c.chen, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot, chen.yu On 7/23/2026 12:04 PM, Luo Gengkun wrote: [ ... ] > +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) { In v2 there is a check if the cpu has been set before writing: cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/ do we need to bring that back? > + cpumask_clear_cpu(cpu, mm->sc_stat.visited_cpus); > + return 0; > + } > + [ ... ] > > - 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) { Does using for_each_cpu_and(i, sched_domain_span(sd), cpus) reduce the race window to read mm->sc_stat.pcpu_sched? [ ... ] I ran hackbench[1] with above changes on Intel Sapphire Rapids with memory interleave enabled, and on AMD Milan. There is no much difference on Sapphire Rapids, while a slight regress on Milan AMD EPYC 9554P 64-Core when NUMA balancing is enabled(but it looks to be within run-to-run variance) Please double check if this is the case on your AMD server. And you can add my tag in next version: Tested-by: Chen Yu <yu.c.chen@intel.com> Data on Milan ./launch.sh compare hackbench base_numab visit_numab ========================================= Hackbench Comparison: base_numab vs visit_numab ========================================= MODE GROUPS FDS | base_numab(s) | visit_numab(s) | DIFF(%) | VERDICT ---------- ------ -----+--------------------+--------------------+------------+----------- process 1 10 | 34.564 ±0.53% | 34.103 ±1.48% | 1.33% | IMPROVED process 1 16 | 60.556 ±0.52% | 62.296 ±2.11% | -2.87% | REGRESSED process 1 2 | 2.981 ±0.03% | 3.814 ±21.34% | -27.94% | REGRESSED <--run-to-run variance process 1 20 | 80.683 ±1.51% | 81.392 ±0.28% | -0.88% | REGRESSED process 1 4 | 8.446 ±12.50% | 7.185 ±6.86% | 14.93% | IMPROVED process 1 6 | 11.355 ±1.09% | 11.775 ±0.98% | -3.70% | REGRESSED process 1 8 | 19.282 ±6.61% | 19.976 ±7.10% | -3.60% | REGRESSED process 2 10 | 36.729 ±2.08% | 36.899 ±1.25% | -0.46% | REGRESSED process 2 16 | 63.758 ±0.47% | 63.373 ±0.74% | 0.60% | IMPROVED process 2 2 | 3.240 ±4.01% | 3.289 ±6.17% | -1.51% | REGRESSED process 2 20 | 85.519 ±1.97% | 83.396 ±0.26% | 2.48% | IMPROVED process 2 4 | 8.551 ±3.08% | 9.132 ±9.56% | -6.79% | REGRESSED process 2 6 | 15.256 ±10.90% | 11.869 ±1.21% | 22.20% | IMPROVED process 2 8 | 26.729 ±2.24% | 25.681 ±11.10% | 3.92% | IMPROVED process 4 10 | 39.921 ±1.01% | 39.274 ±0.37% | 1.62% | IMPROVED process 4 16 | 74.032 ±2.08% | 83.334 ±5.79% | -12.56% | REGRESSED process 4 2 | 4.339 ±11.59% | 4.367 ±10.33% | -0.65% | REGRESSED process 4 20 | 120.967 ±0.67% | 124.640 ±3.31% | -3.04% | REGRESSED process 4 4 | 11.088 ±4.61% | 11.250 ±6.90% | -1.46% | REGRESSED process 4 6 | 21.023 ±0.77% | 20.639 ±1.40% | 1.83% | IMPROVED process 4 8 | 31.368 ±0.36% | 30.306 ±0.77% | 3.39% | IMPROVED process 8 10 | 56.356 ±1.47% | 55.669 ±0.98% | 1.22% | IMPROVED process 8 16 | 144.752 ±0.84% | 139.409 ±0.61% | 3.69% | IMPROVED process 8 2 | 5.697 ±4.14% | 5.947 ±5.28% | -4.39% | REGRESSED process 8 20 | 220.395 ±0.32% | 215.589 ±0.66% | 2.18% | IMPROVED process 8 4 | 14.589 ±7.09% | 14.148 ±2.56% | 3.02% | IMPROVED process 8 6 | 23.872 ±1.39% | 24.558 ±4.92% | -2.87% | REGRESSED process 8 8 | 34.434 ±0.60% | 36.422 ±4.85% | -5.77% | REGRESSED threads 1 10 | 35.582 ±0.62% | 35.697 ±0.94% | -0.32% | REGRESSED threads 1 16 | 62.996 ±1.36% | 63.255 ±0.65% | -0.41% | REGRESSED threads 1 2 | 3.216 ±0.16% | 3.253 ±1.20% | -1.15% | REGRESSED threads 1 20 | 83.611 ±1.53% | 82.771 ±0.23% | 1.00% | IMPROVED threads 1 4 | 6.952 ±0.50% | 6.957 ±0.06% | -0.07% | REGRESSED threads 1 6 | 14.028 ±3.49% | 12.997 ±0.41% | 7.35% | IMPROVED threads 1 8 | 21.511 ±1.71% | 21.395 ±2.73% | 0.54% | IMPROVED threads 2 10 | 37.655 ±1.42% | 38.516 ±0.54% | -2.29% | REGRESSED threads 2 16 | 67.366 ±2.88% | 68.230 ±3.80% | -1.28% | REGRESSED threads 2 2 | 3.301 ±1.21% | 3.344 ±1.50% | -1.30% | REGRESSED threads 2 20 | 94.511 ±1.12% | 87.065 ±0.73% | 7.88% | IMPROVED threads 2 4 | 8.793 ±7.80% | 10.709 ±3.75% | -21.79% | REGRESSED threads 2 6 | 16.934 ±11.50% | 16.483 ±15.58% | 2.66% | IMPROVED threads 2 8 | 26.700 ±7.11% | 25.430 ±3.37% | 4.76% | IMPROVED threads 4 10 | 40.121 ±1.36% | 40.582 ±2.06% | -1.15% | REGRESSED threads 4 16 | 83.463 ±2.77% | 79.598 ±0.41% | 4.63% | IMPROVED threads 4 2 | 4.855 ±3.95% | 4.753 ±15.72% | 2.10% | IMPROVED threads 4 20 | 134.076 ±0.44% | 131.447 ±1.24% | 1.96% | IMPROVED threads 4 4 | 13.193 ±6.68% | 11.804 ±13.90% | 10.53% | IMPROVED threads 4 6 | 21.001 ±3.50% | 18.046 ±0.96% | 14.07% | IMPROVED threads 4 8 | 30.247 ±2.34% | 28.422 ±2.57% | 6.03% | IMPROVED threads 8 10 | 58.323 ±0.46% | 60.638 ±1.94% | -3.97% | REGRESSED threads 8 16 | 149.357 ±0.83% | 153.554 ±0.57% | -2.81% | REGRESSED threads 8 2 | 5.157 ±3.26% | 5.542 ±6.69% | -7.47% | REGRESSED threads 8 20 | 227.619 ±0.40% | 229.307 ±1.23% | -0.74% | REGRESSED threads 8 4 | 15.586 ±3.65% | 14.631 ±0.11% | 6.13% | IMPROVED threads 8 6 | 25.401 ±2.47% | 24.989 ±2.39% | 1.62% | IMPROVED threads 8 8 | 36.215 ±3.22% | 37.484 ±3.35% | -3.50% | REGRESSED Note: Values shown as mean ±%stddev (seconds). DIFF based on means. DIFF(%) = (base_numab - visit_numab) / base_numab * 100 Positive = improvement (visit_numab faster), Negative = regression. [1] https://github.com/chen-yu-surf/bench_tool.git ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-27 1:09 ` Chen, Yu C @ 2026-07-28 8:53 ` Luo Gengkun 2026-07-28 12:08 ` : " Chen Yu 0 siblings, 1 reply; 15+ messages in thread From: Luo Gengkun @ 2026-07-28 8:53 UTC (permalink / raw) To: Chen, Yu C Cc: tim.c.chen, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot, chen.yu On 2026/7/27 9:09, Chen, Yu C wrote: > On 7/23/2026 12:04 PM, Luo Gengkun wrote: > > [ ... ] > >> +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) { > > In v2 there is a check if the cpu has been set before writing: > cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) > https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/ > do we need to bring that back? > I don't think we need it back. Here is why: In v2, for_each_cpu was used instead of for_each_cpu_and in the inner loop, meaning some CPUs being checked might not have been set. Therefore, cpumask_test_cpu was necessary to filter out those cases. Now, with for_each_cpu_and(i, sched_domain_span(sd), &mm->sc_stat.visited_cpus), we can ensure each scanned CPU is set, so the issue no longer exists. Furthermore, the only place where the visited_cpus bits are cleared is task_cache_work(), which is only called once per scan period, there is no risk of the bit being cleared concurrently mid-loop. However, is there a possibility that the current task_cache_work() execution hasn't finished yet when the next scan window arrives? For instance, if the current task work is heavily delayed or preempted by unexpected interrupt, jiffies could advance past next_scan before the loop completes. If we move the `work->next = work;` to the very end of task_cache_work(), would that resolve this issue? By doing so, the existing `work->next == work` check in task_tick_cache() should fail and no new task work will be submitted. Please let me know if I'm missing something. >> + cpumask_clear_cpu(cpu, mm->sc_stat.visited_cpus); >> + return 0; >> + } >> + > > [ ... ] > >> - 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) { > > Does using > for_each_cpu_and(i, sched_domain_span(sd), cpus) > reduce the race window to read mm->sc_stat.pcpu_sched? Agree. Since we now solely use visited_cpus to filter CPUs and get_scan_cpumasks() has been removed, this inner loop will no longer incorrectly skip those CPUs in cross-node LLC domains. Thus, the issue previously raised by Sashiko no longer exists. > > [ ... ] > > I ran hackbench[1] with above changes on Intel Sapphire Rapids with memory interleave enabled, > and on AMD Milan. There is no much difference on Sapphire Rapids, while a slight regress on > Milan AMD EPYC 9554P 64-Core when NUMA balancing is enabled(but it looks to be within > run-to-run variance) Please double check if this is the case on your AMD server. > And you can add my tag in next version: > > Tested-by: Chen Yu <yu.c.chen@intel.com> > > > Data on Milan > ./launch.sh compare hackbench base_numab visit_numab > ========================================= > Hackbench Comparison: base_numab vs visit_numab > ========================================= > MODE GROUPS FDS | base_numab(s) | visit_numab(s) | DIFF(%) | VERDICT > ---------- ------ -----+--------------------+--------------------+------------+----------- > process 1 10 | 34.564 ±0.53% | 34.103 ±1.48% | 1.33% | IMPROVED > process 1 16 | 60.556 ±0.52% | 62.296 ±2.11% | -2.87% | REGRESSED > process 1 2 | 2.981 ±0.03% | 3.814 ±21.34% | -27.94% | REGRESSED <--run-to-run variance > process 1 20 | 80.683 ±1.51% | 81.392 ±0.28% | -0.88% | REGRESSED > process 1 4 | 8.446 ±12.50% | 7.185 ±6.86% | 14.93% | IMPROVED > process 1 6 | 11.355 ±1.09% | 11.775 ±0.98% | -3.70% | REGRESSED > process 1 8 | 19.282 ±6.61% | 19.976 ±7.10% | -3.60% | REGRESSED > process 2 10 | 36.729 ±2.08% | 36.899 ±1.25% | -0.46% | REGRESSED > process 2 16 | 63.758 ±0.47% | 63.373 ±0.74% | 0.60% | IMPROVED > process 2 2 | 3.240 ±4.01% | 3.289 ±6.17% | -1.51% | REGRESSED > process 2 20 | 85.519 ±1.97% | 83.396 ±0.26% | 2.48% | IMPROVED > process 2 4 | 8.551 ±3.08% | 9.132 ±9.56% | -6.79% | REGRESSED > process 2 6 | 15.256 ±10.90% | 11.869 ±1.21% | 22.20% | IMPROVED > process 2 8 | 26.729 ±2.24% | 25.681 ±11.10% | 3.92% | IMPROVED > process 4 10 | 39.921 ±1.01% | 39.274 ±0.37% | 1.62% | IMPROVED > process 4 16 | 74.032 ±2.08% | 83.334 ±5.79% | -12.56% | REGRESSED > process 4 2 | 4.339 ±11.59% | 4.367 ±10.33% | -0.65% | REGRESSED > process 4 20 | 120.967 ±0.67% | 124.640 ±3.31% | -3.04% | REGRESSED > process 4 4 | 11.088 ±4.61% | 11.250 ±6.90% | -1.46% | REGRESSED > process 4 6 | 21.023 ±0.77% | 20.639 ±1.40% | 1.83% | IMPROVED > process 4 8 | 31.368 ±0.36% | 30.306 ±0.77% | 3.39% | IMPROVED > process 8 10 | 56.356 ±1.47% | 55.669 ±0.98% | 1.22% | IMPROVED > process 8 16 | 144.752 ±0.84% | 139.409 ±0.61% | 3.69% | IMPROVED > process 8 2 | 5.697 ±4.14% | 5.947 ±5.28% | -4.39% | REGRESSED > process 8 20 | 220.395 ±0.32% | 215.589 ±0.66% | 2.18% | IMPROVED > process 8 4 | 14.589 ±7.09% | 14.148 ±2.56% | 3.02% | IMPROVED > process 8 6 | 23.872 ±1.39% | 24.558 ±4.92% | -2.87% | REGRESSED > process 8 8 | 34.434 ±0.60% | 36.422 ±4.85% | -5.77% | REGRESSED > threads 1 10 | 35.582 ±0.62% | 35.697 ±0.94% | -0.32% | REGRESSED > threads 1 16 | 62.996 ±1.36% | 63.255 ±0.65% | -0.41% | REGRESSED > threads 1 2 | 3.216 ±0.16% | 3.253 ±1.20% | -1.15% | REGRESSED > threads 1 20 | 83.611 ±1.53% | 82.771 ±0.23% | 1.00% | IMPROVED > threads 1 4 | 6.952 ±0.50% | 6.957 ±0.06% | -0.07% | REGRESSED > threads 1 6 | 14.028 ±3.49% | 12.997 ±0.41% | 7.35% | IMPROVED > threads 1 8 | 21.511 ±1.71% | 21.395 ±2.73% | 0.54% | IMPROVED > threads 2 10 | 37.655 ±1.42% | 38.516 ±0.54% | -2.29% | REGRESSED > threads 2 16 | 67.366 ±2.88% | 68.230 ±3.80% | -1.28% | REGRESSED > threads 2 2 | 3.301 ±1.21% | 3.344 ±1.50% | -1.30% | REGRESSED > threads 2 20 | 94.511 ±1.12% | 87.065 ±0.73% | 7.88% | IMPROVED > threads 2 4 | 8.793 ±7.80% | 10.709 ±3.75% | -21.79% | REGRESSED > threads 2 6 | 16.934 ±11.50% | 16.483 ±15.58% | 2.66% | IMPROVED > threads 2 8 | 26.700 ±7.11% | 25.430 ±3.37% | 4.76% | IMPROVED > threads 4 10 | 40.121 ±1.36% | 40.582 ±2.06% | -1.15% | REGRESSED > threads 4 16 | 83.463 ±2.77% | 79.598 ±0.41% | 4.63% | IMPROVED > threads 4 2 | 4.855 ±3.95% | 4.753 ±15.72% | 2.10% | IMPROVED > threads 4 20 | 134.076 ±0.44% | 131.447 ±1.24% | 1.96% | IMPROVED > threads 4 4 | 13.193 ±6.68% | 11.804 ±13.90% | 10.53% | IMPROVED > threads 4 6 | 21.001 ±3.50% | 18.046 ±0.96% | 14.07% | IMPROVED > threads 4 8 | 30.247 ±2.34% | 28.422 ±2.57% | 6.03% | IMPROVED > threads 8 10 | 58.323 ±0.46% | 60.638 ±1.94% | -3.97% | REGRESSED > threads 8 16 | 149.357 ±0.83% | 153.554 ±0.57% | -2.81% | REGRESSED > threads 8 2 | 5.157 ±3.26% | 5.542 ±6.69% | -7.47% | REGRESSED > threads 8 20 | 227.619 ±0.40% | 229.307 ±1.23% | -0.74% | REGRESSED > threads 8 4 | 15.586 ±3.65% | 14.631 ±0.11% | 6.13% | IMPROVED > threads 8 6 | 25.401 ±2.47% | 24.989 ±2.39% | 1.62% | IMPROVED > threads 8 8 | 36.215 ±3.22% | 37.484 ±3.35% | -3.50% | REGRESSED > > Note: Values shown as mean ±%stddev (seconds). DIFF based on means. > DIFF(%) = (base_numab - visit_numab) / base_numab * 100 > Positive = improvement (visit_numab faster), Negative = regression. > > > [1] https://github.com/chen-yu-surf/bench_tool.git > Thanks for the testing and the tag! I believe that the results are likely due to run-to-run variance. Below are my test results, and I noticed that some test cases regressed on my side while they improved on yours. For example, the threads 2 2 case showed completely opposite trends between our environments. Data AMD EPYC 9654 ========================================= Hackbench comparison BASE: base_numab_0728 TEST: visit_numab_0728 ========================================= MODE G FD | BASE(s) | TEST(s) | DIFF% | RESULT ------- -- ----+-----------------+-----------------+---------+---------- process 1 10 | 63.856/4.7% | 65.950/0.8% | -3.28% | REGRESSED process 1 16 | 107.365/1.5% | 106.472/1.0% | 0.83% | IMPROVED process 1 2 | 9.813/0.7% | 9.215/3.8% | 6.09% | IMPROVED process 1 20 | 130.314/0.9% | 132.356/0.9% | -1.57% | REGRESSED process 1 4 | 18.624/2.9% | 19.663/6.3% | -5.58% | REGRESSED process 1 6 | 33.364/3.5% | 32.145/1.4% | 3.65% | IMPROVED process 1 8 | 45.797/4.5% | 45.128/10.7% | 1.46% | IMPROVED process 2 10 | 63.760/1.1% | 64.099/0.4% | -0.53% | REGRESSED process 2 16 | 107.555/1.3% | 105.975/0.6% | 1.47% | IMPROVED process 2 2 | 9.995/1.5% | 9.794/3.1% | 2.01% | IMPROVED process 2 20 | 132.837/0.3% | 133.416/1.5% | -0.44% | REGRESSED process 2 4 | 19.617/3.5% | 21.275/5.1% | -8.45% | REGRESSED process 2 6 | 33.962/3.2% | 34.376/2.6% | -1.22% | REGRESSED process 2 8 | 47.400/4.2% | 49.125/1.2% | -3.64% | REGRESSED process 4 10 | 63.588/1.6% | 64.254/0.9% | -1.05% | REGRESSED process 4 16 | 106.697/0.6% | 105.565/0.8% | 1.06% | IMPROVED process 4 2 | 8.720/4.4% | 9.915/4.9% | -13.70% | REGRESSED process 4 20 | 132.137/0.9% | 133.264/0.3% | -0.85% | REGRESSED process 4 4 | 22.398/3.8% | 19.267/4.1% | 13.98% | IMPROVED process 4 6 | 33.693/2.8% | 35.592/0.7% | -5.64% | REGRESSED process 4 8 | 50.416/2.5% | 49.572/2.9% | 1.67% | IMPROVED process 8 10 | 63.858/0.9% | 65.193/0.9% | -2.09% | REGRESSED process 8 16 | 106.599/1.0% | 106.779/0.6% | -0.17% | REGRESSED process 8 2 | 9.581/4.7% | 8.549/4.1% | 10.77% | IMPROVED process 8 20 | 134.607/0.4% | 133.946/1.2% | 0.49% | IMPROVED process 8 4 | 20.298/2.6% | 22.213/3.6% | -9.43% | REGRESSED process 8 6 | 36.864/1.3% | 36.373/0.9% | 1.33% | IMPROVED process 8 8 | 51.013/1.6% | 51.373/0.9% | -0.71% | REGRESSED threads 1 10 | 64.432/1.4% | 65.835/2.6% | -2.18% | REGRESSED threads 1 16 | 109.816/3.2% | 99.865/3.9% | 9.06% | IMPROVED threads 1 2 | 7.246/30.6% | 5.060/0.7% | 30.17% | IMPROVED threads 1 20 | 116.748/0.6% | 121.446/4.4% | -4.02% | REGRESSED threads 1 4 | 18.362/20.5% | 11.685/1.5% | 36.36% | IMPROVED threads 1 6 | 31.788/1.7% | 35.312/4.7% | -11.09% | REGRESSED threads 1 8 | 44.421/3.6% | 51.299/6.4% | -15.48% | REGRESSED threads 2 10 | 65.545/4.0% | 70.301/2.3% | -7.26% | REGRESSED threads 2 16 | 96.461/5.7% | 107.936/1.9% | -11.90% | REGRESSED threads 2 2 | 5.634/5.5% | 5.472/2.3% | 2.88% | IMPROVED threads 2 20 | 115.252/2.2% | 134.723/0.5% | -16.89% | REGRESSED threads 2 4 | 23.177/10.5% | 20.270/9.2% | 12.54% | IMPROVED threads 2 6 | 40.291/4.0% | 38.460/3.2% | 4.54% | IMPROVED threads 2 8 | 50.297/6.5% | 49.422/2.8% | 1.74% | IMPROVED threads 4 10 | 60.081/6.4% | 65.470/2.2% | -8.97% | REGRESSED threads 4 16 | 104.858/5.6% | 109.031/1.7% | -3.98% | REGRESSED threads 4 2 | 10.508/4.1% | 10.232/2.7% | 2.63% | IMPROVED threads 4 20 | 136.411/1.2% | 134.034/1.6% | 1.74% | IMPROVED threads 4 4 | 23.156/4.8% | 24.758/3.9% | -6.92% | REGRESSED threads 4 6 | 36.906/3.9% | 40.785/0.6% | -10.51% | REGRESSED threads 4 8 | 48.687/6.0% | 53.476/4.8% | -9.84% | REGRESSED threads 8 10 | 64.297/4.5% | 65.892/0.4% | -2.48% | REGRESSED threads 8 16 | 110.432/0.4% | 109.087/1.6% | 1.22% | IMPROVED threads 8 2 | 9.147/5.5% | 9.948/7.2% | -8.76% | REGRESSED threads 8 20 | 138.656/1.1% | 138.635/0.8% | 0.02% | IMPROVED threads 8 4 | 21.609/4.1% | 24.328/5.5% | -12.58% | REGRESSED threads 8 6 | 42.429/4.2% | 38.484/2.4% | 9.30% | IMPROVED threads 8 8 | 52.500/3.0% | 52.328/0.9% | 0.33% | IMPROVED thanks, Gengkun > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-28 8:53 ` Luo Gengkun @ 2026-07-28 12:08 ` Chen Yu 2026-07-29 9:19 ` Luo Gengkun 0 siblings, 1 reply; 15+ messages in thread From: Chen Yu @ 2026-07-28 12:08 UTC (permalink / raw) To: Luo Gengkun Cc: Chen, Yu C, tim.c.chen, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot On Tue, Jul 28, 2026 at 04:53:59PM +0800, Luo Gengkun wrote: > > On 2026/7/27 9:09, Chen, Yu C wrote: > > On 7/23/2026 12:04 PM, Luo Gengkun wrote: > > > > [ ... ] > > > > > 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) { > > > > In v2 there is a check if the cpu has been set before writing: > > cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) > > https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/ > > do we need to bring that back? > > > I don't think we need it back. Here is why: > > In v2, for_each_cpu was used instead of for_each_cpu_and in the inner loop, > meaning some CPUs being checked might not have been set. Therefore, > cpumask_test_cpu was necessary to filter out those cases. > > Now, with for_each_cpu_and(i, sched_domain_span(sd), &mm->sc_stat.visited_cpus), > we can ensure each scanned CPU is set, so the issue no longer exists. > Furthermore, the only place where the visited_cpus bits are cleared is > task_cache_work(), which is only called once per scan period, there is no > risk of the bit being cleared concurrently mid-loop. > Make sense. > However, is there a possibility that the current task_cache_work() execution > hasn't finished yet when the next scan window arrives? For instance, if the > current task work is heavily delayed or preempted by unexpected interrupt, > jiffies could advance past next_scan before the loop completes. > If we move the `work->next = work;` to the very end of task_cache_work(), > would that resolve this issue? By doing so, the existing `work->next == work` > check in task_tick_cache() should fail and no new task work will be submitted. > > Please let me know if I'm missing something. > There are two layers of protection: first a cheap timeout gate (time_before) that skips scanning until the next period, and then a try_cmpxchg that atomically picks a single winner among the threads that pass the timeout — this actually guarantees only one scanner per mm at a time, no? thanks, Chenyu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-28 12:08 ` : " Chen Yu @ 2026-07-29 9:19 ` Luo Gengkun 2026-07-29 13:48 ` Chen, Yu C 2026-07-29 18:29 ` Tim Chen 0 siblings, 2 replies; 15+ messages in thread From: Luo Gengkun @ 2026-07-29 9:19 UTC (permalink / raw) To: Chen Yu Cc: Chen, Yu C, tim.c.chen, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot On 2026/7/28 20:08, Chen Yu wrote: > On Tue, Jul 28, 2026 at 04:53:59PM +0800, Luo Gengkun wrote: >> >> On 2026/7/27 9:09, Chen, Yu C wrote: >>> On 7/23/2026 12:04 PM, Luo Gengkun wrote: >>> >>> [ ... ] >>> >>>> 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) { >>> >>> In v2 there is a check if the cpu has been set before writing: >>> cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) >>> https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/ >>> do we need to bring that back? >>> >> I don't think we need it back. Here is why: >> >> In v2, for_each_cpu was used instead of for_each_cpu_and in the inner loop, >> meaning some CPUs being checked might not have been set. Therefore, >> cpumask_test_cpu was necessary to filter out those cases. >> >> Now, with for_each_cpu_and(i, sched_domain_span(sd), &mm->sc_stat.visited_cpus), >> we can ensure each scanned CPU is set, so the issue no longer exists. >> Furthermore, the only place where the visited_cpus bits are cleared is >> task_cache_work(), which is only called once per scan period, there is no >> risk of the bit being cleared concurrently mid-loop. >> > > Make sense. > >> However, is there a possibility that the current task_cache_work() execution >> hasn't finished yet when the next scan window arrives? For instance, if the >> current task work is heavily delayed or preempted by unexpected interrupt, >> jiffies could advance past next_scan before the loop completes. >> If we move the `work->next = work;` to the very end of task_cache_work(), >> would that resolve this issue? By doing so, the existing `work->next == work` >> check in task_tick_cache() should fail and no new task work will be submitted. >> >> Please let me know if I'm missing something. >> > > There are two layers of protection: first a cheap timeout gate (time_before) > that skips scanning until the next period, and then a try_cmpxchg that atomically > picks a single winner among the threads that pass the timeout — this actually > guarantees only one scanner per mm at a time, no? What I am worried about is the following scenario: Thread A (CPU 0) Thread B (CPU 1) ================ ================ task_cache_work() | +-> try_cmpxchg() == true | (Sets next_scan = now + 10) | +-> Enters Scanning Loop (jiffies = 100) | [ Delayed / Preempted ] | jiffies advances 100 -> 115. | Thread A STILL in the loop! | task_cache_work() (jiffies = 115) | | | +-> next_scan == 110 (pass timeout check) | | | +-> try_cmpxchg() == true | | (Sets next_scan = 115 + 10) | | In other words, concurrent execution of task_cache_work() from two adjacent periods can occur under extreme conditions; do we need to take this scenario into consideration? Perhaps we need an explicit state flag (e.g., using test_and_set_bit) to ensure that the previous task_cache_work() execution has fully completed before allowing a new thread to proceed, regardless of whether the next scan window has arrived. What do you think? thanks, Gengkun > > thanks, > Chenyu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-29 9:19 ` Luo Gengkun @ 2026-07-29 13:48 ` Chen, Yu C 2026-07-29 18:29 ` Tim Chen 1 sibling, 0 replies; 15+ messages in thread From: Chen, Yu C @ 2026-07-29 13:48 UTC (permalink / raw) To: Luo Gengkun Cc: tim.c.chen, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot, Chen Yu On 7/29/2026 5:19 PM, Luo Gengkun wrote: >>> However, is there a possibility that the current task_cache_work() >>> execution >>> hasn't finished yet when the next scan window arrives? For instance, >>> if the >>> current task work is heavily delayed or preempted by unexpected >>> interrupt, >>> jiffies could advance past next_scan before the loop completes. >>> If we move the `work->next = work;` to the very end of >>> task_cache_work(), >>> would that resolve this issue? By doing so, the existing `work->next >>> == work` >>> check in task_tick_cache() should fail and no new task work will be >>> submitted. >>> >>> Please let me know if I'm missing something. >>> >> >> There are two layers of protection: first a cheap timeout gate >> (time_before) >> that skips scanning until the next period, and then a try_cmpxchg that >> atomically >> picks a single winner among the threads that pass the timeout — this >> actually >> guarantees only one scanner per mm at a time, no? > > What I am worried about is the following scenario: > > Thread A (CPU 0) Thread B (CPU 1) > ================ ================ > task_cache_work() > | > +-> try_cmpxchg() == true > | (Sets next_scan = now + 10) > | > +-> Enters Scanning Loop (jiffies = 100) > | [ Delayed / Preempted ] > | jiffies advances 100 -> 115. > | Thread A STILL in the loop! > | task_cache_work() (jiffies > = 115) > > | | > | +-> next_scan == 110 > (pass timeout check) > > | | > | +-> try_cmpxchg() == true > > | | (Sets next_scan = > 115 + 10) > | | > > In other words, concurrent execution of task_cache_work() from two > adjacent periods can > occur under extreme conditions; Nice, this sequence is very clear, thanks. > do we need to take this scenario into > consideration? > > Perhaps we need an explicit state flag (e.g., using test_and_set_bit) to > ensure > that the previous task_cache_work() execution has fully completed before > allowing > a new thread to proceed, regardless of whether the next scan window has > arrived. > What do you think? > The scan iterates over a lock-free snapshot of visited_cpus, so even if account_mm_sched() concurrently adds more bits, missing them is acceptable. The same goes for concurrent shrinking by task_cache_work(). That is to say, in this rare case, a "duplicated" cpumask_clear() in fraction_mm_sched() might be tolerated IMO. How about adjusting the comment from /* only 1 thread is allowed to scan */ to /* elect a single scanner per epoch */ in your next version? thanks, Chenyu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-29 9:19 ` Luo Gengkun 2026-07-29 13:48 ` Chen, Yu C @ 2026-07-29 18:29 ` Tim Chen 2026-07-30 2:22 ` Tim Chen 1 sibling, 1 reply; 15+ messages in thread From: Tim Chen @ 2026-07-29 18:29 UTC (permalink / raw) To: Luo Gengkun, Chen Yu Cc: Chen, Yu C, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot On Wed, 2026-07-29 at 17:19 +0800, Luo Gengkun wrote: > > On 2026/7/28 20:08, Chen Yu wrote: > > On Tue, Jul 28, 2026 at 04:53:59PM +0800, Luo Gengkun wrote: > > > > > > On 2026/7/27 9:09, Chen, Yu C wrote: > > > > On 7/23/2026 12:04 PM, Luo Gengkun wrote: > > > > > > > > [ ... ] > > > > > > > > > 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) { > > > > > > > > In v2 there is a check if the cpu has been set before writing: > > > > cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) > > > > https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/ > > > > do we need to bring that back? > > > > > > > I don't think we need it back. Here is why: > > > > > > In v2, for_each_cpu was used instead of for_each_cpu_and in the inner loop, > > > meaning some CPUs being checked might not have been set. Therefore, > > > cpumask_test_cpu was necessary to filter out those cases. > > > > > > Now, with for_each_cpu_and(i, sched_domain_span(sd), &mm->sc_stat.visited_cpus), > > > we can ensure each scanned CPU is set, so the issue no longer exists. > > > Furthermore, the only place where the visited_cpus bits are cleared is > > > task_cache_work(), which is only called once per scan period, there is no > > > risk of the bit being cleared concurrently mid-loop. > > > > > > > Make sense. > > > > > However, is there a possibility that the current task_cache_work() execution > > > hasn't finished yet when the next scan window arrives? For instance, if the > > > current task work is heavily delayed or preempted by unexpected interrupt, > > > jiffies could advance past next_scan before the loop completes. > > > If we move the `work->next = work;` to the very end of task_cache_work(), > > > would that resolve this issue? By doing so, the existing `work->next == work` > > > check in task_tick_cache() should fail and no new task work will be submitted. > > > > > > Please let me know if I'm missing something. > > > > > > > There are two layers of protection: first a cheap timeout gate (time_before) > > that skips scanning until the next period, and then a try_cmpxchg that atomically > > picks a single winner among the threads that pass the timeout — this actually > > guarantees only one scanner per mm at a time, no? > > What I am worried about is the following scenario: > > Thread A (CPU 0) Thread B (CPU 1) > ================ ================ > task_cache_work() > | > +-> try_cmpxchg() == true > | (Sets next_scan = now + 10) > | > +-> Enters Scanning Loop (jiffies = 100) > | [ Delayed / Preempted ] > | jiffies advances 100 -> 115. > | Thread A STILL in the loop! > | task_cache_work() (jiffies = 115) > > | | > | +-> next_scan == 110 (pass timeout check) > > | | > | +-> try_cmpxchg() == true > > | | (Sets next_scan = 115 + 10) > | | > > In other words, concurrent execution of task_cache_work() from two adjacent periods can > occur under extreme conditions; do we need to take this scenario into consideration? > > Perhaps we need an explicit state flag (e.g., using test_and_set_bit) to ensure > that the previous task_cache_work() execution has fully completed before allowing > a new thread to proceed, regardless of whether the next scan window has arrived. > What do you think? First, the EPOCH_PERIOD is fairly long (10 msec) so it is unlikely that Thread A hasn't completed its work. Second, suppose the above scenario happened, the two thread above are serialized in the work on updating occupancy and visited cpus under the cpus_read_lock when looping through the cpus in task_cache_work(). So I think we should be okay without an explicit state flag. That said, I think we should use mm->sc_stat.lock instead of cpus_read_lock in task_cache_work()'s cpu loop. That will improve scalability. Tim > > thanks, > Gengkun > > > > > thanks, > > Chenyu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-29 18:29 ` Tim Chen @ 2026-07-30 2:22 ` Tim Chen 2026-07-30 13:40 ` Luo Gengkun 0 siblings, 1 reply; 15+ messages in thread From: Tim Chen @ 2026-07-30 2:22 UTC (permalink / raw) To: Luo Gengkun, Chen Yu Cc: Chen, Yu C, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot On Wed, 2026-07-29 at 11:29 -0700, Tim Chen wrote: > On Wed, 2026-07-29 at 17:19 +0800, Luo Gengkun wrote: > > > > On 2026/7/28 20:08, Chen Yu wrote: > > > On Tue, Jul 28, 2026 at 04:53:59PM +0800, Luo Gengkun wrote: > > > > > > > > On 2026/7/27 9:09, Chen, Yu C wrote: > > > > > On 7/23/2026 12:04 PM, Luo Gengkun wrote: > > > > > > > > > > [ ... ] > > > > > > > > > > > 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) { > > > > > > > > > > In v2 there is a check if the cpu has been set before writing: > > > > > cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) > > > > > https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/ > > > > > do we need to bring that back? > > > > > > > > > I don't think we need it back. Here is why: > > > > > > > > In v2, for_each_cpu was used instead of for_each_cpu_and in the inner loop, > > > > meaning some CPUs being checked might not have been set. Therefore, > > > > cpumask_test_cpu was necessary to filter out those cases. > > > > > > > > Now, with for_each_cpu_and(i, sched_domain_span(sd), &mm->sc_stat.visited_cpus), > > > > we can ensure each scanned CPU is set, so the issue no longer exists. > > > > Furthermore, the only place where the visited_cpus bits are cleared is > > > > task_cache_work(), which is only called once per scan period, there is no > > > > risk of the bit being cleared concurrently mid-loop. > > > > > > > > > > Make sense. > > > > > > > However, is there a possibility that the current task_cache_work() execution > > > > hasn't finished yet when the next scan window arrives? For instance, if the > > > > current task work is heavily delayed or preempted by unexpected interrupt, > > > > jiffies could advance past next_scan before the loop completes. > > > > If we move the `work->next = work;` to the very end of task_cache_work(), > > > > would that resolve this issue? By doing so, the existing `work->next == work` > > > > check in task_tick_cache() should fail and no new task work will be submitted. > > > > > > > > Please let me know if I'm missing something. > > > > > > > > > > There are two layers of protection: first a cheap timeout gate (time_before) > > > that skips scanning until the next period, and then a try_cmpxchg that atomically > > > picks a single winner among the threads that pass the timeout — this actually > > > guarantees only one scanner per mm at a time, no? > > > > What I am worried about is the following scenario: > > > > Thread A (CPU 0) Thread B (CPU 1) > > ================ ================ > > task_cache_work() > > | > > +-> try_cmpxchg() == true > > | (Sets next_scan = now + 10) > > | > > +-> Enters Scanning Loop (jiffies = 100) > > | [ Delayed / Preempted ] > > | jiffies advances 100 -> 115. > > | Thread A STILL in the loop! > > | task_cache_work() (jiffies = 115) > > > > | | > > | +-> next_scan == 110 (pass timeout check) > > > > | | > > | +-> try_cmpxchg() == true > > > > | | (Sets next_scan = 115 + 10) > > | | > > > > In other words, concurrent execution of task_cache_work() from two adjacent periods can > > occur under extreme conditions; do we need to take this scenario into consideration? > > > > Perhaps we need an explicit state flag (e.g., using test_and_set_bit) to ensure > > that the previous task_cache_work() execution has fully completed before allowing > > a new thread to proceed, regardless of whether the next scan window has arrived. > > What do you think? > > First, the EPOCH_PERIOD is fairly long (10 msec) so it is unlikely that Thread A > hasn't completed its work. > > Second, suppose the above scenario happened, the two thread above > are serialized in the work on updating occupancy > and visited cpus under the cpus_read_lock when looping through the cpus > in task_cache_work(). > > So I think we should be okay without an explicit state flag. > > That said, I think we should use mm->sc_stat.lock instead of cpus_read_lock > in task_cache_work()'s cpu loop. That will improve scalability. It slipped my mind the cpus_read_lock is read lock, so the two threads could indeed go in parallel. But the update on each cpu is protected by rq->cpu_epoch_lock, and epochs don't go backwards. So we should still get consistent updates on epochs and occupancy stats. Tim ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-30 2:22 ` Tim Chen @ 2026-07-30 13:40 ` Luo Gengkun 2026-07-30 16:57 ` Tim Chen 0 siblings, 1 reply; 15+ messages in thread From: Luo Gengkun @ 2026-07-30 13:40 UTC (permalink / raw) To: Tim Chen, Chen Yu Cc: Chen, Yu C, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot On 2026/7/30 10:22, Tim Chen wrote: > On Wed, 2026-07-29 at 11:29 -0700, Tim Chen wrote: >> On Wed, 2026-07-29 at 17:19 +0800, Luo Gengkun wrote: >>> >>> On 2026/7/28 20:08, Chen Yu wrote: >>>> On Tue, Jul 28, 2026 at 04:53:59PM +0800, Luo Gengkun wrote: >>>>> >>>>> On 2026/7/27 9:09, Chen, Yu C wrote: >>>>>> On 7/23/2026 12:04 PM, Luo Gengkun wrote: >>>>>> >>>>>> [ ... ] >>>>>> >>>>>>> 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) { >>>>>> >>>>>> In v2 there is a check if the cpu has been set before writing: >>>>>> cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) >>>>>> https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/ >>>>>> do we need to bring that back? >>>>>> >>>>> I don't think we need it back. Here is why: >>>>> >>>>> In v2, for_each_cpu was used instead of for_each_cpu_and in the inner loop, >>>>> meaning some CPUs being checked might not have been set. Therefore, >>>>> cpumask_test_cpu was necessary to filter out those cases. >>>>> >>>>> Now, with for_each_cpu_and(i, sched_domain_span(sd), &mm->sc_stat.visited_cpus), >>>>> we can ensure each scanned CPU is set, so the issue no longer exists. >>>>> Furthermore, the only place where the visited_cpus bits are cleared is >>>>> task_cache_work(), which is only called once per scan period, there is no >>>>> risk of the bit being cleared concurrently mid-loop. >>>>> >>>> >>>> Make sense. >>>> >>>>> However, is there a possibility that the current task_cache_work() execution >>>>> hasn't finished yet when the next scan window arrives? For instance, if the >>>>> current task work is heavily delayed or preempted by unexpected interrupt, >>>>> jiffies could advance past next_scan before the loop completes. >>>>> If we move the `work->next = work;` to the very end of task_cache_work(), >>>>> would that resolve this issue? By doing so, the existing `work->next == work` >>>>> check in task_tick_cache() should fail and no new task work will be submitted. >>>>> >>>>> Please let me know if I'm missing something. >>>>> >>>> >>>> There are two layers of protection: first a cheap timeout gate (time_before) >>>> that skips scanning until the next period, and then a try_cmpxchg that atomically >>>> picks a single winner among the threads that pass the timeout — this actually >>>> guarantees only one scanner per mm at a time, no? >>> >>> What I am worried about is the following scenario: >>> >>> Thread A (CPU 0) Thread B (CPU 1) >>> ================ ================ >>> task_cache_work() >>> | >>> +-> try_cmpxchg() == true >>> | (Sets next_scan = now + 10) >>> | >>> +-> Enters Scanning Loop (jiffies = 100) >>> | [ Delayed / Preempted ] >>> | jiffies advances 100 -> 115. >>> | Thread A STILL in the loop! >>> | task_cache_work() (jiffies = 115) >>> >>> | | >>> | +-> next_scan == 110 (pass timeout check) >>> >>> | | >>> | +-> try_cmpxchg() == true >>> >>> | | (Sets next_scan = 115 + 10) >>> | | >>> >>> In other words, concurrent execution of task_cache_work() from two adjacent periods can >>> occur under extreme conditions; do we need to take this scenario into consideration? >>> >>> Perhaps we need an explicit state flag (e.g., using test_and_set_bit) to ensure >>> that the previous task_cache_work() execution has fully completed before allowing >>> a new thread to proceed, regardless of whether the next scan window has arrived. >>> What do you think? >> >> First, the EPOCH_PERIOD is fairly long (10 msec) so it is unlikely that Thread A >> hasn't completed its work. >> >> Second, suppose the above scenario happened, the two thread above >> are serialized in the work on updating occupancy >> and visited cpus under the cpus_read_lock when looping through the cpus >> in task_cache_work(). >> >> So I think we should be okay without an explicit state flag. >> >> That said, I think we should use mm->sc_stat.lock instead of cpus_read_lock >> in task_cache_work()'s cpu loop. That will improve scalability. > > It slipped my mind the cpus_read_lock is read lock, so the two > threads could indeed go in parallel. > > But the update on each cpu is protected by rq->cpu_epoch_lock, and > epochs don't go backwards. So we should still get consistent updates on > epochs and occupancy stats. > > Tim When talking about mm->sc_stat.lock, I found an issue that may be worth paying attention. Below is the relevant code snippet: task_tick_cache() { ... /* avoid moving backwards */ if (time_after_eq(mm->sc_stat.epoch, epoch)) return; guard(raw_spinlock)(&mm->sc_stat.lock); if (work->next == work) { task_work_add(p, work, TWA_RESUME); WRITE_ONCE(mm->sc_stat.epoch, epoch); } .. } Actually, I don't think time_after_eq() can effectively avoid moving backwards because this check is performed entirely outside the protection of the spinlock. The following sequence diagram describes this race condition in detail: Thread A (CPU 0) Thread B (CPU 1) ============================ ============================ [ Initial State: mm->sc_stat.epoch = 90 ] task_tick_cache() task_tick_cache() | | +-> Read rq->cpu_epoch = 100 +-> Read rq->cpu_epoch = 101 | | +-> Lockless Check (90, 100) -> PASS +-> Lockless Check (90, 101) -> PASS | | | +-> Acquires lock first | +-> Writes mm->sc_stat.epoch = 101 | +-> Releases lock | [ mm->sc_stat.epoch is now 101 ] | +-> Acquires lock | +-> work->next == work (STILL TRUE! | Because cache_work is per-thread, Thread B's | submission cannot clear Thread A's local state) | +-> WRITE_ONCE(mm->sc_stat.epoch, 100) !!! <-- BUG: Epoch moves backwards! | The solution is straightforward: we should validate the epoch sequence under the protection of the lock. Furthermore, we should use a trylock here instead of a blocking spinlock to eliminate busy-waiting in the sensitive timer tick path. Since the lock being held implies that another thread is already submitting the cache work, it is safe to skip this tick and return immediately if the trylock fails. Below is output from lock stat: lock_stat version 0.4 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- class name con-bounces contentions waittime-min waittime-max waittime-total waittime-avg acq-bounces acquisitions holdtime-min holdtime-max holdtime-total holdtime-avg ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- &mm->sc_stat.lock: 122711 122883 0.12 136.54 1493110.82 12.15 152643 167402 0.12 12.87 99842.76 0.60 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -1763,15 +1763,23 @@ static void task_tick_cache(struct rq *rq, struct task_struct *p) return; epoch = rq->cpu_epoch; - /* avoid moving backwards */ - if (time_after_eq(mm->sc_stat.epoch, epoch)) - return; - guard(raw_spinlock)(&mm->sc_stat.lock); + /* + * Use trylock instead of a blocking lock to avoid spinning in the + * sensitive timer tick/scheduler path. If the lock is contended, + * which mean another thread is submitting the work. In this case + * skip this tick to reduce redundant cache_work submissions within + * the same epoch. + */ + scoped_cond_guard(raw_spinlock_try, return, &mm->sc_stat.lock) { + /* avoid moving backwards */ + if (time_after_eq(mm->sc_stat.epoch, epoch)) + return; - if (work->next == work) { - task_work_add(p, work, TWA_RESUME); - WRITE_ONCE(mm->sc_stat.epoch, epoch); + if (work->next == work) { + task_work_add(p, work, TWA_RESUME); + WRITE_ONCE(mm->sc_stat.epoch, epoch); + } } } The above code prevents the epoch from moving backward and reduces lock contention. What do you think? thanks, Gengkun > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-30 13:40 ` Luo Gengkun @ 2026-07-30 16:57 ` Tim Chen 2026-07-31 6:58 ` Chen, Yu C 0 siblings, 1 reply; 15+ messages in thread From: Tim Chen @ 2026-07-30 16:57 UTC (permalink / raw) To: Luo Gengkun, Chen Yu Cc: Chen, Yu C, dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot On Thu, 2026-07-30 at 21:40 +0800, Luo Gengkun wrote: > > On 2026/7/30 10:22, Tim Chen wrote: > > On Wed, 2026-07-29 at 11:29 -0700, Tim Chen wrote: > > > On Wed, 2026-07-29 at 17:19 +0800, Luo Gengkun wrote: > > > > > > > > On 2026/7/28 20:08, Chen Yu wrote: > > > > > On Tue, Jul 28, 2026 at 04:53:59PM +0800, Luo Gengkun wrote: > > > > > > > > > > > > On 2026/7/27 9:09, Chen, Yu C wrote: > > > > > > > On 7/23/2026 12:04 PM, Luo Gengkun wrote: > > > > > > > > > > > > > > [ ... ] > > > > > > > > > > > > > > > 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) { > > > > > > > > > > > > > > In v2 there is a check if the cpu has been set before writing: > > > > > > > cpumask_test_cpu(cpu_of(rq), &mm->sc_stat.visited_cpus) > > > > > > > https://lore.kernel.org/all/20260414150745.225416-1-luogengkun2@huawei.com/ > > > > > > > do we need to bring that back? > > > > > > > > > > > > > I don't think we need it back. Here is why: > > > > > > > > > > > > In v2, for_each_cpu was used instead of for_each_cpu_and in the inner loop, > > > > > > meaning some CPUs being checked might not have been set. Therefore, > > > > > > cpumask_test_cpu was necessary to filter out those cases. > > > > > > > > > > > > Now, with for_each_cpu_and(i, sched_domain_span(sd), &mm->sc_stat.visited_cpus), > > > > > > we can ensure each scanned CPU is set, so the issue no longer exists. > > > > > > Furthermore, the only place where the visited_cpus bits are cleared is > > > > > > task_cache_work(), which is only called once per scan period, there is no > > > > > > risk of the bit being cleared concurrently mid-loop. > > > > > > > > > > > > > > > > Make sense. > > > > > > > > > > > However, is there a possibility that the current task_cache_work() execution > > > > > > hasn't finished yet when the next scan window arrives? For instance, if the > > > > > > current task work is heavily delayed or preempted by unexpected interrupt, > > > > > > jiffies could advance past next_scan before the loop completes. > > > > > > If we move the `work->next = work;` to the very end of task_cache_work(), > > > > > > would that resolve this issue? By doing so, the existing `work->next == work` > > > > > > check in task_tick_cache() should fail and no new task work will be submitted. > > > > > > > > > > > > Please let me know if I'm missing something. > > > > > > > > > > > > > > > > There are two layers of protection: first a cheap timeout gate (time_before) > > > > > that skips scanning until the next period, and then a try_cmpxchg that atomically > > > > > picks a single winner among the threads that pass the timeout — this actually > > > > > guarantees only one scanner per mm at a time, no? > > > > > > > > What I am worried about is the following scenario: > > > > > > > > Thread A (CPU 0) Thread B (CPU 1) > > > > ================ ================ > > > > task_cache_work() > > > > | > > > > +-> try_cmpxchg() == true > > > > | (Sets next_scan = now + 10) > > > > | > > > > +-> Enters Scanning Loop (jiffies = 100) > > > > | [ Delayed / Preempted ] > > > > | jiffies advances 100 -> 115. > > > > | Thread A STILL in the loop! > > > > | task_cache_work() (jiffies = 115) > > > > > > > > | | > > > > | +-> next_scan == 110 (pass timeout check) > > > > > > > > | | > > > > | +-> try_cmpxchg() == true > > > > > > > > | | (Sets next_scan = 115 + 10) > > > > | | > > > > > > > > In other words, concurrent execution of task_cache_work() from two adjacent periods can > > > > occur under extreme conditions; do we need to take this scenario into consideration? > > > > > > > > Perhaps we need an explicit state flag (e.g., using test_and_set_bit) to ensure > > > > that the previous task_cache_work() execution has fully completed before allowing > > > > a new thread to proceed, regardless of whether the next scan window has arrived. > > > > What do you think? > > > > > > First, the EPOCH_PERIOD is fairly long (10 msec) so it is unlikely that Thread A > > > hasn't completed its work. > > > > > > Second, suppose the above scenario happened, the two thread above > > > are serialized in the work on updating occupancy > > > and visited cpus under the cpus_read_lock when looping through the cpus > > > in task_cache_work(). > > > > > > So I think we should be okay without an explicit state flag. > > > > > > That said, I think we should use mm->sc_stat.lock instead of cpus_read_lock > > > in task_cache_work()'s cpu loop. That will improve scalability. > > > > It slipped my mind the cpus_read_lock is read lock, so the two > > threads could indeed go in parallel. > > > > But the update on each cpu is protected by rq->cpu_epoch_lock, and > > epochs don't go backwards. So we should still get consistent updates on > > epochs and occupancy stats. > > > > Tim > > When talking about mm->sc_stat.lock, I found an issue that may be > worth paying attention. Below is the relevant code snippet: > > task_tick_cache() > { > ... > /* avoid moving backwards */ > if (time_after_eq(mm->sc_stat.epoch, epoch)) > return; > > guard(raw_spinlock)(&mm->sc_stat.lock); > > if (work->next == work) { > task_work_add(p, work, TWA_RESUME); > WRITE_ONCE(mm->sc_stat.epoch, epoch); > } > .. > } > Actually, I don't think time_after_eq() can effectively avoid moving > backwards because this check is performed entirely outside the > protection of the spinlock. The following sequence diagram describes > this race condition in detail: > > Thread A (CPU 0) Thread B (CPU 1) > ============================ ============================ > [ Initial State: mm->sc_stat.epoch = 90 ] > > task_tick_cache() task_tick_cache() > > | | > +-> Read rq->cpu_epoch = 100 +-> Read rq->cpu_epoch = 101 > > | | > +-> Lockless Check (90, 100) -> PASS +-> Lockless Check (90, 101) -> PASS > > | | > | +-> Acquires lock first > | +-> Writes mm->sc_stat.epoch = 101 > | +-> Releases lock > | [ mm->sc_stat.epoch is now 101 ] > | > +-> Acquires lock > | > +-> work->next == work (STILL TRUE! > | Because cache_work is per-thread, Thread B's > | submission cannot clear Thread A's local state) > | > +-> WRITE_ONCE(mm->sc_stat.epoch, 100) !!! <-- BUG: Epoch moves backwards! That would not happen because in __update_mm_sched(), Thread A will be reading the updated rq->cpu_epoch and again check whether time is moving backwards and will skip the update if that's the case. > | > > The solution is straightforward: we should validate the epoch sequence > under the protection of the lock. > You do not want to acquire the lock before the time check. As account_mm_sched() will happen very often and you will to skip those unnecessary lock acquisitions when rq epoch is not yet due for an update. Thanks. Tim > Furthermore, we should use a trylock here instead of a blocking spinlock > to eliminate busy-waiting in the sensitive timer tick path. Since the > lock being held implies that another thread is already submitting the > cache work, it is safe to skip this tick and return immediately if the > trylock fails. > > Below is output from lock stat: > > lock_stat version 0.4 > ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- > class name con-bounces contentions waittime-min waittime-max waittime-total waittime-avg acq-bounces acquisitions holdtime-min holdtime-max holdtime-total holdtime-avg > ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- > &mm->sc_stat.lock: 122711 122883 0.12 136.54 1493110.82 12.15 152643 167402 0.12 12.87 99842.76 0.60 > > > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -1763,15 +1763,23 @@ static void task_tick_cache(struct rq *rq, struct task_struct *p) > return; > > epoch = rq->cpu_epoch; > - /* avoid moving backwards */ > - if (time_after_eq(mm->sc_stat.epoch, epoch)) > - return; > > - guard(raw_spinlock)(&mm->sc_stat.lock); > + /* > + * Use trylock instead of a blocking lock to avoid spinning in the > + * sensitive timer tick/scheduler path. If the lock is contended, > + * which mean another thread is submitting the work. In this case > + * skip this tick to reduce redundant cache_work submissions within > + * the same epoch. > + */ > + scoped_cond_guard(raw_spinlock_try, return, &mm->sc_stat.lock) { > + /* avoid moving backwards */ > + if (time_after_eq(mm->sc_stat.epoch, epoch)) > + return; > > - if (work->next == work) { > - task_work_add(p, work, TWA_RESUME); > - WRITE_ONCE(mm->sc_stat.epoch, epoch); > + if (work->next == work) { > + task_work_add(p, work, TWA_RESUME); > + WRITE_ONCE(mm->sc_stat.epoch, epoch); > + } > } > } > > The above code prevents the epoch from moving backward and > reduces lock contention. > > What do you think? > > thanks, > Gengkun > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-30 16:57 ` Tim Chen @ 2026-07-31 6:58 ` Chen, Yu C 2026-07-31 8:31 ` Luo Gengkun 0 siblings, 1 reply; 15+ messages in thread From: Chen, Yu C @ 2026-07-31 6:58 UTC (permalink / raw) To: Tim Chen, Luo Gengkun Cc: dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot, Chen Yu On 7/31/2026 12:57 AM, Tim Chen wrote: [ ... ] >> >> When talking about mm->sc_stat.lock, I found an issue that may be >> worth paying attention. Below is the relevant code snippet: >> >> task_tick_cache() >> { >> ... >> /* avoid moving backwards */ >> if (time_after_eq(mm->sc_stat.epoch, epoch)) >> return; >> >> guard(raw_spinlock)(&mm->sc_stat.lock); >> >> if (work->next == work) { >> task_work_add(p, work, TWA_RESUME); >> WRITE_ONCE(mm->sc_stat.epoch, epoch); >> } >> .. >> } >> Actually, I don't think time_after_eq() can effectively avoid moving >> backwards because this check is performed entirely outside the >> protection of the spinlock. The following sequence diagram describes >> this race condition in detail: >> >> Thread A (CPU 0) Thread B (CPU 1) >> ============================ ============================ >> [ Initial State: mm->sc_stat.epoch = 90 ] >> >> task_tick_cache() task_tick_cache() >> >> | | >> +-> Read rq->cpu_epoch = 100 +-> Read rq->cpu_epoch = 101 >> >> | | >> +-> Lockless Check (90, 100) -> PASS +-> Lockless Check (90, 101) -> PASS >> >> | | >> | +-> Acquires lock first >> | +-> Writes mm->sc_stat.epoch = 101 >> | +-> Releases lock >> | [ mm->sc_stat.epoch is now 101 ] >> | >> +-> Acquires lock >> | >> +-> work->next == work (STILL TRUE! >> | Because cache_work is per-thread, Thread B's >> | submission cannot clear Thread A's local state) >> | >> +-> WRITE_ONCE(mm->sc_stat.epoch, 100) !!! <-- BUG: Epoch moves backwards! > > > That would not happen because in __update_mm_sched(), Thread A will be > reading the updated rq->cpu_epoch and again check > whether time is moving backwards and will skip the update if > that's the case. > > Maybe the backward comment in the code was a little confusing. The "avoid moving backwards" logic was introduced to prevent a negative timeout value in commit df0d98475954: if (epoch - READ_ONCE(mm->sc_stat.epoch) > EPOCH_LLC_AFFINITY_TIMEOUT) That is to say, by design, we want mm->sc_stat.epoch to chase after the CPU's epoch and never jump ahead of any CPU's epoch. Otherwise, the subtraction above could result in a huge value. Later, in commit c1e7fe5e75ed, that negative delta was avoided by: if ((long)(epoch - READ_ONCE(mm->sc_stat.epoch)) So now, mm->sc_stat.epoch is only best-effort to not go backward. If it actually does go backward, in my opinion it’s not a big deal. thanks, Chenyu ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-31 6:58 ` Chen, Yu C @ 2026-07-31 8:31 ` Luo Gengkun 0 siblings, 0 replies; 15+ messages in thread From: Luo Gengkun @ 2026-07-31 8:31 UTC (permalink / raw) To: Chen, Yu C, Tim Chen Cc: dietmar.eggemann, rostedt, peterz, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel, mingo, juri.lelli, vincent.guittot, Chen Yu On 2026/7/31 14:58, Chen, Yu C wrote: > On 7/31/2026 12:57 AM, Tim Chen wrote: > > [ ... ] > >>> >>> When talking about mm->sc_stat.lock, I found an issue that may be >>> worth paying attention. Below is the relevant code snippet: >>> >>> task_tick_cache() >>> { >>> ... >>> /* avoid moving backwards */ >>> if (time_after_eq(mm->sc_stat.epoch, epoch)) >>> return; >>> >>> guard(raw_spinlock)(&mm->sc_stat.lock); >>> >>> if (work->next == work) { >>> task_work_add(p, work, TWA_RESUME); >>> WRITE_ONCE(mm->sc_stat.epoch, epoch); >>> } >>> .. >>> } >>> Actually, I don't think time_after_eq() can effectively avoid moving >>> backwards because this check is performed entirely outside the >>> protection of the spinlock. The following sequence diagram describes >>> this race condition in detail: >>> >>> Thread A (CPU 0) Thread B (CPU 1) >>> ============================ ============================ >>> [ Initial State: mm->sc_stat.epoch = 90 ] >>> task_tick_cache() task_tick_cache() >>> | | >>> +-> Read rq->cpu_epoch = 100 +-> Read rq->cpu_epoch = 101 >>> | | >>> +-> Lockless Check (90, 100) -> PASS +-> Lockless Check (90, 101) -> PASS >>> | | >>> | +-> Acquires lock first >>> | +-> Writes mm->sc_stat.epoch = 101 >>> | +-> Releases lock >>> | [ mm->sc_stat.epoch is now 101 ] >>> | >>> +-> Acquires lock >>> | >>> +-> work->next == work (STILL TRUE! >>> | Because cache_work is per-thread, Thread B's >>> | submission cannot clear Thread A's local state) >>> | >>> +-> WRITE_ONCE(mm->sc_stat.epoch, 100) !!! <-- BUG: Epoch moves backwards! >> >> >> That would not happen because in __update_mm_sched(), Thread A will be >> reading the updated rq->cpu_epoch and again check >> whether time is moving backwards and will skip the update if >> that's the case. >> >> > > Maybe the backward comment in the code was a little confusing. > The "avoid moving backwards" logic was introduced to prevent a negative > timeout value in commit df0d98475954: > > if (epoch - READ_ONCE(mm->sc_stat.epoch) > EPOCH_LLC_AFFINITY_TIMEOUT) > > That is to say, by design, we want mm->sc_stat.epoch to chase after the > CPU's epoch and never jump ahead of any CPU's epoch. Otherwise, the subtraction > above could result in a huge value. > > Later, in commit c1e7fe5e75ed, that negative delta was avoided by: > > if ((long)(epoch - READ_ONCE(mm->sc_stat.epoch)) > So now, mm->sc_stat.epoch is only best-effort to not go backward. If it > actually does go backward, in my opinion it’s not a big deal. > > thanks, > Chenyu > > Thank you for your clarifying explanation :) Gengkun > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus 2026-07-23 4:04 ` [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus Luo Gengkun 2026-07-27 1:09 ` Chen, Yu C @ 2026-07-28 20:16 ` Tim Chen 1 sibling, 0 replies; 15+ messages in thread From: Tim Chen @ 2026-07-28 20:16 UTC (permalink / raw) To: Luo Gengkun, peterz, mingo, juri.lelli, vincent.guittot, yu.c.chen Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel On Thu, 2026-07-23 at 04:04 +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. > > Now that we know exactly which CPUs to scan from visited_cpus, we can remove > get_scan_cpumasks(). Thanks. The code looks good to me. Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com> > > Signed-off-by: Luo Gengkun <luogengkun2@huawei.com> > --- > include/linux/mm_types.h | 6 +++ > include/linux/sched.h | 2 + > kernel/sched/fair.c | 94 ++++++++++++++++------------------------ > 3 files changed, 46 insertions(+), 56 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..10d442074f21 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; > + if (!cpumask_test_cpu(cpu_of(rq), mm->sc_stat.visited_cpus)) > + cpumask_set_cpu(cpu_of(rq), mm->sc_stat.visited_cpus); > } > > /* > @@ -1761,51 +1775,6 @@ static void task_tick_cache(struct rq *rq, struct task_struct *p) > } > } > > -static void get_scan_cpumasks(cpumask_var_t cpus, struct task_struct *p) > -{ > -#ifdef CONFIG_NUMA_BALANCING > - int cpu, curr_cpu, nid, pref_nid; > - > - if (!static_branch_likely(&sched_numa_balancing)) > - goto out; > - > - cpu = READ_ONCE(p->mm->sc_stat.cpu); > - if (cpu != -1) > - nid = cpu_to_node(cpu); > - curr_cpu = task_cpu(p); > - > - /* > - * Scanning in the preferred NUMA node is ideal. However, the NUMA > - * preferred node is per-task rather than per-process. It is possible > - * for different threads of the process to have distinct preferred > - * nodes; consequently, the process-wide preferred LLC may bounce > - * between different nodes. As a workaround, maintain the scan > - * CPU mask to also cover the process's current preferred LLC and the > - * current running node to mitigate the bouncing risk. > - * TBD: numa_group should be considered during task aggregation. > - */ > - pref_nid = p->numa_preferred_nid; > - /* honor the task's preferred node */ > - if (pref_nid == NUMA_NO_NODE) > - goto out; > - > - cpumask_or(cpus, cpus, cpumask_of_node(pref_nid)); > - > - /* honor the task's preferred LLC CPU */ > - if (cpu != -1 && !cpumask_test_cpu(cpu, cpus) && nid != NUMA_NO_NODE) > - cpumask_or(cpus, cpus, cpumask_of_node(nid)); > - > - /* make sure the task's current running node is included */ > - if (!cpumask_test_cpu(curr_cpu, cpus)) > - cpumask_or(cpus, cpus, cpumask_of_node(cpu_to_node(curr_cpu))); > - > - return; > - > -out: > -#endif > - cpumask_copy(cpus, cpu_online_mask); > -} > - > static inline void update_avg_scale(u64 *avg, u64 sample) > { > int factor = per_cpu(sd_llc_size, raw_smp_processor_id()); > @@ -1866,7 +1835,18 @@ static void task_cache_work(struct callback_head *work) > scoped_guard (cpus_read_lock) { > guard(rcu)(); > > - get_scan_cpumasks(cpus, p); > + /* > + * 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, cpu_online_mask, mm->sc_stat.visited_cpus); > > for_each_cpu(cpu, cpus) { > /* XXX sched_cluster_active */ > @@ -1877,19 +1857,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++; > } > > /* ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v8 2/2] -- DO NOT APPLY!!! -- sched/cache/debug: Add trace event and sched feature to track scan cost 2026-07-23 4:04 [PATCH v8 0/2] Cache aware scheduling: Reduce the overhead of task_cache_work Luo Gengkun 2026-07-23 4:04 ` [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus Luo Gengkun @ 2026-07-23 4:04 ` Luo Gengkun 1 sibling, 0 replies; 15+ messages in thread From: Luo Gengkun @ 2026-07-23 4:04 UTC (permalink / raw) To: peterz, mingo, juri.lelli, vincent.guittot, tim.c.chen, yu.c.chen Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel This commit re-adds get_scan_cpumasks() and introduces two sched features as well as trace events to track the scanning cost for testing purposes. Signed-off-by: Luo Gengkun <luogengkun2@huawei.com> --- include/trace/events/sched.h | 21 +++++++++++ kernel/sched/fair.c | 73 +++++++++++++++++++++++++++++++++--- kernel/sched/features.h | 2 + 3 files changed, 91 insertions(+), 5 deletions(-) diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h index 535860581f15..aced624f198d 100644 --- a/include/trace/events/sched.h +++ b/include/trace/events/sched.h @@ -10,6 +10,27 @@ #include <linux/tracepoint.h> #include <linux/binfmts.h> +TRACE_EVENT(sched_cache_scan, + + TP_PROTO(struct task_struct *t, int scan), + + TP_ARGS(t, scan), + + TP_STRUCT__entry( + __string( comm, t->comm ) + __field( pid_t, pid ) + __field( int, scan ) + ), + + TP_fast_assign( + __assign_str(comm); + __entry->pid = t->pid; + __entry->scan = scan; + ), + + TP_printk("comm=%s pid=%d scan=%d", __get_str(comm), __entry->pid, + __entry->scan) +); /* * Tracepoint for calling kthread_stop, performed to end a kthread: */ diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 10d442074f21..2d4e543ee158 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -1648,7 +1648,8 @@ static unsigned long fraction_mm_sched(int cpu, __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) { + if (sched_feat(SC_VISIT) && + (rq->cpu_epoch - pcpu_sched->epoch_last_visit) > llc_epoch_affinity_timeout) { cpumask_clear_cpu(cpu, mm->sc_stat.visited_cpus); return 0; } @@ -1723,7 +1724,8 @@ void account_mm_sched(struct rq *rq, struct task_struct *p, s64 delta_exec) rq->cpu_runtime += delta_exec; epoch = rq->cpu_epoch; pcpu_sched->epoch_last_visit = epoch; - if (!cpumask_test_cpu(cpu_of(rq), mm->sc_stat.visited_cpus)) + if (sched_feat(SC_VISIT) && + !cpumask_test_cpu(cpu_of(rq), mm->sc_stat.visited_cpus)) cpumask_set_cpu(cpu_of(rq), mm->sc_stat.visited_cpus); } @@ -1775,6 +1777,51 @@ static void task_tick_cache(struct rq *rq, struct task_struct *p) } } +static void get_scan_cpumasks(cpumask_var_t cpus, struct task_struct *p) +{ +#ifdef CONFIG_NUMA_BALANCING + int cpu, curr_cpu, nid, pref_nid; + + if (!static_branch_likely(&sched_numa_balancing)) + goto out; + + cpu = READ_ONCE(p->mm->sc_stat.cpu); + if (cpu != -1) + nid = cpu_to_node(cpu); + curr_cpu = task_cpu(p); + + /* + * Scanning in the preferred NUMA node is ideal. However, the NUMA + * preferred node is per-task rather than per-process. It is possible + * for different threads of the process to have distinct preferred + * nodes; consequently, the process-wide preferred LLC may bounce + * between different nodes. As a workaround, maintain the scan + * CPU mask to also cover the process's current preferred LLC and the + * current running node to mitigate the bouncing risk. + * TBD: numa_group should be considered during task aggregation. + */ + pref_nid = p->numa_preferred_nid; + /* honor the task's preferred node */ + if (pref_nid == NUMA_NO_NODE) + goto out; + + cpumask_or(cpus, cpus, cpumask_of_node(pref_nid)); + + /* honor the task's preferred LLC CPU */ + if (cpu != -1 && !cpumask_test_cpu(cpu, cpus) && nid != NUMA_NO_NODE) + cpumask_or(cpus, cpus, cpumask_of_node(nid)); + + /* make sure the task's current running node is included */ + if (!cpumask_test_cpu(curr_cpu, cpus)) + cpumask_or(cpus, cpus, cpumask_of_node(cpu_to_node(curr_cpu))); + + return; + +out: +#endif + cpumask_copy(cpus, cpu_online_mask); +} + static inline void update_avg_scale(u64 *avg, u64 sample) { int factor = per_cpu(sd_llc_size, raw_smp_processor_id()); @@ -1801,7 +1848,8 @@ static void task_cache_work(struct callback_head *work) unsigned long curr_m_a_occ = 0; struct mm_struct *mm = p->mm; unsigned long m_a_occ = 0; - cpumask_var_t cpus; + cpumask_var_t cpus, scan_cpus; + int scanned = 0; WARN_ON_ONCE(work != &p->cache_work); @@ -1832,6 +1880,11 @@ static void task_cache_work(struct callback_head *work) if (!zalloc_cpumask_var(&cpus, GFP_KERNEL)) return; + if (!zalloc_cpumask_var(&scan_cpus, GFP_KERNEL)) { + free_cpumask_var(cpus); + return; + } + scoped_guard (cpus_read_lock) { guard(rcu)(); @@ -1846,7 +1899,10 @@ static void task_cache_work(struct callback_head *work) * implies this process hasn't run on that CPU for a long * time, and will be captured in the next cycle. */ - cpumask_and(cpus, cpu_online_mask, mm->sc_stat.visited_cpus); + if (sched_feat(SC_NODE)) + get_scan_cpumasks(cpus, p); + else if (sched_feat(SC_VISIT)) + cpumask_and(cpus, cpu_online_mask, mm->sc_stat.visited_cpus); for_each_cpu(cpu, cpus) { /* XXX sched_cluster_active */ @@ -1857,12 +1913,17 @@ static void task_cache_work(struct callback_head *work) if (!sd) continue; - for_each_cpu_and(i, sched_domain_span(sd), mm->sc_stat.visited_cpus) { + cpumask_copy(scan_cpus, sched_domain_span(sd)); + if (sched_feat(SC_VISIT)) + cpumask_and(scan_cpus, scan_cpus, mm->sc_stat.visited_cpus); + + for_each_cpu(i, scan_cpus) { cur = rcu_dereference_all(cpu_rq(i)->curr); if (cur && !(cur->flags & (PF_EXITING | PF_KTHREAD)) && cur->mm == mm) nr_running++; + scanned++; occ = fraction_mm_sched(i, mm); if (occ == 0) continue; @@ -1917,6 +1978,8 @@ static void task_cache_work(struct callback_head *work) update_avg_scale(&mm->sc_stat.nr_running_avg, nr_running); free_cpumask_var(cpus); + free_cpumask_var(scan_cpus); + trace_sched_cache_scan(p, scanned); } void init_sched_mm(struct task_struct *p) diff --git a/kernel/sched/features.h b/kernel/sched/features.h index 8f0dee8fc475..98701c52b919 100644 --- a/kernel/sched/features.h +++ b/kernel/sched/features.h @@ -142,3 +142,5 @@ SCHED_FEAT(LATENCY_WARN, false) */ SCHED_FEAT(NI_RANDOM, true) SCHED_FEAT(NI_RATE, true) +SCHED_FEAT(SC_VISIT, false) +SCHED_FEAT(SC_NODE, false) -- 2.34.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-07-31 8:31 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-07-23 4:04 [PATCH v8 0/2] Cache aware scheduling: Reduce the overhead of task_cache_work Luo Gengkun 2026-07-23 4:04 ` [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus Luo Gengkun 2026-07-27 1:09 ` Chen, Yu C 2026-07-28 8:53 ` Luo Gengkun 2026-07-28 12:08 ` : " Chen Yu 2026-07-29 9:19 ` Luo Gengkun 2026-07-29 13:48 ` Chen, Yu C 2026-07-29 18:29 ` Tim Chen 2026-07-30 2:22 ` Tim Chen 2026-07-30 13:40 ` Luo Gengkun 2026-07-30 16:57 ` Tim Chen 2026-07-31 6:58 ` Chen, Yu C 2026-07-31 8:31 ` Luo Gengkun 2026-07-28 20:16 ` Tim Chen 2026-07-23 4:04 ` [PATCH v8 2/2] -- DO NOT APPLY!!! -- sched/cache/debug: Add trace event and sched feature to track scan cost Luo Gengkun
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®