Re: [PATCH v2 3/2] sched/deadline: Check bandwidth overflow earlier for hotplug

[Qais Yousef <qyousef@layalina.io>]

On 02/17/25 15:52, Juri Lelli wrote:
> On 16/02/25 16:33, Qais Yousef wrote:
> > On 02/13/25 07:20, Juri Lelli wrote:
> > > On 12/02/25 19:22, Dietmar Eggemann wrote:
> > > > On 11/02/2025 11:42, Juri Lelli wrote:
> > > 
> > > ...
> > > 
> > > > > What about we actually ignore them consistently? We already do that for
> > > > > admission control, so maybe we can do that when rebuilding domains as
> > > > > well (until we find maybe a better way to deal with them).
> > > > > 
> > > > > Does the following make any difference?
> > > > 
> > > > It at least seems to solve the issue. And like you mentioned on irc, we
> > > > don't know the bw req of sugov anyway.
> > > > 
> > > > So with this change we start with 'dl_bw->total_bw = 0' even w/ sugov tasks.
> > > > 
> > > > dl_rq[0]:
> > > >   .dl_nr_running                 : 0
> > > >   .dl_bw->bw                     : 996147
> > > >   .dl_bw->total_bw               : 0       <-- !
> > > > 
> > > > IMHO, people who want to run serious DL can always check whether there
> > > > are already these infrastructural DL tasks or even avoid schedutil.
> > > 
> > > It definitely not ideal and admittedly gross, but not worse than what we
> > > are doing already considering we ignore sugovs at AC and the current
> > > bandwidth allocation its there only to help with PI. So, duck tape. :/
> > > 
> > > A more proper way to work with this would entail coming up with sensible
> > > bandwidth allocation for sugovs, but that's most probably hardware
> > > specific, so I am not sure how we can make that general enough.
> > 
> > I haven't been following the problem closely, but one thing I was considering
> > and I don't know if it makes sense to you and could help with this problem too.
> > Shall we lump sugov with stopper class or create a new sched_class (seems
> > unnecessary, I think stopper should do)? With the consolidate cpufreq update
> > patch I've been working on Vincent raised issues with potential new ctx switch
> > and to improve that I needed to look at improving sugov wakeup path. If we
> > decouple it from DL I think that might fix your problem here and could allow us
> > to special case it for other problems like the ones I faced more easily without
> > missing up with DL.
> > 
> > Has the time come to consider retire the simple solution of making sugov a fake
> > DL task?
> 
> Problem is that 'ideally' we would want to explicitly take sugovs into
> account when designing the system. We don't do that currently as a
> 'temporary solution' that seemed simpler than a proper approach (started
> wondering if it's indeed simpler). So, not sure if moving sugovs outside
> DL is something we want to do.

Okay I see. The issue though is that for a DL system with power management
features on that warrant to wake up a sugov thread to update the frequency is
sort of half broken by design. I don't see the benefit over using RT in this
case. But I appreciate I could be misguided. So take it easy on me if it is
obviously wrong understanding :) I know in Android usage of DL has been
difficult, but many systems ship with slow switch hardware.

How does DL handle the long softirqs from block and network layers by the way?
This has been in a practice a problem for RT tasks so they should be to DL.
sugov done in stopper should be handled similarly IMHO. I *think* it would be
simpler to masquerade sugov thread as irq pressure.

You can use the rate_limit_us as a potential guide for how much bandwidth sugov
needs if moving it to another class really doesn't make sense instead?