[PATCH v3 0/2] Rework interface between scheduler and schedutil governor

Vincent Guittot posted 2 patches 2 years, 1 month ago
There is a newer version of this series
include/linux/energy_model.h     |  1 -
kernel/sched/core.c              | 82 ++++++++++++-------------------
kernel/sched/cpufreq_schedutil.c | 69 ++++++++++++++++----------
kernel/sched/fair.c              | 22 +++++++--
kernel/sched/sched.h             | 84 +++-----------------------------
5 files changed, 100 insertions(+), 158 deletions(-)
[PATCH v3 0/2] Rework interface between scheduler and schedutil governor
Posted by Vincent Guittot 2 years, 1 month ago
Following the discussion with Qais [1] about how to handle uclamp
requirements and after syncing with him, we agreed that I should move
forward on the patchset to rework the interface between scheduler and
schedutil governor to provide more information to the latter. Scheduler
(and EAS in particular) doesn't need anymore to guess estimate which
headroom the governor wants to apply and will directly ask for the target
freq. Then the governor directly gets the actual utilization and new
minimum and maximum boundaries to select this target frequency and
doesn't have to deal anymore with scheduler internals like uclamp when
including iowait boost.

[1] https://lore.kernel.org/lkml/CAKfTPtA5JqNCauG-rP3wGfq+p8EEVx9Tvwj6ksM3SYCwRmfCTg@mail.gmail.com/

Changes since v2:
- remove useless target variable

Changes since v1:
- fix a bug (always set max even when returning early)
- fix typos
  
Vincent Guittot (2):
  sched/schedutil: Rework performance estimation
  sched/schedutil: Rework iowait boost

 include/linux/energy_model.h     |  1 -
 kernel/sched/core.c              | 82 ++++++++++++-------------------
 kernel/sched/cpufreq_schedutil.c | 69 ++++++++++++++++----------
 kernel/sched/fair.c              | 22 +++++++--
 kernel/sched/sched.h             | 84 +++-----------------------------
 5 files changed, 100 insertions(+), 158 deletions(-)

-- 
2.34.1
Re: [PATCH v3 0/2] Rework interface between scheduler and schedutil governor
Posted by Rafael J. Wysocki 2 years, 1 month ago
On Fri, Nov 3, 2023 at 2:18 PM Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> Following the discussion with Qais [1] about how to handle uclamp
> requirements and after syncing with him, we agreed that I should move
> forward on the patchset to rework the interface between scheduler and
> schedutil governor to provide more information to the latter. Scheduler
> (and EAS in particular) doesn't need anymore to guess estimate which
> headroom the governor wants to apply and will directly ask for the target
> freq. Then the governor directly gets the actual utilization and new
> minimum and maximum boundaries to select this target frequency and
> doesn't have to deal anymore with scheduler internals like uclamp when
> including iowait boost.
>
> [1] https://lore.kernel.org/lkml/CAKfTPtA5JqNCauG-rP3wGfq+p8EEVx9Tvwj6ksM3SYCwRmfCTg@mail.gmail.com/
>
> Changes since v2:
> - remove useless target variable
>
> Changes since v1:
> - fix a bug (always set max even when returning early)
> - fix typos
>
> Vincent Guittot (2):
>   sched/schedutil: Rework performance estimation
>   sched/schedutil: Rework iowait boost
>
>  include/linux/energy_model.h     |  1 -
>  kernel/sched/core.c              | 82 ++++++++++++-------------------
>  kernel/sched/cpufreq_schedutil.c | 69 ++++++++++++++++----------
>  kernel/sched/fair.c              | 22 +++++++--
>  kernel/sched/sched.h             | 84 +++-----------------------------
>  5 files changed, 100 insertions(+), 158 deletions(-)
>
> --

For the schedutil changes in the series:

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

