Re: : [PATCH v8 1/2] sched/cache: Reduce the overhead of task_cache_work by only scan the visisted cpus

[Chen, Yu C]

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