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

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

Hey,

On 01/08/25 18:03, Yuri Andriaccio wrote:
> Hi,
> 
> thanks for reviewing the patch.
> 
> > > @@ -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?
> 
> Now that I think about it, you are correct. Since the fair-servers' rate is
> fixed (i.e. by default 50ms every second), the bandwidth must be scaled for both
> the CPU and the server, or equally, neither needs scaling for the check in
> question.
> 
> ...
> 
> > > @@ -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?
> 
> Yes, I suppose so. I'll update the patch to return as soon as the error
> condition is met.
> 
> Additionally, I'll also update some of the checks in the above function to
> reflect the aforementioned fixed rate behaviour for fair-servers.

Don't think you had a chance to send a new version yet, no worries!

But, I just noticed that this seems to regress cpu hotplug. With this
applied, offlining of cpus fails with device or resource busy on my test
system. Can you please double check?

Thanks!
Juri