Re: [PATCH v3 1/8] sched/deadline: Reject debugfs dl_server writes for not-active CPUs.

[Juri Lelli]

Hello,

On 13/11/26 11:50, Sebastian Andrzej Siewior wrote:
> sched_server_write_common() checks if the CPU is online before allowing
> fair_server updates. This is important because updating the fair_server
> runtime/ period will use dl_bw_cpus() will return only the number of
> CPUs which are also marked active.
> 
> The CPU is marked active as the last step during CPU-hotplug. If the CPU
> has been marked "online" but not yet "active" or is stuck between those
> two states then any update of period/ runtime will lead to a divide-by-0
> error.
> 
> Let sched_server_write_common() check of active CPUs instead of online.
> Additonaly, let dl_server_apply_params() return an error if dl_bw_cpus()
> return 0 CPUs to avoid the divide error.
> 
> Fixes: 4043f54984161 ("sched/deadline: Reject debugfs dl_server writes for offline CPUs")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
>  kernel/sched/deadline.c | 3 +++
>  kernel/sched/debug.c    | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index dfae08625342e..10beb54281881 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1920,6 +1920,9 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio
>  	cpus = dl_bw_cpus(cpu);
>  	cap = dl_bw_capacity(cpu);
>  
> +	if (!cpus)
> +		return -EBUSY;
> +
>  	if (__dl_overflow(dl_b, cap, old_bw, new_bw))
>  		return -EBUSY;
>  
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index e6a3b516c703b..1545139e52443 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -410,7 +410,7 @@ static ssize_t sched_server_write_common(struct file *filp, const char __user *u
>  			return  -EINVAL;
>  		}
>  
> -		if (!cpu_online(cpu_of(rq)))
> +		if (!cpu_active(cpu_of(rq)))
>  			return -EBUSY;

Mathieu's approach [1] looks safer to me. sched_cpu_deactivate() calls
set_cpu_active(false) without rq->lock, so a write that passed the
cpu_active() check can still be in flight until sched_set_rq_offline()
(which the !cpus check then has to cover). On the online side,
set_cpu_active(true) runs before the domains rebuild, so the check lets
the write through against the def rd's accounting. rq->online is only
changed under the rq->lock this write already holds.

Thanks,
Juri

1 - 20261108185406.1549770-6-mathieu.desnoyers@xxxxxxxxxxxx