2022-03-28 22:10:08

by Vincent Guittot

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add latency_nice priority

Hi Dietmar,


On Mon, 28 Mar 2022 at 11:24, Dietmar Eggemann <[email protected]> wrote:
>
> On 11/03/2022 17:14, Vincent Guittot wrote:
> > This patchset restarts the work about adding a latency nice priority to
> > describe the latency tolerance of cfs tasks.
> >
> > The patches [1-4] have been done by Parth:
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > I have just rebased and moved the set of latency priority outside the
> > priority update. I have removed the reviewed tag because the patches
> > are 2 years old.
> >
> > The patches [5-6] use latency nice priority to decide if a cfs task can
> > preempt the current running task. Patch 5 gives some tests results with
> > cyclictests and hackbench to highlight the benefit of latency nice
> > priority for short interactive task or long intensive tasks.
>
> The Android specific `latency_nice` (in Android `latency_sensitive`
> [latency_nice < 0]) use case `Skip energy aware task placement` favors
> an idle CPU over the EAS search path for a `latency_sensitive` task.
>
> https://lkml.kernel.org/r/[email protected]
>
> This is Android proprietary code similar to what we have in
> find_idlest_group_cpu() in mainline.
> We talked to the Android folks last week and IMHO they are not convinced
> that they can switch this to the proposed `latency_nice->tweak
> preemption` use case.

Thanks for discussing this with Android folks. It's not always easy to
change the behavior of a product and I would be interested to discuss
this with them. Sometimes you need a PoC to get convinced


2022-04-01 18:34:30

by Qais Yousef

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add latency_nice priority

+CC Wei

On 03/28/22 14:56, Vincent Guittot wrote:
> Hi Dietmar,
>
>
> On Mon, 28 Mar 2022 at 11:24, Dietmar Eggemann <[email protected]> wrote:
> >
> > On 11/03/2022 17:14, Vincent Guittot wrote:
> > > This patchset restarts the work about adding a latency nice priority to
> > > describe the latency tolerance of cfs tasks.
> > >
> > > The patches [1-4] have been done by Parth:
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > I have just rebased and moved the set of latency priority outside the
> > > priority update. I have removed the reviewed tag because the patches
> > > are 2 years old.
> > >
> > > The patches [5-6] use latency nice priority to decide if a cfs task can
> > > preempt the current running task. Patch 5 gives some tests results with
> > > cyclictests and hackbench to highlight the benefit of latency nice
> > > priority for short interactive task or long intensive tasks.
> >
> > The Android specific `latency_nice` (in Android `latency_sensitive`
> > [latency_nice < 0]) use case `Skip energy aware task placement` favors
> > an idle CPU over the EAS search path for a `latency_sensitive` task.
> >
> > https://lkml.kernel.org/r/[email protected]
> >
> > This is Android proprietary code similar to what we have in
> > find_idlest_group_cpu() in mainline.
> > We talked to the Android folks last week and IMHO they are not convinced
> > that they can switch this to the proposed `latency_nice->tweak
> > preemption` use case.
>
> Thanks for discussing this with Android folks. It's not always easy to
> change the behavior of a product and I would be interested to discuss
> this with them. Sometimes you need a PoC to get convinced

I think it's good to clarify for me at least here whether you intend this as
a replacement for disable EAS and revert to CAS or you see this as an
additional thing? As I understood from the discussion we had on the cover
letter, this is an additional improvement and not intended to replace any of
the previous use cases we brought up before.

Wei/Quentin, any thoughts on this?

Thanks

--
Qais Yousef

2022-04-05 00:52:09

by Vincent Guittot

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add latency_nice priority

On Fri, 1 Apr 2022 at 14:15, Qais Yousef <[email protected]> wrote:
>
> +CC Wei
>
> On 03/28/22 14:56, Vincent Guittot wrote:
> > Hi Dietmar,
> >
> >
> > On Mon, 28 Mar 2022 at 11:24, Dietmar Eggemann <[email protected]> wrote:
> > >
> > > On 11/03/2022 17:14, Vincent Guittot wrote:
> > > > This patchset restarts the work about adding a latency nice priority to
> > > > describe the latency tolerance of cfs tasks.
> > > >
> > > > The patches [1-4] have been done by Parth:
> > > > https://lore.kernel.org/lkml/[email protected]/
> > > >
> > > > I have just rebased and moved the set of latency priority outside the
> > > > priority update. I have removed the reviewed tag because the patches
> > > > are 2 years old.
> > > >
> > > > The patches [5-6] use latency nice priority to decide if a cfs task can
> > > > preempt the current running task. Patch 5 gives some tests results with
> > > > cyclictests and hackbench to highlight the benefit of latency nice
> > > > priority for short interactive task or long intensive tasks.
> > >
> > > The Android specific `latency_nice` (in Android `latency_sensitive`
> > > [latency_nice < 0]) use case `Skip energy aware task placement` favors
> > > an idle CPU over the EAS search path for a `latency_sensitive` task.
> > >
> > > https://lkml.kernel.org/r/[email protected]
> > >
> > > This is Android proprietary code similar to what we have in
> > > find_idlest_group_cpu() in mainline.
> > > We talked to the Android folks last week and IMHO they are not convinced
> > > that they can switch this to the proposed `latency_nice->tweak
> > > preemption` use case.
> >
> > Thanks for discussing this with Android folks. It's not always easy to
> > change the behavior of a product and I would be interested to discuss
> > this with them. Sometimes you need a PoC to get convinced
>
> I think it's good to clarify for me at least here whether you intend this as
> a replacement for disable EAS and revert to CAS or you see this as an
> additional thing? As I understood from the discussion we had on the cover
> letter, this is an additional improvement and not intended to replace any of
> the previous use cases we brought up before.

