From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-173.mta0.migadu.com (out-173.mta0.migadu.com [91.218.175.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 58BB331AABF for ; Fri, 7 Aug 2026 09:52:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.173 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786096367; cv=none; b=Rsyq+xM9KPjcmDDZkckDCAJfSfAEIdNyci1oFe93oirlCisijwbF/ItyVcsnxhxsiEv6SrETvwzwsUkcyBISagt52OFgpJsluLFPC8FUM3fcpKm5sslhU7wZe3R+s3JMa9PkL7tXKd8nG7nAJFe+pO5T8l4PgUrBdEWzWNnE6eg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786096367; c=relaxed/simple; bh=1n/caTSynCIsMwF5fq/nn9CY/QNhZOXaqa64eI+Hzzc=; h=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References: In-Reply-To:Content-Type; b=s6rIpR9R2li7g4TBuuidD/KB+zEJmYjyfztv4JJLy1+EP/pFqzfzzfKKzsPw0RIq8WeQSMeWQDU25h7NOCDiUVhJIPG26FDoQMldEch0fUZSY6eGi7qB3Zs2YV1tskcjCBnv6EqbZPva35uW07eWbN3/b7YjEvxHEhrg9OR/dUI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=vkuc52bq; arc=none smtp.client-ip=91.218.175.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="vkuc52bq" Message-ID: <73b7d519-e815-4439-8470-d3f67b4c8177@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1786096362; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=gpbvR9l/sd97iYzPcrlBsiGL1DB5i8cZ2u/7k8u84i0=; b=vkuc52bqdP/xMXImedOgd8KM8mkP4bzzyHJrnzD3khLNBAcL2Mrb+8qLnepDEQFHqIdDrA Usym3w0+NUhjJz6yx6iARuopfOooza7tZtUH0cfEVD0bszmhdkereNXAinRA8qj1VI++Wt BP/jrmqKFUDKxfxOb8YIzD0WgOped8g= Date: Fri, 7 Aug 2026 17:52:26 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Guopeng Zhang Subject: Re: [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown 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@vger.kernel.org References: <70eeb3ebd144b74aaa406d732589a05f6299cf05.1784277342.git.zhangguopeng@kylinos.cn> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT 在 2026/7/27 12:49, Suren Baghdasaryan 写道: > On Fri, Jul 17, 2026 at 2:14 AM Guopeng Zhang wrote: >> >> From: Guopeng Zhang >> >> 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 #include #include "sched.h" +#include +#include + +#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 >> --- >> 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