Re: [PATCH v9 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus
"Chen, Yu C" <[email protected]>
| Newsgroups | org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 8/12/2026 5:03 PM, Luo Gengkun wrote:
>
>
> On 2026/8/11 15:46, Chen, Yu C wrote:
>> Hi Gengkun,
>> On 8/11/2026 10:27 AM, Luo Gengkun wrote:
>>
>> I did a further study on this and have two minor questions:
>>
>>>> @@ -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);
>>
>> visited_cpus bits are only cleared inside fraction_mm_sched(), which is
>> reachable in task_cache_work() - but that loop is skipped when
>> invalid_llc_nr()
>> returns true for any single-threaded process. As a result, single-
>> threaded
>> processes keep setting bits in account_mm_sched() without using them.
>> Maybe a gate would be useful:
> From a technical perspective, adding a gate here is unnecessary.
>
> The overhead is virtually nonexistent, especially since it resides on a
> path
> already burdened by heavier operations like __update_mm_sched(). If we were
> to care about performance and optimization, focusing on __update_mm_sched()
> would be far more meaningful than adding checks here.
>
> What do you think?
Makes sense. I was previously worried about the multiple accesses to the
shared mm->variable, but since it's only a single thread with no race, it
should be fine.
>>
>> if (get_nr_threads(p) > 1 &&
>> !cpumask_test_cpu(cpu_of(rq), mm->sc_stat.visited_cpus))
>> cpumask_set_cpu(cpu_of(rq), mm->sc_stat.visited_cpus);
>>
>>
>> [ ... ]
>>
>>>> @@ -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);
>>
>> I'm thinking of if this could bring cross-node bouncing. Is it doable
>> to honor the result from NUMA preference:
>> get_scan_cpumasks(cpus, p);
>> cpumask_and(cpus, cpus, mm->sc_stat.visited_cpus);
>
> I looked closely at get_scan_cpumasks(). The CPU mask it returns is the
> union of node(p->numa_preferred_nid), node(mm->sc_stat.cpu), and
> node(task_cpu(p)).
> Its purpose was only to mitigate sc_stat.cpu bouncing — it did not fully
> eliminate it. Relying on visited_cpus alone follows the actual footprint
> of where the threads really ran, making it even less prone to bouncing.
>
> Additionally, even if the numa node derived from visited_cpus disagrees
> with a
> given thread's numa_preferred_nid, get_pref_llc() still prevents that
> thread from being migrated to that node, so it remains safe either way.
>
> Please let me know if I'm missing something.
>
I'm not against removing the node-aware-scan-first strategy, as long
as it doesn't cause any bouncing regression. I also heard from Jianyong
who is working on extending the CAS from a single-preferred LLC to
multiple-preferred LLCs, that removing the NUMA-based scan would benefit
that work. So, your change on this is fine with me.
thanks,
Chenyu