Re: [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement

[Guopeng Zhang <guopeng.zhang@linux.dev>]

在 2026/7/27 12:03, Suren Baghdasaryan 写道:
> On Fri, Jul 17, 2026 at 2:14 AM Guopeng Zhang <guopeng.zhang@linux.dev> wrote:
>>
>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>
>> psi_trigger_destroy() clears rtpoll_task while holding the trigger
>> lock, but has to drop the lock before stopping the worker because
>> psi_rtpoll_work() takes the same lock. A new trigger can therefore
>> install a replacement worker before the old one exits.
>>
>> rtpoll_wakeup is shared by both workers. The wait condition currently
> 
> To be clear, "both workers" refer to the old worked being stopped and
> the new worked created by psi_trigger_create(). Please spell that out
> to avoid confusion.
> 
Hi Suren,

Sorry for the late reply, I got tied up with some other work :)

Thanks for the review. Yes, I'll make that explicit in v2.

>> consumes it before checking whether the worker should stop. If the
>> replacement timer sets the wakeup while the old worker is being stopped,
>> the old worker can consume it and then exit. Since the one-shot timer has
>> already fired and rtpoll_scheduled remains set, the replacement can stay
>> asleep and rtpolling can stall.
>>
>> Make the wait condition only observe the wakeup. Check for stop before
>> consuming it and, once consumed, always process the work.
>>
>> Fixes: 461daba06bdc ("psi: eliminate kthread_worker from psi trigger scheduling mechanism")
>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>> ---
>>  kernel/sched/psi.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
>> index 4e152410653d..b9e2a93a757b 100644
>> --- a/kernel/sched/psi.c
>> +++ b/kernel/sched/psi.c
>> @@ -743,10 +743,16 @@ static int psi_rtpoll_worker(void *data)
>>
>>         while (true) {
>>                 wait_event_interruptible(group->rtpoll_wait,
>> -                               atomic_cmpxchg(&group->rtpoll_wakeup, 1, 0) ||
>> +                               atomic_read(&group->rtpoll_wakeup) ||
>>                                 kthread_should_stop());
>>                 if (kthread_should_stop())
>>                         break;
>> +               /*
>> +                * Consume the wakeup only after checking for stop. Once consumed,
>> +                * always run the work so a replacement worker cannot lose it.
>> +                */
>> +               if (atomic_cmpxchg(&group->rtpoll_wakeup, 1, 0) != 1)
>> +                       continue;
> 
> This works but a simpler fix is to reorder conditions in
> wait_event_interruptible() call like this:
> 
>          while (true) {
>                  wait_event_interruptible(group->rtpoll_wait,
>                                 kthread_should_stop() ||
>                                 atomic_cmpxchg(&group->rtpoll_wakeup, 1, 0));
>                  if (kthread_should_stop())
>                          break;
> 
>                  psi_rtpoll_work(group);
>          }
> 
> The old worker would check kthread_should_stop() and stop before it
> can consume rtpoll_wakeup. Do you agree?
> 

Could the following race still happen?

old worker                         teardown
-------------------------------------------------------
kthread_should_stop() == false
cmpxchg(wakeup, 1, 0) succeeds
                                   kthread_stop() sets SHOULD_STOP
kthread_should_stop() == true
exits without running the work

In this case, it seems the old worker could still consume the wakeup and
then observe the stop request before calling psi_rtpoll_work().

Therefore, reordering the conditions appears to narrow the race window,
but does not close it completely.

Thanks,
Guopeng

>>
>>                 psi_rtpoll_work(group);
>>         }
>> --
>> 2.43.0