and I'm assuming this series to be targeted at sched.
Re: [PATCH v3 0/2] Rework interface between scheduler and schedutil governor
Posted by Peter Zijlstra 2 years, 1 month ago
On Mon, Nov 06, 2023 at 04:05:40PM +0100, Rafael J. Wysocki wrote:
> On Fri, Nov 3, 2023 at 2:18 PM Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
> >
> > Following the discussion with Qais [1] about how to handle uclamp
> > requirements and after syncing with him, we agreed that I should move
> > forward on the patchset to rework the interface between scheduler and
> > schedutil governor to provide more information to the latter. Scheduler
> > (and EAS in particular) doesn't need anymore to guess estimate which
> > headroom the governor wants to apply and will directly ask for the target
> > freq. Then the governor directly gets the actual utilization and new
> > minimum and maximum boundaries to select this target frequency and
> > doesn't have to deal anymore with scheduler internals like uclamp when
> > including iowait boost.
> >
> > [1] https://lore.kernel.org/lkml/CAKfTPtA5JqNCauG-rP3wGfq+p8EEVx9Tvwj6ksM3SYCwRmfCTg@mail.gmail.com/
> >
> > Changes since v2:
> > - remove useless target variable
> >
> > Changes since v1:
> > - fix a bug (always set max even when returning early)
> > - fix typos
> >
> > Vincent Guittot (2):
> >   sched/schedutil: Rework performance estimation
> >   sched/schedutil: Rework iowait boost
> >
> >  include/linux/energy_model.h     |  1 -
> >  kernel/sched/core.c              | 82 ++++++++++++-------------------
> >  kernel/sched/cpufreq_schedutil.c | 69 ++++++++++++++++----------
> >  kernel/sched/fair.c              | 22 +++++++--
> >  kernel/sched/sched.h             | 84 +++-----------------------------
> >  5 files changed, 100 insertions(+), 158 deletions(-)
> >
> > --
> 
> For the schedutil changes in the series:
> 
> Acked-by: Rafael J. Wysocki <rafael@kernel.org>
> 
> and I'm assuming this series to be targeted at sched.

Sure, I'll go queue it. Thanks!
Re: [PATCH v3 0/2] Rework interface between scheduler and schedutil governor
Posted by Qais Yousef 2 years, 1 month ago
On 11/16/23 15:34, Peter Zijlstra wrote:
> On Mon, Nov 06, 2023 at 04:05:40PM +0100, Rafael J. Wysocki wrote:
> > On Fri, Nov 3, 2023 at 2:18 PM Vincent Guittot
> > <vincent.guittot@linaro.org> wrote:
> > >
> > > Following the discussion with Qais [1] about how to handle uclamp
> > > requirements and after syncing with him, we agreed that I should move
> > > forward on the patchset to rework the interface between scheduler and
> > > schedutil governor to provide more information to the latter. Scheduler
> > > (and EAS in particular) doesn't need anymore to guess estimate which
> > > headroom the governor wants to apply and will directly ask for the target
> > > freq. Then the governor directly gets the actual utilization and new
> > > minimum and maximum boundaries to select this target frequency and
> > > doesn't have to deal anymore with scheduler internals like uclamp when
> > > including iowait boost.

Thanks a lot for taking over Vincent and helping with this! And sorry for
delayed review, was out travelling between holiday and LPC so haven't caught up
with the list properly yet..

Beside the comments on patch 1, it looks good to me. Do we want to generalize
the way the interface is called though so that scheduler is not tightly coupled
to schedutil? Speaking with Intel folks in LPC, it seemed they rely on firmware
to make a lot of decision and if we further generalize how the interface is
called (I think we need a new cpufreq wrapper akin to cpufreq_update_util()) to
allow governors to hook into it and do their own thing. This could allow them
to use uclamp and these min/max perf hints.

But I haven't thought this fully through. So something to consider separately
anyway to not hold this up unnecessarily. Maybe we do want to keep schedutil
tightly integrated and get people to switch to schedutil instead..

> > >
> > > [1] https://lore.kernel.org/lkml/CAKfTPtA5JqNCauG-rP3wGfq+p8EEVx9Tvwj6ksM3SYCwRmfCTg@mail.gmail.com/
> > >
> > > Changes since v2:
> > > - remove useless target variable
> > >
> > > Changes since v1:
> > > - fix a bug (always set max even when returning early)
> > > - fix typos
> > >
> > > Vincent Guittot (2):
> > >   sched/schedutil: Rework performance estimation
> > >   sched/schedutil: Rework iowait boost
> > >
> > >  include/linux/energy_model.h     |  1 -
> > >  kernel/sched/core.c              | 82 ++++++++++++-------------------
> > >  kernel/sched/cpufreq_schedutil.c | 69 ++++++++++++++++----------
> > >  kernel/sched/fair.c              | 22 +++++++--
> > >  kernel/sched/sched.h             | 84 +++-----------------------------
> > >  5 files changed, 100 insertions(+), 158 deletions(-)
> > >
> > > --
> > 
> > For the schedutil changes in the series:
> > 
> > Acked-by: Rafael J. Wysocki <rafael@kernel.org>
> > 
> > and I'm assuming this series to be targeted at sched.
> 
> Sure, I'll go queue it. Thanks!

Sorry for being late. If this wasn't queued already I think worth waiting to
iron out some comments on patch 1 first.


Thanks!

--
Qais Yousef