Re: [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown

在 2026/7/27 12:49, Suren Baghdasaryan 写道:
> On Fri, Jul 17, 2026 at 2:14 AM Guopeng Zhang <[email protected]> wrote:
>>
>> From: Guopeng Zhang <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>>  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