From: "Chen, Yu C" <yu.c.chen@intel.com>
To: Luo Gengkun <luogengkun2@huawei.com>
Cc: <tim.c.chen@linux.intel.com>, <dietmar.eggemann@arm.com>,
<rostedt@goodmis.org>, <peterz@infradead.org>,
<bsegall@google.com>, <mgorman@suse.de>, <vschneid@redhat.com>,
<kprateek.nayak@amd.com>, <linux-kernel@vger.kernel.org>,
<mingo@redhat.com>, <juri.lelli@redhat.com>,
<vincent.guittot@linaro.org>, Chen Yu <chen.yu@linux.dev>
Subject: Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus
Date: Wed, 29 Jul 2026 21:48:19 +0800 [thread overview]
Message-ID: <81e3a13f-6f08-414d-aa29-86fb022aac49@intel.com> (raw)
In-Reply-To: <8b7cd508-3b89-4fc0-85fd-5a8d35ed06ca@huawei.com>
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
next prev parent reply other threads:[~2026-07-29 13:48 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=81e3a13f-6f08-414d-aa29-86fb022aac49@intel.com \
--to=yu.c.chen@intel.com \
--cc=bsegall@google.com \
--cc=chen.yu@linux.dev \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luogengkun2@huawei.com \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tim.c.chen@linux.intel.com \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®