Re: [PATCH v5 7/7] sched/fair: Fair server interface

[Daniel Bristot de Oliveira]

On 2/15/24 14:57, Joel Fernandes wrote:
> Hello, Daniel,
> 
> On 2/14/2024 9:23 AM, Daniel Bristot de Oliveira wrote:
>> On 2/13/24 03:13, Joel Fernandes wrote:
>>>
>>>
>>> On 11/4/2023 6:59 AM, Daniel Bristot de Oliveira wrote:
>>>> Add an interface for fair server setup on debugfs.
>>>>
>>>> Each rq have three files under /sys/kernel/debug/sched/rq/CPU{ID}:
>>>>
>>>>  - fair_server_runtime: set runtime in ns
>>>>  - fair_server_period: set period in ns
>>>>  - fair_server_defer: on/off for the defer mechanism
>>>
>>> Btw Daniel, there is an interesting side-effect of this interface having runtime
>>> and period in 2 separate files :)
>>>
>>> Say I want to set a CPU to 5ms / 10ms.
>>>
>>> I cannot set either period or runtime to 5ms or 10ms directly.
>>>
>>> I have to first set period to 100ms, then set runtime to 50ms, then set period
>>> to 50ms, then set runtime to 5ms, then finally set period to 10ms.
>>
>> Hummm yeah I could reproduce that, it seems that it is not even a problem of having
>> two files, but a bug in the logic, I will have a look.
> 
> Thanks for taking a look. My colleague Suleiman hit the issue too. He's able to
> not set 45ms/50ms for instance.

I isolated the problem. It is not an interface problem.

Long story short, the servers are initialized at the defrootdomain, but
the dl_bw info is not being carried over to the new domain because the
servers are not a task.

I am discussing this with Valentin (topology) & Juri. We will try to find a
solution, or at least an presentable XXX: solution... in the next days.

You can work around it by disabling the admission control via:

# sysctl kernel.sched_rt_runtime_us=-1

the the values will be accepted. For the best of my knowledge, you guys are
only using SCHED_RR/FIFO so the admission control for DL is not an issue.

>> I still need to finish testing, and to make a proper cover page with all updates, the
>> latest thing is here (tm):
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/bristot/linux.git/log/?h=dl_server_v6
>>
>> It is based on peter's sched/more. I will probably re-send it today or tomorrow,
>> but at least you can have a look at it.
>>> Another reason to send it is to get the regression test machinery running....
> 
> Sure, looking forward to it. I rebased on above tree and it applied cleanly.
> What I'll do is I will send our patches today (not those in sched/more) after a
> bit more testing and tweaks.
> 
> There are 2 reasons for this:
> 1. Can get the build robot do its thing.
> 2. Our internal system checks whether patches backported were posted upstream to
> list.
> 
> Hope that sounds good to you and we can start reviewing as well.

If it helps downstream for you guys, it is not a problem for me. Still, peter is
the person that has more comments to give so...

-- Daniel