Re: [PATCH v5 14/29] sched/deadline: Fix this_bw when a task is changed during a remote wakeup

[Juri Lelli]

Hello,

On 08/11/26 13:53, Mathieu Desnoyers wrote:
> The this_bw of a runqueue is the bandwidth of the deadline tasks
> assigned to it, running or not. GRUB uses it to compute the inactive
> utilization of the runqueue. When a task wakes up on another CPU,
> migrate_task_rq_dl() takes its bandwidth off the old runqueue. The
> enqueue adds it to the new one.
> 
> Between these two steps, no runqueue counts the bandwidth.
> try_to_wake_up() can leave the enqueue to the wake list of the new CPU,
> and return before it runs. task_cpu() already points at the new CPU. A
> change of the task in that window gets this_bw wrong.
> 
> In this example, T is a deadline task that blocks on CPU 0. A timer
> wakes it up there, and try_to_wake_up() picks CPU 1. S, on CPU 2,
> changes the bandwidth of T from "old bw" to "new bw":
> 
>   CPU 0                               CPU 1                       CPU 2
>   T blocks
>   this_bw of CPU 0 counts T
>   <timer> try_to_wake_up(T)
>     set_task_cpu(T, 1)
>       migrate_task_rq_dl()
>         sub_rq_bw(): CPU 0 drops T
>     T on the wake list of CPU 1
>     return
>                                                                   S: sched_setattr(T)
>                                                                     task_rq_lock(T): CPU 1
>                                                                     T is not queued
>                                                                     dl_change_utilization()
>                                                                       this_bw of CPU 1:
>                                                                       - old bw + new bw
>                                       sched_ttwu_pending()
>                                         enqueue_task_dl(T)
>                                           task_contending()
>                                             add_rq_bw():
>                                             this_bw of CPU 1:
>                                             + new bw
> 
> CPU 1 then counts the new bandwidth twice, less the old one. Repeated,
> this makes this_bw drift until it underflows. If T left the class in
> that window, switched_from_dl() would take its bandwidth off CPU 1 the
> same way. A task that enters the class in that window would be counted
> twice, by switched_to_dl() and by the enqueue.
> 
> This needs a wakeup that moves the task, so a root domain of more than
> one CPU, with busy CPUs. CONFIG_SCHED_PROXY_EXEC=y makes such wakeups
> more common. In testing, a task sleeps and wakes in a busy two-CPU
> partition root, and another CPU changes its runtime. The kernel warns
> within a second in each of 8 VMs:
> 
>   WARNING: kernel/sched/deadline.c:352 at dl_rq_change_utilization+0x71/0x2e0, CPU#0: wake_race/109
>    __sub_rq_bw(): this_bw underflow
>   WARNING: kernel/sched/deadline.c:316 at task_contending+0x244/0x260, CPU#6: swapper/6/0
>    __add_running_bw(): running_bw overflow
> 
> Fix this with a new bit, dl_bw_oncpu, that tells whether a runqueue
> counts the bandwidth of the task. add_rq_bw() adds the bandwidth only if
> no runqueue counts it. sub_rq_bw() takes it off only if one does.
> dl_change_utilization() does nothing while no runqueue counts the
> bandwidth. The enqueue adds the new one.
> 
> With this patch, none of the 8 VMs warns. deadline_bw_test, added later
> in this series, tests this in params_waking and random_mix. The Sashiko
> AI review of the first posting found the double count, and a bit that
> the first version of this patch left set.
> 
> Fixes: 8fd27231c330 ("sched/deadline: Track the "total rq utilization" too")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> ---

Reviewed-by: Juri Lelli <juri.lelli@xxxxxxxxxx>

Thanks,
Juri