Re: [PATCH v2] sched/deadline: Remove fair-servers from real-time task's bandwidth accounting

[Juri Lelli <juri.lelli@redhat.com>]

Hi Yuri,

On 25/07/25 18:44, Yuri Andriaccio wrote:
> Fair-servers are currently used in place of the old RT_THROTTLING mechanism to
> prevent the starvation of SCHED_OTHER (and other lower priority) tasks when
> real-time FIFO/RR processes are trying to fully utilize the CPU. To allow the
> RT_THROTTLING mechanism, the maximum allocatable bandwidth for real-time tasks
> has been limited to 95% of the CPU-time.
> 
> The RT_THROTTLING mechanism is now removed in favor of fair-servers, which are
> currently set to use, as expected, 5% of the CPU-time. Still, they share the
> same bandwidth that allows to run real-time tasks, and which is still set to 95%
> of the total CPU-time. This means that by removing the RT_THROTTLING mechanism,
> the bandwidth remaning for real-time SCHED_DEADLINE tasks and other dl-servers
> (FIFO/RR are not affected) is only 90%.
> 
> This patch reclaims the 5% lost CPU-time, which is definitely reserved for
> SCHED_OTHER tasks, but should not be accounted togheter with the other real-time
> tasks. More generally, the fair-servers' bandwidth must not be accounted with
> other real-time tasks.
> 
> Updates:
> - Make the fair-servers' bandwidth not be accounted into the total allocated
>   bandwidth for real-time tasks.
> - Remove the admission control test when allocating a fair-server.
> - Do not account for fair-servers in the GRUB's bandwidth reclaiming mechanism.
> - Limit the max bandwidth to (BW_UNIT - max_rt_bw) when changing the parameters
>   of a fair-server, preventing overcommitment.
> - Add dl_bw_fair, which computes the total allocated bandwidth of the
>   fair-servers in the given root-domain.
> - Update admission tests (in sched_dl_global_validate) when changing the
>   maximum allocatable bandwidth for real-time tasks, preventing overcommitment.
> 
> Since the fair-server's bandwidth can be changed through debugfs, it has not
> been enforced that a fair-server's bw must be always equal to (BW_UNIT -
> max_rt_bw), rather it must be less or equal to this value. This allows retaining
> the fair-servers' settings changed through the debugfs when chaning the
> max_rt_bw.
> 
> This also means that in order to increase the maximum bandwidth for real-time
> tasks, the bw of fair-servers must be first decreased through debugfs otherwise
> admission tests will fail, and viceversa, to increase the bw of fair-servers,
> the bw of real-time tasks must be reduced beforehand.
> 
> This v2 version addresses the compilation error on i386 reported at:
> https://lore.kernel.org/oe-kbuild-all/202507220727.BmA1Osdg-lkp@intel.com/
> 
> v1: https://lore.kernel.org/all/20250721111131.309388-1-yurand2000@gmail.com/
> 
> Signed-off-by: Yuri Andriaccio <yurand2000@gmail.com>
> ---

Thanks for this. I have been testing it and it looks good. Just a couple
of comments below.

...

> @@ -1688,17 +1690,14 @@ 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);
> +	max_bw = div64_ul(cap_scale(BW_UNIT - dl_b->bw, cap), (unsigned long)cpus);

fc975cfb3639 ("sched/deadline: Fix dl_server runtime calculation
formula") essentially removed cap/freq scaling for dl-servers. Should we
rather not scale max_bw here as well?

> -	if (__dl_overflow(dl_b, cap, old_bw, new_bw))
> +	if (new_bw > max_bw)
>  		return -EBUSY;
>  
>  	if (init) {
>  		__add_rq_bw(new_bw, &rq->dl);
> -		__dl_add(dl_b, new_bw, cpus);
>  	} else {
> -		__dl_sub(dl_b, dl_se->dl_bw, cpus);
> -		__dl_add(dl_b, new_bw, cpus);
> -
>  		dl_rq_change_utilization(rq, dl_se, new_bw);
>  	}

...

> @@ -3149,10 +3138,13 @@ int sched_dl_global_validate(void)
>  			goto next;
>  
>  		dl_b = dl_bw_of(cpu);
> -		cpus = dl_bw_cpus(cpu);
> +		cap = dl_bw_capacity(cpu);
> +		fair_bw = dl_bw_fair(cpu);
>  
>  		raw_spin_lock_irqsave(&dl_b->lock, flags);
> -		if (new_bw * cpus < dl_b->total_bw)
> +		if (cap_scale(new_bw, cap) < dl_b->total_bw)
> +			ret = -EBUSY;

It's kind of a minor one, but can't we return early at this point already?

> +		if (cap_scale(new_bw, cap) + fair_bw > cap_scale(BW_UNIT, cap))
>  			ret = -EBUSY;
>  		raw_spin_unlock_irqrestore(&dl_b->lock, flags);

Thanks!
Juri