Re: [PATCH v7 6/9] sched/fair: Add sched group latency support

[Joel Fernandes]

> On Nov 4, 2022, at 7:21 AM, Qais Yousef <qyousef@xxxxxxxxxxx> wrote:
> 
> On 11/03/22 18:02, Vincent Guittot wrote:
>>> On Thu, 3 Nov 2022 at 15:27, Qais Yousef <qyousef@xxxxxxxxxxx> wrote:
>>> 
>>> On 11/03/22 09:46, Vincent Guittot wrote:
>>>> On Tue, 1 Nov 2022 at 20:28, Qais Yousef <qyousef@xxxxxxxxxxx> wrote:
>>>>> 
>>>>> On 10/28/22 11:34, Vincent Guittot wrote:
>>>>>> Task can set its latency priority with sched_setattr(), which is then used
>>>>>> to set the latency offset of its sched_enity, but sched group entities
>>>>>> still have the default latency offset value.
>>>>>> 
>>>>>> Add a latency.nice field in cpu cgroup controller to set the latency
>>>>>> priority of the group similarly to sched_setattr(). The latency priority
>>>>>> is then used to set the offset of the sched_entities of the group.
>>>>>> 
>>>>>> Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
>>>>>> ---
>>>>>> Documentation/admin-guide/cgroup-v2.rst |  8 ++++
>>>>>> kernel/sched/core.c                     | 52 +++++++++++++++++++++++++
>>>>>> kernel/sched/fair.c                     | 33 ++++++++++++++++
>>>>>> kernel/sched/sched.h                    |  4 ++
>>>>>> 4 files changed, 97 insertions(+)
>>>>>> 
>>>>>> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
>>>>>> index be4a77baf784..d8ae7e411f9c 100644
>>>>>> --- a/Documentation/admin-guide/cgroup-v2.rst
>>>>>> +++ b/Documentation/admin-guide/cgroup-v2.rst
>>>>>> @@ -1095,6 +1095,14 @@ All time durations are in microseconds.
>>>>>>         values similar to the sched_setattr(2). This maximum utilization
>>>>>>         value is used to clamp the task specific maximum utilization clamp.
>>>>>> 
>>>>>> +  cpu.latency.nice
>>>>>> +     A read-write single value file which exists on non-root
>>>>>> +     cgroups.  The default is "0".
>>>>>> +
>>>>>> +     The nice value is in the range [-20, 19].
>>>>>> +
>>>>>> +     This interface file allows reading and setting latency using the
>>>>>> +     same values used by sched_setattr(2).
>>>>> 
>>>>> I'm still not sure about this [1].
>>>> 
>>>> I'm still not sure about what you are trying to say here ...
>>>> 
>>>> This is about setting a latency nice prio to a group level.
>>>> 
>>>>> 
>>>>> In some scenarios we'd like to get the effective latency_nice of the task. How
>>>>> will the task inherit the cgroup value or be impacted by it?
>>>>> 
>>>>> For example if there are tasks that belong to a latency sensitive cgroup; and
>>>>> I'd like to skip some searches in EAS to improve that latency sensitivity - how
>>>>> would I extract this info in EAS path given these tasks are using default
>>>>> latency_nice value? And if should happen if their latency_nice is set to
>>>>> something else other than default?
>>>>> 
>>>>> [1] https://lore.kernel.org/lkml/20221012160734.hrkb5jcjdq7r23pr@wubuntu/
>>>> 
>>>> Hmm so you are speaking about something that is not part of the patch.
>>>> Let focus on the patchset for now
>>> 
>>> I am focusing on this patchset. Isn't this an essential part of the design?
>>> Once the interface is out we can't change it. As it stands, I can't see how it
>> 
>> So, are you speaking about the interface i.e. setting a value between [-20:19]
> 
> About how the cgroup and per task interface interact.
> 
> How to get the effective value of latency_nice for a task that belongs to
> a hierarchy?
> 
> If I have a task that has p->latency_nice = 20 but it belongs to a cgroup that
> has tg->cpu.latency.nice = -19

Just for the task placement signal, One way is to go through the se hierarchy till the root and get the minimum. Then make that the effective value. So In your example that would make it -19 so prefer idle = 1.  We should need a Boolean signal. Not pretty but not the end of the world imho.

> And I want to use this interface in EAS; how should I interpret these values?
> How should I walk up the hierarchy and decide the _effective_ latency_nice
> value

Ah, using a min function?

Joel



>> 
> Thanks
> 
> --
> Qais Yousef
> 
>> 
>>> 
>>> 
>>> Thanks
>>> 
>>> --
>>> Qais Yousef