Re: [RFC PATCH] sched/core: Stash task priority after dequeue and put_prev_task() in sched_change_begin()

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Jan 06, 2026 at 07:52:39AM +0000, K Prateek Nayak wrote:
> When running amd-pstate driver on a PREEMPT_RT kernel on a shared memory
> system (Zen3 and prior), the following splat was observed from
> triggering the WARN_ON_ONCE() in rq_pin_lock():
> 
>     ------------[ cut here ]------------
>     WARNING: kernel/sched/sched.h:1807 at __schedule+0x122/0x17c0, CPU#8: swapper/0/1

Can you enable CONFIG_DEBUG_BUGVERBOSE_DETAILED?

(not critical this time, since you already said rq_pin_lock() and that
only has the one WARN; it does help in general because 'obviously' 1807
isn't actually in rq_pin_lock() for me).

>     Call Trace:
>      <TASK>
>      preempt_schedule+0x41/0x60
>      preempt_schedule_thunk+0x16/0x30
>      try_to_wake_up+0x341/0x7c0
>      autoremove_wake_function+0x12/0x40
>      __wake_up_common+0x78/0xa0
>      __wake_up+0x31/0x50
>      send_pcc_cmd+0x133/0x310
>      cppc_set_reg_val+0x10e/0x220

> 
> Inspecting the set of events that led to the warning being triggered
> showed the following:
> 
>     systemd-1  [008] dN.31 ...: do_set_cpus_allowed: set_cpus_allowed begin!
> 
>     systemd-1  [008] dN.31 ...: sched_change_begin: Begin!
>     systemd-1  [008] dN.31 ...: sched_change_begin: Before dequeue_task()!
>     systemd-1  [008] dN.31 ...: update_curr_dl_se: update_curr_dl_se: ENQUEUE_REPLENISH
>     systemd-1  [008] dN.31 ...: enqueue_dl_entity: enqueue_dl_entity: ENQUEUE_REPLENISH
>     systemd-1  [008] dN.31 ...: replenish_dl_entity: Replenish before: 14815760217
>     systemd-1  [008] dN.31 ...: replenish_dl_entity: Replenish after: 14816960047
>     systemd-1  [008] dN.31 ...: sched_change_begin: Before put_prev_task()!
> 
>     systemd-1  [008] dN.31 ...: sched_change_end: Before enqueue_task()!
>     systemd-1  [008] dN.31 ...: sched_change_end: Before put_prev_task()!
>     systemd-1  [008] dN.31 ...: prio_changed_dl: Queuing pull task on prio change: 14815760217 -> 14816960047
>     systemd-1  [008] dN.31 ...: prio_changed_dl: Queuing balance callback!
>     systemd-1  [008] dN.31 ...: sched_change_end: End!
> 
>     systemd-1  [008] dN.31 ...: do_set_cpus_allowed: set_cpus_allowed end!
>     systemd-1  [008] dN.21 ...: __schedule: Woops! Balance callback found!
> 
> 1. sched_change_begin() from guard(sched_change) in
>    do_set_cpus_allowed() stashes the priority, which for the deadline
>    task, is "p->dl.deadline".
> 2. The dequeue of the deadline task replenishes the deadline.
> 3. The task is enqueued back after guard's scope ends and since there is
>    no *_CLASS flags set, sched_change_end() calls
>    dl_sched_class->prio_changed() which compares the deadline.
> 4. Since deadline was moved on dequeue, prio_changed_dl() sees the value
>    differ from the stashed value and queues a balance pull callback.
> 5. do_set_cpus_allowed() finishes and drops the rq_lock without doing a
>    do_balance_callbacks().
> 6. Grabbing the rq_lock() at subsequent __schedule() triggers the
>    warning since the balance pull callback was never executed before
>    dropping the lock.
> 
> Since the dequeue on a deadline task can push its deadline, stash the
> task prio towards the end of sched_change_begin().
> 
> The modification to priority within the sched_change guard's scope will
> still be considered as sched_change_end() will supply the priority
> stashed at the end of constructor's execution as the old priority to
> sched_class->prio_changed().
> 

Would not something like so make more sense?

---
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 80c9559a3e30..60e0c25aae78 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -3306,6 +3306,8 @@ static void switched_to_dl(struct rq *rq, struct task_struct *p)
 
 static u64 get_prio_dl(struct rq *rq, struct task_struct *p)
 {
+	if (task_current_donor(rq, p))
+		update_curr_dl(rq);
 	return p->dl.deadline;
 }