Re: [PATCH v2 04/21] sched/deadline: Do not divide by zero active CPUs of a root domain

[Juri Lelli]

Hello,

On 04/11/26 16:11, Mathieu Desnoyers wrote:
> __dl_sub() and __dl_add() spread a change of a root domain's total_bw
> over the extra_bw of its active CPUs, dividing it by their number, which
> their callers take from dl_bw_cpus(). That number is 0 for a root domain
> whose CPUs all went offline, such as the default root domain, which
> holds the offline CPUs: the division then traps.
> 
> A debugfs write of a fair server's runtime or period reaches it while
> its CPU goes offline or comes online. From the rebuild of the root
> domains in sched_cpu_deactivate() until the CPU is offline, and from the
> CPU online until sched_cpu_activate(), the CPU sits in the default root
> domain, with no active CPU. A write that leaves the server's bandwidth
> as it is passes admission there, __dl_overflow() comparing that root
> domain's capacity, 0, with total_bw less the old bandwidth plus the new,
> 0 as well, and dl_server_apply_params() divides. Any other value is
> refused with EBUSY, higher directly, lower through the unsigned
> subtraction. Once the CPU is offline the write is refused, since
> commit 4043f5498416 ("sched/deadline: Reject debugfs dl_server writes
> for offline CPUs").
> 
> Writing a fair server's own runtime back in a loop while its CPU goes
> offline and online oopses on the first round, in 8-CPU VMs of v7.4-rc1
> with the four deadline fixes of sched/urgent:
> 
>   Oops: divide error: 0000 [#1] SMP NOPTI
>   CPU: 6 UID: 0 PID: 110 Comm: dl_server_div0
>   RIP: 0010:dl_server_apply_params+0x335/0x380
>   Call Trace:
>    <TASK>
>    sched_server_write_common.isra.0+0x1c0/0x260
>    full_proxy_write+0x58/0x90
>    vfs_write+0xcb/0x550
>    ksys_write+0x6c/0xe0
>    do_syscall_64+0x109/0x5d0
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> inactive_task_timer() can reach it too, found by reading: it releases
> the bandwidth of a task that left SCHED_DEADLINE, or died, before its
> 0-lag time from the root domain of the CPU the task is on, and that CPU
> may go offline before the timer fires. With admission control off, so
> may sched_setattr() to SCHED_DEADLINE of a task asleep on an offline
> CPU.
> 
> With no active CPU there is no extra_bw to update: change total_bw
> alone. 20 rounds of the reproducer leave no warning then.
> 
> Fixes: daec57983670 ("sched/deadline: Reclaim bandwidth not used by dl tasks")
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> ---
>  kernel/sched/deadline.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 963fda6ddb7f..dadda1a528ec 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -189,18 +189,25 @@ void __dl_update(struct dl_bw *dl_b, s64 bw)
>  	}
>  }
>  
> +/*
> + * @cpus is the number of active CPUs of the root domain, whose extra_bw
> + * shares the change: none once they all went offline, and total_bw alone
> + * changes then.
> + */
>  static inline
>  void __dl_sub(struct dl_bw *dl_b, u64 tsk_bw, int cpus)
>  {
>  	dl_b->total_bw -= tsk_bw;
> -	__dl_update(dl_b, (s32)tsk_bw / cpus);
> +	if (cpus)
> +		__dl_update(dl_b, (s32)tsk_bw / cpus);
>  }
>  
>  static inline
>  void __dl_add(struct dl_bw *dl_b, u64 tsk_bw, int cpus)
>  {
>  	dl_b->total_bw += tsk_bw;
> -	__dl_update(dl_b, -((s32)tsk_bw / cpus));
> +	if (cpus)
> +		__dl_update(dl_b, -((s32)tsk_bw / cpus));
>  }

I wonder if it makes sense to add/sub from total_bw when cpus == 0.

Wouldn't be better, if possible, to avoid calls with 0 cpus?

Thanks,
Juri