* [PATCH 0/3] sched/psi: Fix rtpoll teardown races
@ 2026-07-17 9:14 Guopeng Zhang
2026-07-17 9:14 ` [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement Guopeng Zhang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Guopeng Zhang @ 2026-07-17 9:14 UTC (permalink / raw)
To: Johannes Weiner, Suren Baghdasaryan
Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, Tejun Heo, Zhaoyang Huang,
ziwei.dai, Chengming Zhou, linux-kernel
From: Guopeng Zhang <zhangguopeng@kylinos.cn>
Last-trigger rtpoll teardown clears rtpoll_task under
rtpoll_trigger_lock, but has to stop the old worker after dropping the
lock because the worker takes the same lock. A new trigger can therefore
install a replacement worker before the old worker exits. Lockless
psi_schedule_rtpoll_work() readers can also overlap with teardown after
having observed the old worker.
This leaves three separate races:
- the old worker can consume a timer wakeup intended for its replacement
and then exit, leaving the replacement asleep while
rtpoll_scheduled remains set;
- a lockless reader that observed the old rtpoll_task can call
mod_timer() after teardown called timer_delete(), leaving a stale timer
pending after the last trigger has been removed;
- after a replacement has published its worker and armed the timer, the
old teardown can clear rtpoll_scheduled, allowing a later task change
to rearm an already pending timer unnecessarily.
The patches are ordered as follows:
1. Check for worker stop before consuming the shared wakeup, and process
every wakeup that is consumed.
2. Wait for pre-existing RCU readers before deleting the timer
synchronously under rtpoll_trigger_lock.
3. Building on patch 2, clear rtpoll_scheduled during the locked timer
teardown, before a replacement worker can be published.
Testing
=======
Temporary A/B instrumentation directly exercised the timer rearm race
addressed by patch 2. One lockless RCU reader per group was held for up to
10 ms after it read a non-NULL rtpoll_task. On the unfixed kernel,
teardown unpublished the worker, deleted the timer, and released the
reader. The reader then observed the NULL pointer and rearmed the timer
with a 10-second expiry. On the fixed kernel, synchronize_rcu() waited for
the reader to rearm the timer and leave its read-side critical section
before timer_delete_sync() removed it.
After the grace period and before stopping the old worker, the
instrumentation checked under rtpoll_trigger_lock whether the timer was
pending while no replacement worker was published. It logged and
synchronously deleted any timer found this way.
Both runs used the same test VM, kernel configuration, script, workload,
and instrumentation.
Without patch 2:
CPU workers: 128
Duration: 60 seconds
Trigger cycles: 2029
CPU PSI some total delta: 60561126 usec
Pending timer after teardown: observed
With patch 2:
CPU workers: 128
Duration: 60 seconds
Trigger cycles: 2045
CPU PSI some total delta: 60423170 usec
Pending timer after teardown: not observed in 2045 trigger cycles
The temporary instrumentation is not part of this series.
Guopeng Zhang (3):
sched/psi: Avoid losing wakeups during rtpoll worker replacement
sched/psi: Prevent stale timer rearm after rtpoll teardown
sched/psi: Avoid clobbering rtpoll_scheduled during teardown
kernel/sched/psi.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
base-commit: 1a1757b76427f6201bfe0bf1bea9f7574f332a93
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement
2026-07-17 9:14 [PATCH 0/3] sched/psi: Fix rtpoll teardown races Guopeng Zhang
@ 2026-07-17 9:14 ` Guopeng Zhang
2026-07-27 4:03 ` Suren Baghdasaryan
2026-07-17 9:14 ` [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown Guopeng Zhang
2026-07-17 9:14 ` [PATCH 3/3] sched/psi: Avoid clobbering rtpoll_scheduled during teardown Guopeng Zhang
2 siblings, 1 reply; 9+ messages in thread
From: Guopeng Zhang @ 2026-07-17 9:14 UTC (permalink / raw)
To: Johannes Weiner, Suren Baghdasaryan
Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, Tejun Heo, Zhaoyang Huang,
ziwei.dai, Chengming Zhou, linux-kernel
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
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;
psi_rtpoll_work(group);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown
2026-07-17 9:14 [PATCH 0/3] sched/psi: Fix rtpoll teardown races Guopeng Zhang
2026-07-17 9:14 ` [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement Guopeng Zhang
@ 2026-07-17 9:14 ` Guopeng Zhang
2026-07-27 4:49 ` Suren Baghdasaryan
2026-07-17 9:14 ` [PATCH 3/3] sched/psi: Avoid clobbering rtpoll_scheduled during teardown Guopeng Zhang
2 siblings, 1 reply; 9+ messages in thread
From: Guopeng Zhang @ 2026-07-17 9:14 UTC (permalink / raw)
To: Johannes Weiner, Suren Baghdasaryan
Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, Tejun Heo, Zhaoyang Huang,
ziwei.dai, Chengming Zhou, linux-kernel
From: Guopeng Zhang <zhangguopeng@kylinos.cn>
psi_schedule_rtpoll_work() reads rtpoll_task under RCU before calling
mod_timer(). Last-trigger teardown clears the pointer and deletes the
timer before waiting for existing readers. A reader that saw the old task
can therefore rearm the timer after timer_delete(), leaving a stale timer
pending after trigger teardown.
psi_cgroup_free() shuts down rtpoll_timer before freeing the group, so the
pending timer cannot outlive the psi_group. It can still fire after the
last trigger has been removed and wake the waitqueue when no worker is
published, and trigger teardown does not leave the timer quiesced.
After publishing NULL, wait for existing readers while holding
rtpoll_trigger_lock, then use timer_delete_sync() to drain the callback.
Holding the lock also prevents a new trigger from reusing the timer until
teardown has finished with it.
Fixes: 8f91efd870ea ("psi: Fix race between psi_trigger_create/destroy")
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
kernel/sched/psi.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index b9e2a93a757b..db9c56fa8923 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -1488,18 +1488,22 @@ void psi_trigger_destroy(struct psi_trigger *t)
group->rtpoll_task,
lockdep_is_held(&group->rtpoll_trigger_lock));
rcu_assign_pointer(group->rtpoll_task, NULL);
- timer_delete(&group->rtpoll_timer);
+ /*
+ * Wait for psi_schedule_rtpoll_work() to either
+ * observe the NULL task or finish rearming the timer.
+ * Keeping the mutex held also prevents a new trigger
+ * from installing a task before the old timer is gone.
+ */
+ synchronize_rcu();
+ timer_delete_sync(&group->rtpoll_timer);
}
}
mutex_unlock(&group->rtpoll_trigger_lock);
}
- /*
- * Wait for psi_schedule_rtpoll_work RCU to complete its read-side
- * critical section before destroying the trigger and optionally the
- * rtpoll_task.
- */
- synchronize_rcu();
+ /* The last-trigger path has already waited for RCU readers above. */
+ if (!task_to_destroy)
+ synchronize_rcu();
/*
* Stop kthread 'psimon' after releasing rtpoll_trigger_lock to prevent
* a deadlock while waiting for psi_rtpoll_work to acquire
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] sched/psi: Avoid clobbering rtpoll_scheduled during teardown
2026-07-17 9:14 [PATCH 0/3] sched/psi: Fix rtpoll teardown races Guopeng Zhang
2026-07-17 9:14 ` [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement Guopeng Zhang
2026-07-17 9:14 ` [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown Guopeng Zhang
@ 2026-07-17 9:14 ` Guopeng Zhang
2026-07-27 5:53 ` Suren Baghdasaryan
2 siblings, 1 reply; 9+ messages in thread
From: Guopeng Zhang @ 2026-07-17 9:14 UTC (permalink / raw)
To: Johannes Weiner, Suren Baghdasaryan
Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, Tejun Heo, Zhaoyang Huang,
ziwei.dai, Chengming Zhou, linux-kernel
From: Guopeng Zhang <zhangguopeng@kylinos.cn>
Last-trigger teardown has to drop rtpoll_trigger_lock before calling
kthread_stop() because the worker takes the same lock. A new trigger
can therefore install a replacement worker and schedule its timer
before the old worker has stopped.
Teardown currently clears rtpoll_scheduled only after kthread_stop()
returns. That late clear can overwrite state installed for the
replacement, leaving rtpoll_scheduled clear while its timer is pending.
The next task change can then rearm the timer unnecessarily.
Reset the flag after draining the old timer while still holding
rtpoll_trigger_lock, before a replacement can be published, and leave it
untouched after unlocking.
Fixes: 710ffe671e01 ("sched/psi: Stop relying on timer_pending() for poll_work rescheduling")
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
kernel/sched/psi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index db9c56fa8923..bec329fafb4d 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -1496,6 +1496,7 @@ void psi_trigger_destroy(struct psi_trigger *t)
*/
synchronize_rcu();
timer_delete_sync(&group->rtpoll_timer);
+ atomic_set(&group->rtpoll_scheduled, 0);
}
}
mutex_unlock(&group->rtpoll_trigger_lock);
@@ -1515,7 +1516,6 @@ void psi_trigger_destroy(struct psi_trigger *t)
* can no longer be found through group->rtpoll_task.
*/
kthread_stop(task_to_destroy);
- atomic_set(&group->rtpoll_scheduled, 0);
}
kfree(t);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement
2026-07-17 9:14 ` [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement Guopeng Zhang
@ 2026-07-27 4:03 ` Suren Baghdasaryan
2026-08-07 9:51 ` Guopeng Zhang
0 siblings, 1 reply; 9+ messages in thread
From: Suren Baghdasaryan @ 2026-07-27 4:03 UTC (permalink / raw)
To: Guopeng Zhang
Cc: Johannes Weiner, Peter Zijlstra, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, K Prateek Nayak, Tejun Heo,
Zhaoyang Huang, ziwei.dai, Chengming Zhou, linux-kernel
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.
> 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?
>
> psi_rtpoll_work(group);
> }
> --
> 2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown
2026-07-17 9:14 ` [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown Guopeng Zhang
@ 2026-07-27 4:49 ` Suren Baghdasaryan
2026-08-07 9:52 ` Guopeng Zhang
0 siblings, 1 reply; 9+ messages in thread
From: Suren Baghdasaryan @ 2026-07-27 4:49 UTC (permalink / raw)
To: Guopeng Zhang
Cc: Johannes Weiner, Peter Zijlstra, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, K Prateek Nayak, Tejun Heo,
Zhaoyang Huang, ziwei.dai, Chengming Zhou, linux-kernel
On Fri, Jul 17, 2026 at 2:14 AM Guopeng Zhang <guopeng.zhang@linux.dev> wrote:
>
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> psi_schedule_rtpoll_work() reads rtpoll_task under RCU before calling
> mod_timer(). Last-trigger teardown clears the pointer and deletes the
> timer before waiting for existing readers. A reader that saw the old task
> can therefore rearm the timer after timer_delete(), leaving a stale timer
> pending after trigger teardown.
>
> psi_cgroup_free() shuts down rtpoll_timer before freeing the group, so the
> pending timer cannot outlive the psi_group. It can still fire after the
> last trigger has been removed and wake the waitqueue when no worker is
> published, and trigger teardown does not leave the timer quiesced.
quiesced? Don't you just love these AI generated changelogs?
>
> After publishing NULL, wait for existing readers while holding
> rtpoll_trigger_lock, then use timer_delete_sync() to drain the callback.
> Holding the lock also prevents a new trigger from reusing the timer until
> teardown has finished with it.
I've seen a report of this problem generated by AI and I think it's
legitimate; however, so far I could not reproduce it even after
injecting delays to increase the possibility of this race. Have you
been able to reproduce it? If so, could you please share the
reproducer?
>
> Fixes: 8f91efd870ea ("psi: Fix race between psi_trigger_create/destroy")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
> kernel/sched/psi.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
> index b9e2a93a757b..db9c56fa8923 100644
> --- a/kernel/sched/psi.c
> +++ b/kernel/sched/psi.c
> @@ -1488,18 +1488,22 @@ void psi_trigger_destroy(struct psi_trigger *t)
> group->rtpoll_task,
> lockdep_is_held(&group->rtpoll_trigger_lock));
> rcu_assign_pointer(group->rtpoll_task, NULL);
> - timer_delete(&group->rtpoll_timer);
> + /*
> + * Wait for psi_schedule_rtpoll_work() to either
> + * observe the NULL task or finish rearming the timer.
> + * Keeping the mutex held also prevents a new trigger
> + * from installing a task before the old timer is gone.
> + */
> + synchronize_rcu();
> + timer_delete_sync(&group->rtpoll_timer);
Ok, poll_timer_fn() does not take rtpoll_trigger_lock, so I think this
is safe. I would like to double-check the code and run some tests
before approving this fix.
> }
> }
> mutex_unlock(&group->rtpoll_trigger_lock);
> }
>
> - /*
> - * Wait for psi_schedule_rtpoll_work RCU to complete its read-side
> - * critical section before destroying the trigger and optionally the
> - * rtpoll_task.
> - */
> - synchronize_rcu();
> + /* The last-trigger path has already waited for RCU readers above. */
> + if (!task_to_destroy)
> + synchronize_rcu();
> /*
> * Stop kthread 'psimon' after releasing rtpoll_trigger_lock to prevent
> * a deadlock while waiting for psi_rtpoll_work to acquire
> --
> 2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] sched/psi: Avoid clobbering rtpoll_scheduled during teardown
2026-07-17 9:14 ` [PATCH 3/3] sched/psi: Avoid clobbering rtpoll_scheduled during teardown Guopeng Zhang
@ 2026-07-27 5:53 ` Suren Baghdasaryan
0 siblings, 0 replies; 9+ messages in thread
From: Suren Baghdasaryan @ 2026-07-27 5:53 UTC (permalink / raw)
To: Guopeng Zhang
Cc: Johannes Weiner, Peter Zijlstra, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, K Prateek Nayak, Tejun Heo,
Zhaoyang Huang, ziwei.dai, Chengming Zhou, linux-kernel
On Fri, Jul 17, 2026 at 2:15 AM Guopeng Zhang <guopeng.zhang@linux.dev> wrote:
>
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> Last-trigger teardown has to drop rtpoll_trigger_lock before calling
> kthread_stop() because the worker takes the same lock. A new trigger
> can therefore install a replacement worker and schedule its timer
> before the old worker has stopped.
>
> Teardown currently clears rtpoll_scheduled only after kthread_stop()
> returns. That late clear can overwrite state installed for the
> replacement, leaving rtpoll_scheduled clear while its timer is pending.
> The next task change can then rearm the timer unnecessarily.
>
> Reset the flag after draining the old timer while still holding
> rtpoll_trigger_lock, before a replacement can be published, and leave it
> untouched after unlocking.
Makes sense.
>
> Fixes: 710ffe671e01 ("sched/psi: Stop relying on timer_pending() for poll_work rescheduling")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Acked-by: Suren Baghdasaryan <surenb@google.com>
> ---
> kernel/sched/psi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
> index db9c56fa8923..bec329fafb4d 100644
> --- a/kernel/sched/psi.c
> +++ b/kernel/sched/psi.c
> @@ -1496,6 +1496,7 @@ void psi_trigger_destroy(struct psi_trigger *t)
> */
> synchronize_rcu();
> timer_delete_sync(&group->rtpoll_timer);
> + atomic_set(&group->rtpoll_scheduled, 0);
> }
> }
> mutex_unlock(&group->rtpoll_trigger_lock);
> @@ -1515,7 +1516,6 @@ void psi_trigger_destroy(struct psi_trigger *t)
> * can no longer be found through group->rtpoll_task.
> */
> kthread_stop(task_to_destroy);
> - atomic_set(&group->rtpoll_scheduled, 0);
> }
> kfree(t);
> }
> --
> 2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement
2026-07-27 4:03 ` Suren Baghdasaryan
@ 2026-08-07 9:51 ` Guopeng Zhang
0 siblings, 0 replies; 9+ messages in thread
From: Guopeng Zhang @ 2026-08-07 9:51 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Johannes Weiner, Peter Zijlstra, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, K Prateek Nayak, Tejun Heo,
Zhaoyang Huang, ziwei.dai, Chengming Zhou, linux-kernel
在 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown
2026-07-27 4:49 ` Suren Baghdasaryan
@ 2026-08-07 9:52 ` Guopeng Zhang
0 siblings, 0 replies; 9+ messages in thread
From: Guopeng Zhang @ 2026-08-07 9:52 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Johannes Weiner, Peter Zijlstra, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, K Prateek Nayak, Tejun Heo,
Zhaoyang Huang, ziwei.dai, Chengming Zhou, linux-kernel
在 2026/7/27 12:49, 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_schedule_rtpoll_work() reads rtpoll_task under RCU before calling
>> mod_timer(). Last-trigger teardown clears the pointer and deletes the
>> timer before waiting for existing readers. A reader that saw the old task
>> can therefore rearm the timer after timer_delete(), leaving a stale timer
>> pending after trigger teardown.
>>
>> psi_cgroup_free() shuts down rtpoll_timer before freeing the group, so the
>> pending timer cannot outlive the psi_group. It can still fire after the
>> last trigger has been removed and wake the waitqueue when no worker is
>> published, and trigger teardown does not leave the timer quiesced.
>
> quiesced? Don't you just love these AI generated changelogs?
>
Yes, I do rely on LLMs a bit for help with my English :) The downside is that
they occasionally sneak words like "quiesced" into the changelog when I'm not
looking. I'll proofread them more carefully next time.
>>
>> After publishing NULL, wait for existing readers while holding
>> rtpoll_trigger_lock, then use timer_delete_sync() to drain the callback.
>> Holding the lock also prevents a new trigger from reusing the timer until
>> teardown has finished with it.
>
> I've seen a report of this problem generated by AI and I think it's
> legitimate; however, so far I could not reproduce it even after
> injecting delays to increase the possibility of this race. Have you
> been able to reproduce it? If so, could you please share the
> reproducer?
>
Yes, I was able to reproduce it.
I used the following debug instrumentation to widen the race window and
count stale timer rearms:
diff --git a/include/linux/psi_types.h b/include/linux/psi_types.h
index dd10c22299ab..02760a014136 100644
--- a/include/linux/psi_types.h
+++ b/include/linux/psi_types.h
@@ -203,6 +203,11 @@ struct psi_group {
u64 rtpoll_total[NR_PSI_STATES - 1];
u64 rtpoll_next_update;
u64 rtpoll_until;
+
+ atomic_t rtpoll_dbg_stale_rearm;
+ atomic_t rtpoll_dbg_hotpath_arm;
+ struct task_struct *rtpoll_dbg_deleted_task;
+ bool rtpoll_dbg_timer_deleted;
};
#else /* CONFIG_PSI */
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index b9e2a93a757b..bf7d2eaf2196 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -140,6 +140,10 @@
#include <linux/workqueue.h>
#include <linux/psi.h>
#include "sched.h"
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+
+#define PSI_DBG_RACE_UDELAY_US 5000
static int psi_bug __read_mostly;
@@ -223,6 +227,11 @@ static void group_init(struct psi_group *group)
init_waitqueue_head(&group->rtpoll_wait);
timer_setup(&group->rtpoll_timer, poll_timer_fn, 0);
rcu_assign_pointer(group->rtpoll_task, NULL);
+
+ atomic_set(&group->rtpoll_dbg_stale_rearm, 0);
+ atomic_set(&group->rtpoll_dbg_hotpath_arm, 0);
+ group->rtpoll_dbg_deleted_task = NULL;
+ group->rtpoll_dbg_timer_deleted = false;
}
void __init psi_init(void)
@@ -634,18 +643,28 @@ static void psi_schedule_rtpoll_work(struct psi_group *group, unsigned long dela
*/
if (atomic_xchg(&group->rtpoll_scheduled, 1) && !force)
return;
+ if (!force)
+ atomic_inc(&group->rtpoll_dbg_hotpath_arm);
rcu_read_lock();
task = rcu_dereference(group->rtpoll_task);
+ if (task && PSI_DBG_RACE_UDELAY_US)
+ udelay(PSI_DBG_RACE_UDELAY_US);
+ bool stale_rearm = task &&
+ smp_load_acquire(&group->rtpoll_dbg_timer_deleted) &&
+ task == READ_ONCE(group->rtpoll_dbg_deleted_task);
/*
* kworker might be NULL in case psi_trigger_destroy races with
* psi_task_change (hotpath) which can't use locks
*/
- if (likely(task))
+ if (likely(task)) {
mod_timer(&group->rtpoll_timer, jiffies + delay);
- else
+ if (stale_rearm)
+ atomic_inc(&group->rtpoll_dbg_stale_rearm);
+ } else {
atomic_set(&group->rtpoll_scheduled, 0);
+ }
rcu_read_unlock();
}
@@ -1323,6 +1342,8 @@ int psi_trigger_create_rtpoll_worker(struct psi_group *group)
if (!rcu_access_pointer(group->rtpoll_task)) {
atomic_set(&group->rtpoll_wakeup, 0);
wake_up_process(task);
+ WRITE_ONCE(group->rtpoll_dbg_deleted_task, NULL);
+ smp_store_release(&group->rtpoll_dbg_timer_deleted, false);
rcu_assign_pointer(group->rtpoll_task, task);
/*
@@ -1489,6 +1510,8 @@ void psi_trigger_destroy(struct psi_trigger *t)
lockdep_is_held(&group->rtpoll_trigger_lock));
rcu_assign_pointer(group->rtpoll_task, NULL);
timer_delete(&group->rtpoll_timer);
+ WRITE_ONCE(group->rtpoll_dbg_deleted_task, task_to_destroy);
+ smp_store_release(&group->rtpoll_dbg_timer_deleted, true);
}
}
mutex_unlock(&group->rtpoll_trigger_lock);
@@ -1717,6 +1740,10 @@ static int __init psi_proc_init(void)
{
if (psi_enable) {
proc_mkdir("pressure", NULL);
+ debugfs_create_atomic_t("rtpoll_stale_rearm", 0444, NULL,
+ &psi_system.rtpoll_dbg_stale_rearm);
+ debugfs_create_atomic_t("rtpoll_dbg_hotpath_arm", 0444, NULL,
+ &psi_system.rtpoll_dbg_hotpath_arm);
proc_create("pressure/io", 0666, NULL, &psi_io_proc_ops);
proc_create("pressure/memory", 0666, NULL, &psi_memory_proc_ops);
proc_create("pressure/cpu", 0666, NULL, &psi_cpu_proc_ops);
I used the following test script, also with some LLM help:
#!/bin/bash
# Stress driver for the PSI rtpoll stale-timer-rearm race (coordinated mode).
# Single loop: open RT trigger -> IO burst (polling) -> idle (psi_rtpoll_work
# winds down, scheduled=0) -> resume IO (reader fresh-arms, parks in udelay) ->
# close trigger (teardown lands inside the udelay -> stale rearm).
# Env: DURATION BURST IDLE RESUME_DELAY TMPFILE (root, instrumented kernel)
set -u
set +m
DURATION=${DURATION:-60}
BURST=${BURST:-64} # dd reads per active burst
IDLE=${IDLE:-0.15} # idle gap (s); must exceed the polling window (~100ms)
RESUME_DELAY=${RESUME_DELAY:-0.003} # delay before close; lands it inside the udelay window
THRESHOLD_US=${THRESHOLD_US:-1000}
WINDOW_US=${WINDOW_US:-100000}
TMPFILE=${TMPFILE:-/var/tmp/psi_race_buf}
REARM=/sys/kernel/debug/rtpoll_stale_rearm
HOTARM=/sys/kernel/debug/rtpoll_dbg_hotpath_arm
PIDS=/tmp/psi_race.pids
log() { printf '%s\n' "$*"; }
[ "$(id -u)" -eq 0 ] || { log "ERROR: must be root"; exit 1; }
mountpoint -q /sys/kernel/debug 2>/dev/null || mount -t debugfs none /sys/kernel/debug 2>/dev/null
[ -f "$REARM" ] || { log "ERROR: $REARM missing (boot the instrumented kernel)"; exit 1; }
if ! exec 9<>/proc/pressure/io; then log "ERROR: cannot open /proc/pressure/io"; exit 1; fi
if ! printf 'some %s %s\n' "$THRESHOLD_US" "$WINDOW_US" >&9; then
log "ERROR: failed to create PSI trigger"; exec 9>&-; exit 1
fi
exec 9>&-
cleanup() {
[ -f "$PIDS" ] && kill -9 $(cat "$PIDS" 2>/dev/null) 2>/dev/null
pkill -9 -f "if=$TMPFILE" 2>/dev/null
wait 2>/dev/null
rm -f "$PIDS"
}
trap cleanup EXIT
if [ -b "$TMPFILE" ]; then
:
elif [ ! -e "$TMPFILE" ]; then
log "preparing $TMPFILE (512M) ..."
dd if=/dev/zero of="$TMPFILE" bs=1M count=512 oflag=direct 2>/dev/null \
|| dd if=/dev/zero of="$TMPFILE" bs=1M count=512 2>/dev/null
sync
fi
before_r=$(cat "$REARM" 2>/dev/null || echo 0)
before_h=$(cat "$HOTARM" 2>/dev/null || echo 0)
log "=== PSI rtpoll stale-rearm stress (coordinated) ==="
log "duration=${DURATION}s burst=${BURST} idle=${IDLE}s resume_delay=${RESUME_DELAY}s"
log "stale_rearm before: $before_r hotpath_arm before: $before_h"
: > "$PIDS"
end=$((SECONDS + DURATION))
while [ "$SECONDS" -lt "$end" ]; do
exec 3<>/proc/pressure/io 2>/dev/null || break
echo "some $THRESHOLD_US $WINDOW_US" >&3 2>/dev/null || { exec 3>&-; break; }
dd if="$TMPFILE" of=/dev/null bs=64k iflag=direct count="$BURST" 2>/dev/null \
|| dd if="$TMPFILE" of=/dev/null bs=64k count="$BURST" 2>/dev/null
sleep "$IDLE"
taskset -c 1 dd if="$TMPFILE" of=/dev/null bs=64k iflag=direct count=4 2>/dev/null &
echo $! >> "$PIDS"
sleep "$RESUME_DELAY"
exec 3>&-
wait 2>/dev/null
done
cleanup
after_r=$(cat "$REARM" 2>/dev/null || echo 0)
after_h=$(cat "$HOTARM" 2>/dev/null || echo 0)
dr=$((after_r - before_r))
dh=$((after_h - before_h))
log "stale_rearm after: $after_r (delta $dr)"
log "hotpath_arm after: $after_h (delta $dh)"
log ">>> stale rearm this run: $dr (hotpath fresh-arms: $dh) <<<"
if [ "$dr" -gt 0 ]; then log "VERDICT: stale rearm OBSERVED."
else log "VERDICT: stale rearm not observed."
fi
if [ "$dh" -eq 0 ]; then
log "NOTE: hotpath_arm=0 -- reader never fresh-armed (scheduled never 0)."
log " Increase IDLE or confirm PSI polling is active."
fi
Run it as root on the instrumented kernel:
sudo taskset -c 0 bash rtpoll_race_stress.sh
This pins the trigger close path to CPU 0, while the resume dd is pinned
to CPU 1 by the script, so the two paths can overlap across CPUs.
Here are the results from 5-minute runs (DURATION=300), with the same
load and a fresh boot before each run:
without patch 2: rtpoll_stale_rearm = 17 (fresh-arms: 3570)
with patch 2: rtpoll_stale_rearm = 0 (fresh-arms: 3497)
To test with patch 2 applied: move the two lines in psi_trigger_destroy()
(WRITE_ONCE(rtpoll_dbg_deleted_task, ...) + smp_store_release(..., true))
to after timer_delete_sync() — the rest of the instrumentation is identical.
Thanks,
Guopeng
>>
>> Fixes: 8f91efd870ea ("psi: Fix race between psi_trigger_create/destroy")
>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>> ---
>> kernel/sched/psi.c | 18 +++++++++++-------
>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
>> index b9e2a93a757b..db9c56fa8923 100644
>> --- a/kernel/sched/psi.c
>> +++ b/kernel/sched/psi.c
>> @@ -1488,18 +1488,22 @@ void psi_trigger_destroy(struct psi_trigger *t)
>> group->rtpoll_task,
>> lockdep_is_held(&group->rtpoll_trigger_lock));
>> rcu_assign_pointer(group->rtpoll_task, NULL);
>> - timer_delete(&group->rtpoll_timer);
>> + /*
>> + * Wait for psi_schedule_rtpoll_work() to either
>> + * observe the NULL task or finish rearming the timer.
>> + * Keeping the mutex held also prevents a new trigger
>> + * from installing a task before the old timer is gone.
>> + */
>> + synchronize_rcu();
>> + timer_delete_sync(&group->rtpoll_timer);
>
> Ok, poll_timer_fn() does not take rtpoll_trigger_lock, so I think this
> is safe. I would like to double-check the code and run some tests
> before approving this fix.
>
>> }
>> }
>> mutex_unlock(&group->rtpoll_trigger_lock);
>> }
>>
>> - /*
>> - * Wait for psi_schedule_rtpoll_work RCU to complete its read-side
>> - * critical section before destroying the trigger and optionally the
>> - * rtpoll_task.
>> - */
>> - synchronize_rcu();
>> + /* The last-trigger path has already waited for RCU readers above. */
>> + if (!task_to_destroy)
>> + synchronize_rcu();
>> /*
>> * Stop kthread 'psimon' after releasing rtpoll_trigger_lock to prevent
>> * a deadlock while waiting for psi_rtpoll_work to acquire
>> --
>> 2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-07 9:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 9:14 [PATCH 0/3] sched/psi: Fix rtpoll teardown races Guopeng Zhang
2026-07-17 9:14 ` [PATCH 1/3] sched/psi: Avoid losing wakeups during rtpoll worker replacement Guopeng Zhang
2026-07-27 4:03 ` Suren Baghdasaryan
2026-08-07 9:51 ` Guopeng Zhang
2026-07-17 9:14 ` [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown Guopeng Zhang
2026-07-27 4:49 ` Suren Baghdasaryan
2026-08-07 9:52 ` Guopeng Zhang
2026-07-17 9:14 ` [PATCH 3/3] sched/psi: Avoid clobbering rtpoll_scheduled during teardown Guopeng Zhang
2026-07-27 5:53 ` Suren Baghdasaryan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox