[PATCH 0/3] sched/psi: Fix rtpoll teardown races

[PATCH 0/3] sched/psi: Fix rtpoll teardown races

[Guopeng Zhang]

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

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

[Guopeng Zhang]

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

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

[Guopeng Zhang]

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

[PATCH 3/3] sched/psi: Avoid clobbering rtpoll_scheduled during teardown

[Guopeng Zhang]

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