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

From: Joel Fernandes <joel@joelfernandes.org>
To: Daniel Bristot de Oliveira <bristot@kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Valentin Schneider <vschneid@redhat.com>,
	linux-kernel@vger.kernel.org,
	Luca Abeni <luca.abeni@santannapisa.it>,
	Tommaso Cucinotta <tommaso.cucinotta@santannapisa.it>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vineeth Pillai <vineeth@bitbyteword.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Phil Auld <pauld@redhat.com>,
	Suleiman Souhlal <suleiman@google.com>,
	youssefesmat@google.com
Subject: Re: [PATCH v5 7/7] sched/fair: Fair server interface
Date: Thu, 15 Feb 2024 12:41:59 -0500	[thread overview]
Message-ID: <b3e053bf-c9f5-492d-baff-8f251fb00279@joelfernandes.org> (raw)
In-Reply-To: <07b1844a-a9d4-47fb-aea0-c0722b2a8001@kernel.org>

On 2/15/2024 12:27 PM, Daniel Bristot de Oliveira wrote:
> 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.

Nice work on nailing the issue.

> 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.

That's right, we only use deadline for the server. However, on some devices,
schedutil is used and AFAIR its kthread uses SCHED_DEADLINE. I don't anticipate
problems related to admission control and that kthread, so I think your proposed
workaround sounds good to me.

thanks,

 - Joel