This is not a replacement but an additional way to improve latency.
The only thing that could happen is that this feature provides
enhancement that makes the policy of disabling EAS and revert to CAS
becoming less or no more interesting.


>
> Wei/Quentin, any thoughts on this?
>
> Thanks
>
> --
> Qais Yousef

2022-04-10 17:06:26

by Qais Yousef

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add latency_nice priority

On 04/02/22 10:46, Vincent Guittot wrote:
> On Fri, 1 Apr 2022 at 14:15, Qais Yousef <[email protected]> wrote:
> >
> > +CC Wei
> >
> > On 03/28/22 14:56, Vincent Guittot wrote:
> > > Hi Dietmar,
> > >
> > >
> > > On Mon, 28 Mar 2022 at 11:24, Dietmar Eggemann <[email protected]> wrote:
> > > >
> > > > On 11/03/2022 17:14, Vincent Guittot wrote:
> > > > > This patchset restarts the work about adding a latency nice priority to
> > > > > describe the latency tolerance of cfs tasks.
> > > > >
> > > > > The patches [1-4] have been done by Parth:
> > > > > https://lore.kernel.org/lkml/[email protected]/
> > > > >
> > > > > I have just rebased and moved the set of latency priority outside the
> > > > > priority update. I have removed the reviewed tag because the patches
> > > > > are 2 years old.
> > > > >
> > > > > The patches [5-6] use latency nice priority to decide if a cfs task can
> > > > > preempt the current running task. Patch 5 gives some tests results with
> > > > > cyclictests and hackbench to highlight the benefit of latency nice
> > > > > priority for short interactive task or long intensive tasks.
> > > >
> > > > The Android specific `latency_nice` (in Android `latency_sensitive`
> > > > [latency_nice < 0]) use case `Skip energy aware task placement` favors
> > > > an idle CPU over the EAS search path for a `latency_sensitive` task.
> > > >
> > > > https://lkml.kernel.org/r/[email protected]
> > > >
> > > > This is Android proprietary code similar to what we have in
> > > > find_idlest_group_cpu() in mainline.
> > > > We talked to the Android folks last week and IMHO they are not convinced
> > > > that they can switch this to the proposed `latency_nice->tweak
> > > > preemption` use case.
> > >
> > > Thanks for discussing this with Android folks. It's not always easy to
> > > change the behavior of a product and I would be interested to discuss
> > > this with them. Sometimes you need a PoC to get convinced
> >
> > I think it's good to clarify for me at least here whether you intend this as
> > a replacement for disable EAS and revert to CAS or you see this as an
> > additional thing? As I understood from the discussion we had on the cover
> > letter, this is an additional improvement and not intended to replace any of
> > the previous use cases we brought up before.
>
> This is not a replacement but an additional way to improve latency.
> The only thing that could happen is that this feature provides
> enhancement that makes the policy of disabling EAS and revert to CAS
> becoming less or no more interesting.

(Sorry for delayed response, in holiday)

Okay thanks for clarifying.

One other corner case to consider if you're working on next version is what
should happen when there are multiple tasks of the same priority on the rq. RT
scheduler will push/pull tasks to ensure the task will get to run ASAP if
there's another cpu at lower priority is available. Seems a lot of complexity
to add to CFS, but at the same time if 2 important tasks require low latency
are on the same rq, one of them will suffer without introducing the ability to
migrate one of them where it can get to run sooner.

--
Qais Yousef

2022-04-12 05:16:19

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add latency_nice priority

On Sat, 9 Apr 2022 18:08:41 +0100
Qais Yousef <[email protected]> wrote:

> One other corner case to consider if you're working on next version is what
> should happen when there are multiple tasks of the same priority on the rq. RT
> scheduler will push/pull tasks to ensure the task will get to run ASAP if
> there's another cpu at lower priority is available. Seems a lot of complexity
> to add to CFS, but at the same time if 2 important tasks require low latency
> are on the same rq, one of them will suffer without introducing the ability to
> migrate one of them where it can get to run sooner.

Instead of having the greedy algorithm of the RT push/pull logic, how
hard would it be to have the load balancer know of these tasks, and try
to keep them on different CPUs? When two are queued on the same CPU,
could it be possible to just trigger load balancing and let it do the
work?

-- Steve