mbox series

[00/14] Modularize schedutil

Message ID 20200507181012.29791-1-qperret@google.com (mailing list archive)
Headers show
Series Modularize schedutil | expand

Message

Quentin Perret May 7, 2020, 6:09 p.m. UTC
Android is trying very hard to use a single kernel image (commonly
called Generic Kernel Image, or GKI), closely aligned with mainline, to
run on all Android devices regardless of the vendor.

The GKI project intends to not only improve the status quo for Android
users directly (less fragmentation simplifies updatability), but also
to benefit upstream by forcing all vendors to agree on one common
kernel, that we push hard to be aligned with mainline.

One challenge to implement GKI is to avoid bloating the kernel by
compiling too many things in, especially given that different devices
need different things. As such, anything that can be turned into a
module helps GKI, by offering an alternative to having that component
built-in. This is true for pretty much anything that can be made
modular, including drivers as well as other kernel components, such as
CPUFreq governors.

Indeed, in practice, Android devices often ship with only one CPUFreq
governor enabled, and don't require any other that would simply waste
memory for no benefits. All CPUFreq governors can already be built as
modules with one notable exception: schedutil. Though popular in
Android, some devices do not use schedutil, which is why it would be
preferable to not have it unconditionally built in GKI. This series is
an attempt to solve this problem, by making schedutil tristate.

While modularization is usually not something we want to see near the
scheduler, it appeared to me as I wrote those patches that the
particular case of schedutil was actually not too bad to implement.
We already have to support switching governors at run-time, simply
because userspace is free to do that, so the infrastructure for turning
sugov on and off dynamically is already there. Loading the code a little
later doesn't seem to make that a lot worse.

Patches 01-05 refactor some code to break the few dependencies on
schedutil being builtin (notably EAS). Patches 06-12 export various
symbols that schedutil needs when compiled as a module. And finally,
patches 13-14 finish off the work by making the Kconfig tristate.

---
The series is based on Peter's sched/fifo [1] branch (because sugov
uses sched_setscheduler_nocheck()).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/log/?h=sched/fifo

Quentin Perret (14):
  sched: Provide sched_set_deadline()
  sched: cpufreq: Use sched_set_deadline() from sugov
  sched: cpufreq: Introduce 'want_eas' governor flag
  sched: cpufreq: Move sched_cpufreq_governor_change()
  sched: cpufreq: Move schedutil_cpu_util()
  arch_topology: Export cpu_scale per-cpu array
  kthread: Export kthread_bind_mask()
  sched/core: Export runqueues per-cpu array
  sched/cpufreq: Export cpufreq_this_cpu_can_update()
  sched/fair: Export cpu_util_freq()
  tick/sched: Export tick_nohz_get_idle_calls_cpu
  x86: Export arch_scale_freq_key
  sched: cpufreq: Use IS_ENABLED() for schedutil
  sched: cpufreq: Modularize schedutil

 arch/x86/kernel/smpboot.c        |   1 +
 drivers/base/arch_topology.c     |   1 +
 drivers/cpufreq/Kconfig          |   2 +-
 include/linux/cpufreq.h          |   6 +-
 include/linux/sched.h            |   2 +
 include/linux/sched/sysctl.h     |   2 +-
 kernel/kthread.c                 |   1 +
 kernel/sched/core.c              |  18 ++++
 kernel/sched/cpufreq.c           |  34 ++++++
 kernel/sched/cpufreq_schedutil.c | 176 +++----------------------------
 kernel/sched/fair.c              | 119 ++++++++++++++++++++-
 kernel/sched/sched.h             |  36 ++-----
 kernel/sched/topology.c          |  16 +--
 kernel/sysctl.c                  |   2 +-
 kernel/time/tick-sched.c         |   1 +
 15 files changed, 212 insertions(+), 205 deletions(-)

Comments

Valentin Schneider May 7, 2020, 9:34 p.m. UTC | #1
(+Ionela)

Hi Quentin,

Apologies for sidetracking this a bit.

On 07/05/20 19:09, Quentin Perret wrote:
> Android is trying very hard to use a single kernel image (commonly
> called Generic Kernel Image, or GKI), closely aligned with mainline, to
> run on all Android devices regardless of the vendor.
>
> The GKI project intends to not only improve the status quo for Android
> users directly (less fragmentation simplifies updatability), but also
> to benefit upstream by forcing all vendors to agree on one common
> kernel, that we push hard to be aligned with mainline.
>
> One challenge to implement GKI is to avoid bloating the kernel by
> compiling too many things in, especially given that different devices
> need different things. As such, anything that can be turned into a
> module helps GKI, by offering an alternative to having that component
> built-in. This is true for pretty much anything that can be made
> modular, including drivers as well as other kernel components, such as
> CPUFreq governors.
>
> Indeed, in practice, Android devices often ship with only one CPUFreq
> governor enabled, and don't require any other that would simply waste
> memory for no benefits. All CPUFreq governors can already be built as
> modules with one notable exception: schedutil. Though popular in
> Android, some devices do not use schedutil, which is why it would be
> preferable to not have it unconditionally built in GKI. This series is
> an attempt to solve this problem, by making schedutil tristate.
>

I'm curious; why would some Android device not want to roll with schedutil?

When it comes to dynamic policies (i.e. forget performance / powersave, and
put userspace in a corner), I'd be willing to take a stand and say you
should only really be using schedutil nowadays - alignment with the
scheduler, uclamp, yadda yadda.

AFAIA the only schedutil-related quirk we oughta fix for arm/arm64 is that
arch_scale_freq_invariant() thingie, and FWIW I'm hoping to get something
regarding this out sometime soonish. After that, I'd actually want to make
schedutil the default governor for arm/arm64.

I'm not opiniated on the modularization, but if you can, could you please
share some more details as to why schedutil cannot fulfill its role of holy
messiah of governors for GKI?
Viresh Kumar May 8, 2020, 5:33 a.m. UTC | #2
On 07-05-20, 19:09, Quentin Perret wrote:
> Android is trying very hard to use a single kernel image (commonly
> called Generic Kernel Image, or GKI), closely aligned with mainline, to
> run on all Android devices regardless of the vendor.
> 
> The GKI project intends to not only improve the status quo for Android
> users directly (less fragmentation simplifies updatability), but also
> to benefit upstream by forcing all vendors to agree on one common
> kernel, that we push hard to be aligned with mainline.
> 
> One challenge to implement GKI is to avoid bloating the kernel by
> compiling too many things in, especially given that different devices
> need different things. As such, anything that can be turned into a
> module helps GKI, by offering an alternative to having that component
> built-in. This is true for pretty much anything that can be made
> modular, including drivers as well as other kernel components, such as
> CPUFreq governors.
> 
> Indeed, in practice, Android devices often ship with only one CPUFreq
> governor enabled, and don't require any other that would simply waste
> memory for no benefits. All CPUFreq governors can already be built as
> modules with one notable exception: schedutil. Though popular in
> Android, some devices do not use schedutil, which is why it would be
> preferable to not have it unconditionally built in GKI. This series is
> an attempt to solve this problem, by making schedutil tristate.
> 
> While modularization is usually not something we want to see near the
> scheduler, it appeared to me as I wrote those patches that the
> particular case of schedutil was actually not too bad to implement.
> We already have to support switching governors at run-time, simply
> because userspace is free to do that, so the infrastructure for turning
> sugov on and off dynamically is already there. Loading the code a little
> later doesn't seem to make that a lot worse.
> 
> Patches 01-05 refactor some code to break the few dependencies on
> schedutil being builtin (notably EAS). Patches 06-12 export various
> symbols that schedutil needs when compiled as a module. And finally,
> patches 13-14 finish off the work by making the Kconfig tristate.

IMHO, you have over-broken the patches, like first two could be merged
together and all exports could have been done in a single patch, etc.
i.e. all related or similar changes together...

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Peter Zijlstra May 8, 2020, 8:11 a.m. UTC | #3
On Thu, May 07, 2020 at 07:09:58PM +0100, Quentin Perret wrote:
> One challenge to implement GKI is to avoid bloating the kernel by
> compiling too many things in, especially given that different devices
> need different things. As such, anything that can be turned into a
> module helps GKI, by offering an alternative to having that component
> built-in. This is true for pretty much anything that can be made
> modular, including drivers as well as other kernel components, such as
> CPUFreq governors.

The idea is to move to 1 governor, schedutil. Also, I abhor all the
exports this thing does. Modules have no business touching most of that.

Look at every EXPORT you do and wonder ask yourself how you can abuse
it. Modules are not a good thing, they're horrible pieces of crap.
Greg KH May 8, 2020, 10:37 a.m. UTC | #4
On Fri, May 08, 2020 at 10:11:28AM +0200, Peter Zijlstra wrote:
> On Thu, May 07, 2020 at 07:09:58PM +0100, Quentin Perret wrote:
> > One challenge to implement GKI is to avoid bloating the kernel by
> > compiling too many things in, especially given that different devices
> > need different things. As such, anything that can be turned into a
> > module helps GKI, by offering an alternative to having that component
> > built-in. This is true for pretty much anything that can be made
> > modular, including drivers as well as other kernel components, such as
> > CPUFreq governors.
> 
> The idea is to move to 1 governor, schedutil. Also, I abhor all the
> exports this thing does. Modules have no business touching most of that.
> 
> Look at every EXPORT you do and wonder ask yourself how you can abuse
> it. Modules are not a good thing, they're horrible pieces of crap.

Quentin, what is missing from schedutil that warrants the need for a
totally different governor?  Is there specific problems with the
existing ones or is this just an instance of "we wrote our own a long
time ago and never pushed it upstream" from various SoC companies?

thanks,

greg k-h
Quentin Perret May 8, 2020, 11:16 a.m. UTC | #5
On Friday 08 May 2020 at 12:37:21 (+0200), Greg KH wrote:
> On Fri, May 08, 2020 at 10:11:28AM +0200, Peter Zijlstra wrote:
> > On Thu, May 07, 2020 at 07:09:58PM +0100, Quentin Perret wrote:
> > > One challenge to implement GKI is to avoid bloating the kernel by
> > > compiling too many things in, especially given that different devices
> > > need different things. As such, anything that can be turned into a
> > > module helps GKI, by offering an alternative to having that component
> > > built-in. This is true for pretty much anything that can be made
> > > modular, including drivers as well as other kernel components, such as
> > > CPUFreq governors.
> > 
> > The idea is to move to 1 governor, schedutil. Also, I abhor all the
> > exports this thing does. Modules have no business touching most of that.
> > 
> > Look at every EXPORT you do and wonder ask yourself how you can abuse
> > it. Modules are not a good thing, they're horrible pieces of crap.
> 
> Quentin, what is missing from schedutil that warrants the need for a
> totally different governor?  Is there specific problems with the
> existing ones or is this just an instance of "we wrote our own a long
> time ago and never pushed it upstream" from various SoC companies?

In all honesty, this is probably a mix. Some vendors definitely have
their out of tree crap that they'll load anyways, and some others use
other mainline governors for semi-reasonable reasons (e.g, for lower
tier devices, they often don't want to go though the costly and tedious
process of tuning uclamp, so they'll go for a more 'aggressive' governor
like ondemand). Note that this is the minority, though. The majority in
Android use schedutil, and I'm quite happy with that.

However, the point I tried to make here is orthogonal to that. As of
today using another governor than schedutil is fully supported upstream,
and in fact it isn't even enabled by default for most archs. If vendors
feel like using something else makes their product better, then I don't
see why I need to argue with them about that. And frankly I don't see
that support being removed from upstream any time soon.

Don't get me wrong, I do think schedutil is the way to go. And we do
recommend it in Android. I'm simply saying that /mandating/ it in GKI
would only add more friction than really necessary. Making it modular,
OTOH, might ease things a bit.

I guess it all boils down to the code. If modularizing schedutil really
is too ugly, then I'll drop the series and we'll just build it in and
waste memory on devices that don't use it. It's not the end of the
world, but it's a shame if we don't have strong technical reasons to do
that -- all other governors _are_ modular.

IMO the changes in this series aren't *too* bad -- moving
schedutil_cpu_util to fair.c is probably a sensible change in its own
right for instance. The biggest 'issue' is probably the exports, but
these will need discussions on a case by case basis. And I'm happy to
try and rework the code to work around when possible (as is the case for
runqueues for instance).

I hope that helps!

Thanks,
Quentin
Peter Zijlstra May 8, 2020, 11:26 a.m. UTC | #6
On Fri, May 08, 2020 at 12:37:21PM +0200, Greg KH wrote:
> On Fri, May 08, 2020 at 10:11:28AM +0200, Peter Zijlstra wrote:
> > On Thu, May 07, 2020 at 07:09:58PM +0100, Quentin Perret wrote:
> > > One challenge to implement GKI is to avoid bloating the kernel by
> > > compiling too many things in, especially given that different devices
> > > need different things. As such, anything that can be turned into a
> > > module helps GKI, by offering an alternative to having that component
> > > built-in. This is true for pretty much anything that can be made
> > > modular, including drivers as well as other kernel components, such as
> > > CPUFreq governors.
> > 
> > The idea is to move to 1 governor, schedutil. Also, I abhor all the
> > exports this thing does. Modules have no business touching most of that.
> > 
> > Look at every EXPORT you do and wonder ask yourself how you can abuse
> > it. Modules are not a good thing, they're horrible pieces of crap.
> 
> Quentin, what is missing from schedutil that warrants the need for a
> totally different governor?  Is there specific problems with the
> existing ones or is this just an instance of "we wrote our own a long
> time ago and never pushed it upstream" from various SoC companies?

At the very least there's that interactive governor that's really
popular with Android. But IIRC there's a whole scala of home-brew
governors and tweaks out there.

And instead of enabling that crap, we should be discouraging it.
Consolidate and kill the governor interface.
Peter Zijlstra May 8, 2020, 11:31 a.m. UTC | #7
On Fri, May 08, 2020 at 12:16:12PM +0100, Quentin Perret wrote:
> However, the point I tried to make here is orthogonal to that. As of
> today using another governor than schedutil is fully supported upstream,
> and in fact it isn't even enabled by default for most archs. If vendors
> feel like using something else makes their product better, then I don't
> see why I need to argue with them about that. And frankly I don't see
> that support being removed from upstream any time soon.

Right, it'll take a while to get there. But that doesn't mean we
shouldn't encourage schedutil usage wherever and whenever possible. That
includes not making it easier to not use it.

In that respect making it modular goes against our ultimate goal (world
domination, <mad giggles here>).
Quentin Perret May 8, 2020, 1:05 p.m. UTC | #8
On Friday 08 May 2020 at 13:31:41 (+0200), Peter Zijlstra wrote:
> On Fri, May 08, 2020 at 12:16:12PM +0100, Quentin Perret wrote:
> > However, the point I tried to make here is orthogonal to that. As of
> > today using another governor than schedutil is fully supported upstream,
> > and in fact it isn't even enabled by default for most archs. If vendors
> > feel like using something else makes their product better, then I don't
> > see why I need to argue with them about that. And frankly I don't see
> > that support being removed from upstream any time soon.
> 
> Right, it'll take a while to get there. But that doesn't mean we
> shouldn't encourage schedutil usage wherever and whenever possible. That
> includes not making it easier to not use it.
> 
> In that respect making it modular goes against our ultimate goal (world
> domination, <mad giggles here>).

Right, I definitely understand the sentiment. OTOH, things like that
give vendors weapons against GKI ('you-force-us-to-build-in-things-we-dont't-want'
etc etc). That _is_ true to some extent, but it's important we make sure
to keep this to an absolute minimum, otherwise GKI just won't happen
(and I really think that'd be a shame, GKI _is_ a good thing for
upstream).

And sure, schedutil isn't that big, and we can make an exception. But
I'm sure you know what happens when you starting making exceptions ;)

So, all in all, I don't think the series actively makes schedutil worse
by adding out-of-line calls in the hot path or anything like that, and
having it as a module helps with GKI which I'm arguing is a good thing
in the grand scheme of things. That of course is only true if we can
agree on a reasonable set of exported symbols, so I'll give others some
time to complain and see if I can post a v2 addressing these issues!

Cheers,
Quentin
Quentin Perret May 8, 2020, 1:15 p.m. UTC | #9
Hey Valentin,

On Thursday 07 May 2020 at 22:34:17 (+0100), Valentin Schneider wrote:
> I'm curious; why would some Android device not want to roll with schedutil?
> 
> When it comes to dynamic policies (i.e. forget performance / powersave, and
> put userspace in a corner), I'd be willing to take a stand and say you
> should only really be using schedutil nowadays - alignment with the
> scheduler, uclamp, yadda yadda.
> 
> AFAIA the only schedutil-related quirk we oughta fix for arm/arm64 is that
> arch_scale_freq_invariant() thingie, and FWIW I'm hoping to get something
> regarding this out sometime soonish. After that, I'd actually want to make
> schedutil the default governor for arm/arm64.

As in setting CONFIG_CPU_FREQ_DEFAULT_GOV_SCHEDUTIL=y in the arm64
defconfig? If so, you have my Acked-by already :)

> I'm not opiniated on the modularization, but if you can, could you please
> share some more details as to why schedutil cannot fulfill its role of holy
> messiah of governors for GKI?

I guess I answered some of that in the other thread with Peter, but all
in all I'm definitely not trying to make an argument that schedutil
isn't good enough here. I'm trying to say that mandating it in *GKI* is
just likely to cause unnecessary friction, and trust me there is already
enough of that with other topics. Giving the option of having sugov as a
module doesn't prevent us from making it a default for a few arches, so
I think there is ground for an agreement!

Cheers,
Quentin
Quentin Perret May 8, 2020, 1:18 p.m. UTC | #10
Hi Viresh,

On Friday 08 May 2020 at 11:03:59 (+0530), Viresh Kumar wrote:
> IMHO, you have over-broken the patches, like first two could be merged
> together and all exports could have been done in a single patch, etc.
> i.e. all related or similar changes together...

Right, I don't mind squashing the first patches. For the exports, I'm
guessing they'll need a case by case discussion, so it's probably
reasonable to keep them separate, at least for now.

> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Thanks!
Quentin
Rafael J. Wysocki May 8, 2020, 1:40 p.m. UTC | #11
On Fri, May 8, 2020 at 3:05 PM Quentin Perret <qperret@google.com> wrote:
>
> On Friday 08 May 2020 at 13:31:41 (+0200), Peter Zijlstra wrote:
> > On Fri, May 08, 2020 at 12:16:12PM +0100, Quentin Perret wrote:
> > > However, the point I tried to make here is orthogonal to that. As of
> > > today using another governor than schedutil is fully supported upstream,
> > > and in fact it isn't even enabled by default for most archs. If vendors
> > > feel like using something else makes their product better, then I don't
> > > see why I need to argue with them about that. And frankly I don't see
> > > that support being removed from upstream any time soon.
> >
> > Right, it'll take a while to get there. But that doesn't mean we
> > shouldn't encourage schedutil usage wherever and whenever possible. That
> > includes not making it easier to not use it.
> >
> > In that respect making it modular goes against our ultimate goal (world
> > domination, <mad giggles here>).
>
> Right, I definitely understand the sentiment. OTOH, things like that
> give vendors weapons against GKI ('you-force-us-to-build-in-things-we-dont't-want'
> etc etc). That _is_ true to some extent, but it's important we make sure
> to keep this to an absolute minimum, otherwise GKI just won't happen
> (and I really think that'd be a shame, GKI _is_ a good thing for
> upstream).
>
> And sure, schedutil isn't that big, and we can make an exception. But
> I'm sure you know what happens when you starting making exceptions ;)

This is a very weak argument, if it can be regarded as an argument at all.

You will have to make exceptions, the question is how many and on what
criteria and you really need to figure that out for the GKI plan.

> So, all in all, I don't think the series actively makes schedutil worse
> by adding out-of-line calls in the hot path or anything like that, and
> having it as a module helps with GKI which I'm arguing is a good thing
> in the grand scheme of things.

Frankly, I'm not sure if it really helps.

The idea of making schedutil modular seems to be based on the
observation that it is not part of the core kernel, which I don't
agree with.  Arguably, things like util clamps need it to work as
expected.

> That of course is only true if we can
> agree on a reasonable set of exported symbols, so I'll give others some
> time to complain and see if I can post a v2 addressing these issues!

This isn't just about exported symbols, it is about what is regarded
as essential and what isn't.

Cheers!
Peter Zijlstra May 8, 2020, 2:09 p.m. UTC | #12
On Fri, May 08, 2020 at 02:05:07PM +0100, Quentin Perret wrote:
> So, all in all, I don't think the series actively makes schedutil worse
> by adding out-of-line calls in the hot path or anything like that, and

Do note that (afaik) ARM64 (and Power and probably others) has modules
in an address space that is not reachable from regular kernel text and
needs those intermediate trampolines.

While that's probably not a very big deal, it does increase L1$ pressure
and things.
Valentin Schneider May 8, 2020, 2:52 p.m. UTC | #13
On 08/05/20 14:15, Quentin Perret wrote:
> Hey Valentin,
>
> On Thursday 07 May 2020 at 22:34:17 (+0100), Valentin Schneider wrote:
>> I'm curious; why would some Android device not want to roll with schedutil?
>>
>> When it comes to dynamic policies (i.e. forget performance / powersave, and
>> put userspace in a corner), I'd be willing to take a stand and say you
>> should only really be using schedutil nowadays - alignment with the
>> scheduler, uclamp, yadda yadda.
>>
>> AFAIA the only schedutil-related quirk we oughta fix for arm/arm64 is that
>> arch_scale_freq_invariant() thingie, and FWIW I'm hoping to get something
>> regarding this out sometime soonish. After that, I'd actually want to make
>> schedutil the default governor for arm/arm64.
>
> As in setting CONFIG_CPU_FREQ_DEFAULT_GOV_SCHEDUTIL=y in the arm64
> defconfig? If so, you have my Acked-by already :)
>

I'm actually thinking of making it the unconditional default for arm/arm64
in cpufreq's Kconfig, following what has been recently done for
intel_pstate.

>> I'm not opiniated on the modularization, but if you can, could you please
>> share some more details as to why schedutil cannot fulfill its role of holy
>> messiah of governors for GKI?
>
> I guess I answered some of that in the other thread with Peter, but all
> in all I'm definitely not trying to make an argument that schedutil
> isn't good enough here. I'm trying to say that mandating it in *GKI* is
> just likely to cause unnecessary friction, and trust me there is already
> enough of that with other topics.

Right, I appreciate it must be an "interesting" tug of war. My own opinion
has also already been expanded in the rest of the thread; i.e. we should
strive to make schedutil good enough that folks don't feel like they still
need to use ondemand/whatever frankengov. That said, even without GKI, I
get that making some vendors change their already tested-and-tuned setup is
an obstacle course in and of itself.

> Giving the option of having sugov as a
> module doesn't prevent us from making it a default for a few arches, so
> I think there is ground for an agreement!
>
> Cheers,
> Quentin
Viresh Kumar May 11, 2020, 5:21 a.m. UTC | #14
On 08-05-20, 13:26, Peter Zijlstra wrote:
> At the very least there's that interactive governor that's really
> popular with Android. But IIRC there's a whole scala of home-brew
> governors and tweaks out there.

I removed interactive governor from Android long time back :)
Quentin Perret May 11, 2020, 9 a.m. UTC | #15
Hi Rafael,

On Friday 08 May 2020 at 15:40:34 (+0200), Rafael J. Wysocki wrote:
> On Fri, May 8, 2020 at 3:05 PM Quentin Perret <qperret@google.com> wrote:
> >
> > On Friday 08 May 2020 at 13:31:41 (+0200), Peter Zijlstra wrote:
> > > On Fri, May 08, 2020 at 12:16:12PM +0100, Quentin Perret wrote:
> > > > However, the point I tried to make here is orthogonal to that. As of
> > > > today using another governor than schedutil is fully supported upstream,
> > > > and in fact it isn't even enabled by default for most archs. If vendors
> > > > feel like using something else makes their product better, then I don't
> > > > see why I need to argue with them about that. And frankly I don't see
> > > > that support being removed from upstream any time soon.
> > >
> > > Right, it'll take a while to get there. But that doesn't mean we
> > > shouldn't encourage schedutil usage wherever and whenever possible. That
> > > includes not making it easier to not use it.
> > >
> > > In that respect making it modular goes against our ultimate goal (world
> > > domination, <mad giggles here>).
> >
> > Right, I definitely understand the sentiment. OTOH, things like that
> > give vendors weapons against GKI ('you-force-us-to-build-in-things-we-dont't-want'
> > etc etc). That _is_ true to some extent, but it's important we make sure
> > to keep this to an absolute minimum, otherwise GKI just won't happen
> > (and I really think that'd be a shame, GKI _is_ a good thing for
> > upstream).
> >
> > And sure, schedutil isn't that big, and we can make an exception. But
> > I'm sure you know what happens when you starting making exceptions ;)
> 
> This is a very weak argument, if it can be regarded as an argument at all.

Well, fair enough :)

> You will have to make exceptions, the question is how many and on what
> criteria and you really need to figure that out for the GKI plan.

The base idea is, anything that we know from experience is used by
everybody can be built in, anything else will need investigation. And as
you've understood, schedutil falls in that second category.

> > So, all in all, I don't think the series actively makes schedutil worse
> > by adding out-of-line calls in the hot path or anything like that, and
> > having it as a module helps with GKI which I'm arguing is a good thing
> > in the grand scheme of things.
> 
> Frankly, I'm not sure if it really helps.

Oh, why not?

> The idea of making schedutil modular seems to be based on the
> observation that it is not part of the core kernel, which I don't
> agree with.

Right, so that I think is the core of the discussion.

> Arguably, things like util clamps need it to work as
> expected.

Sure, but loading sugov dynamically as a module doesn't change much does
it?

If you are referring to the Kconfig dependency of uclamp on schedutil,
then that is a good point and I will argue that it should be removed.
In fact I'll add a patch to v2 that does just that, with the following
rationale:
 - it is obsolete: the reason that dependency was added originally was
   because sugov was the only place where util clamps where taken into
   accounts. But that is no longer true -- we check them in the capacity
   aware wake-up path as well, which is entirely independent from the
   currently running governor;
 - because of the above, it is (now) largely useless: a compile time
   dependency doesn't guarantee we are actually running with schedutil
   at all;
 - it is artificial: there are no actual compilation dependencies
   between sugov and uclamp, everything will compile just fine without
   that 'depends on';

Or maybe you were thinking of something else?

> > That of course is only true if we can
> > agree on a reasonable set of exported symbols, so I'll give others some
> > time to complain and see if I can post a v2 addressing these issues!
> 
> This isn't just about exported symbols, it is about what is regarded
> as essential and what isn't.

Right, the exported symbols are, IMO, quite interesting because they
show how 'core' the governor is. But what exactly do you mean by
'essential' here? Essential in what sense?

Thanks,
Quentin
Quentin Perret May 11, 2020, 9:12 a.m. UTC | #16
Hi Peter,

On Friday 08 May 2020 at 16:09:38 (+0200), Peter Zijlstra wrote:
> Do note that (afaik) ARM64 (and Power and probably others) has modules
> in an address space that is not reachable from regular kernel text and
> needs those intermediate trampolines.

Right, but I meant this series in its current form doesn't actively make
things worse for existing users. That is, the act of modularizing
schedutil has no impact on those who compile it in (because I agree that
would be a solid reason for refusing that series), and they are still
perfectly free to do so.

The 'new' users who deliberately decide to compile it as a module will
have to pay that price, but that is their choice. And in the Android
case, as you've understood, this is a price we are ready to pay.

Thanks,
Quentin
Rafael J. Wysocki May 11, 2020, 3:26 p.m. UTC | #17
On Mon, May 11, 2020 at 11:00 AM Quentin Perret <qperret@google.com> wrote:
>
> Hi Rafael,
>
> On Friday 08 May 2020 at 15:40:34 (+0200), Rafael J. Wysocki wrote:
> > On Fri, May 8, 2020 at 3:05 PM Quentin Perret <qperret@google.com> wrote:
> > >
> > > On Friday 08 May 2020 at 13:31:41 (+0200), Peter Zijlstra wrote:
> > > > On Fri, May 08, 2020 at 12:16:12PM +0100, Quentin Perret wrote:
> > > > > However, the point I tried to make here is orthogonal to that. As of
> > > > > today using another governor than schedutil is fully supported upstream,
> > > > > and in fact it isn't even enabled by default for most archs. If vendors
> > > > > feel like using something else makes their product better, then I don't
> > > > > see why I need to argue with them about that. And frankly I don't see
> > > > > that support being removed from upstream any time soon.
> > > >
> > > > Right, it'll take a while to get there. But that doesn't mean we
> > > > shouldn't encourage schedutil usage wherever and whenever possible. That
> > > > includes not making it easier to not use it.
> > > >
> > > > In that respect making it modular goes against our ultimate goal (world
> > > > domination, <mad giggles here>).
> > >
> > > Right, I definitely understand the sentiment. OTOH, things like that
> > > give vendors weapons against GKI ('you-force-us-to-build-in-things-we-dont't-want'
> > > etc etc). That _is_ true to some extent, but it's important we make sure
> > > to keep this to an absolute minimum, otherwise GKI just won't happen
> > > (and I really think that'd be a shame, GKI _is_ a good thing for
> > > upstream).
> > >
> > > And sure, schedutil isn't that big, and we can make an exception. But
> > > I'm sure you know what happens when you starting making exceptions ;)
> >
> > This is a very weak argument, if it can be regarded as an argument at all.
>
> Well, fair enough :)
>
> > You will have to make exceptions, the question is how many and on what
> > criteria and you really need to figure that out for the GKI plan.
>
> The base idea is, anything that we know from experience is used by
> everybody can be built in, anything else will need investigation. And as
> you've understood, schedutil falls in that second category.

The fact that the vendor sets up a different governor by default
doesn't mean that there should be no way to switch over to schedutil
IMO.

> > > So, all in all, I don't think the series actively makes schedutil worse
> > > by adding out-of-line calls in the hot path or anything like that, and
> > > having it as a module helps with GKI which I'm arguing is a good thing
> > > in the grand scheme of things.
> >
> > Frankly, I'm not sure if it really helps.
>
> Oh, why not?
>
> > The idea of making schedutil modular seems to be based on the
> > observation that it is not part of the core kernel, which I don't
> > agree with.
>
> Right, so that I think is the core of the discussion.
>
> > Arguably, things like util clamps need it to work as
> > expected.
>
> Sure, but loading sugov dynamically as a module doesn't change much does
> it?
>
> If you are referring to the Kconfig dependency of uclamp on schedutil,
> then that is a good point and I will argue that it should be removed.
> In fact I'll add a patch to v2 that does just that, with the following
> rationale:

Which isn't correct AFAICS.

>  - it is obsolete:

No, it isn't IMO.

> the reason that dependency was added originally was
>    because sugov was the only place where util clamps where taken into
>    accounts. But that is no longer true -- we check them in the capacity
>    aware wake-up path as well, which is entirely independent from the
>    currently running governor;

But this is done under the assumption that the governor will also take
the clamps into account, isn't it?

Otherwise you can see your "low util" tasks running at high
frequencies and "high util" ones running slowly.  Surely, that's not
desirable?

IIUC, the task placement needs to be consistent with the governor's
decisions for things to work as expected.

>  - because of the above, it is (now) largely useless: a compile time
>    dependency doesn't guarantee we are actually running with schedutil
>    at all;
>  - it is artificial: there are no actual compilation dependencies
>    between sugov and uclamp, everything will compile just fine without
>    that 'depends on';

That actually is the case, but it doesn't mean that there is no
dependency in there.

> Or maybe you were thinking of something else?
>
> > > That of course is only true if we can
> > > agree on a reasonable set of exported symbols, so I'll give others some
> > > time to complain and see if I can post a v2 addressing these issues!
> >
> > This isn't just about exported symbols, it is about what is regarded
> > as essential and what isn't.
>
> Right, the exported symbols are, IMO, quite interesting because they
> show how 'core' the governor is. But what exactly do you mean by
> 'essential' here? Essential in what sense?

IMO the question is how much value there is in making it possible to
avoid loading a particular piece of kernel code into memory.

You've demonstrated that it can be done with schedutil, but does that
matter that it needs to be done?

I thought that the original idea was to make it closely integrated
with the scheduler, so it could access the scheduler's data structures
(that we specifically didn't want to expose to the *other* governors)
and so as to avoid forgetting about any dependencies when making
changes to either the scheduler or schedutil.  Allowing it to be build
as a module would make make us have to worry about those things again,
so is it really worth it?
Quentin Perret May 12, 2020, 9:21 a.m. UTC | #18
Hi Rafael,

On Monday 11 May 2020 at 17:26:26 (+0200), Rafael J. Wysocki wrote:
> On Mon, May 11, 2020 at 11:00 AM Quentin Perret <qperret@google.com> wrote:
> > The base idea is, anything that we know from experience is used by
> > everybody can be built in, anything else will need investigation. And as
> > you've understood, schedutil falls in that second category.
> 
> The fact that the vendor sets up a different governor by default
> doesn't mean that there should be no way to switch over to schedutil
> IMO.

Well, there will always be the option to load the schedutil module ;-)

<snip>
> > the reason that dependency was added originally was
> >    because sugov was the only place where util clamps where taken into
> >    accounts. But that is no longer true -- we check them in the capacity
> >    aware wake-up path as well, which is entirely independent from the
> >    currently running governor;
> 
> But this is done under the assumption that the governor will also take
> the clamps into account, isn't it?

Even if that was correct, it's not clear a compile-time dependency makes
that assumption true, right?

For governors and the like, if the option is =n, then you can hard-rely
on it not being used. But if it is =y, you cannot assume anything
what-so-ever. EAS does a run-time check for that exact reason -- a
sole Kconfig dependency typically doesn't work for that.

> Otherwise you can see your "low util" tasks running at high
> frequencies and "high util" ones running slowly.  Surely, that's not
> desirable?
> 
> IIUC, the task placement needs to be consistent with the governor's
> decisions for things to work as expected.

Sure, but, say, the 'performance' governor could give you some of that
too. That is, you could use uclamp.min on some tasks to ensure they are
biased to bigger CPUs, and just stick the frequency to max. I wouldn't
be surprised to see setups like that on non-battery-powered devices for
instance. And yes, there are non-battery-powered devices that use big
little out there (TVs and such, often because the only SOCs matching
their requirements are mobile SOCs).

> >  - because of the above, it is (now) largely useless: a compile time
> >    dependency doesn't guarantee we are actually running with schedutil
> >    at all;
> >  - it is artificial: there are no actual compilation dependencies
> >    between sugov and uclamp, everything will compile just fine without
> >    that 'depends on';
> 
> That actually is the case, but it doesn't mean that there is no
> dependency in there.

Sure, and the dependency did make sense when uclamp was first introduced.
At the time, the clamp values where used _only_ in schedutil. So, it
was fair to say "if schedutil is =n, there is no way the clamps will ever
be useful to anything else, so the uclamp code can be safely compiled
out". That is no longer true, and if you want to make uclamp work only
with schedutil (which I would advise against for the above reason), then
a Kconfig dependency doesn't seem to be the right tool for that anyway.

> > Or maybe you were thinking of something else?
> >
> > > > That of course is only true if we can
> > > > agree on a reasonable set of exported symbols, so I'll give others some
> > > > time to complain and see if I can post a v2 addressing these issues!
> > >
> > > This isn't just about exported symbols, it is about what is regarded
> > > as essential and what isn't.
> >
> > Right, the exported symbols are, IMO, quite interesting because they
> > show how 'core' the governor is. But what exactly do you mean by
> > 'essential' here? Essential in what sense?
> 
> IMO the question is how much value there is in making it possible to
> avoid loading a particular piece of kernel code into memory.
> 
> You've demonstrated that it can be done with schedutil, but does that
> matter that it needs to be done?
> 
> I thought that the original idea was to make it closely integrated
> with the scheduler, so it could access the scheduler's data structures
> (that we specifically didn't want to expose to the *other* governors)
> and so as to avoid forgetting about any dependencies when making
> changes to either the scheduler or schedutil.  Allowing it to be build
> as a module would make make us have to worry about those things again,
> so is it really worth it?

Right, so, if there is a strong technical reason to keep schedutil a
bool option (such as accessing data structures we really don't want to
export), then sure, I'll have no choice but to accept it. Now, assuming
that I fix the usage of 'runqueues', is there anything in particular
that you think is wrong in the series?

Note that if one day keeping schedutil modular becomes a blocker for a
new feature, then we'll have the option to make it bool again. But is
there something like that already?

Thanks,
Quentin
Rafael J. Wysocki May 12, 2020, 10:25 a.m. UTC | #19
On Tue, May 12, 2020 at 11:21 AM Quentin Perret <qperret@google.com> wrote:
>
> Hi Rafael,
>
> On Monday 11 May 2020 at 17:26:26 (+0200), Rafael J. Wysocki wrote:
> > On Mon, May 11, 2020 at 11:00 AM Quentin Perret <qperret@google.com> wrote:
> > > The base idea is, anything that we know from experience is used by
> > > everybody can be built in, anything else will need investigation. And as
> > > you've understood, schedutil falls in that second category.
> >
> > The fact that the vendor sets up a different governor by default
> > doesn't mean that there should be no way to switch over to schedutil
> > IMO.
>
> Well, there will always be the option to load the schedutil module ;-)

Yeah, fair enough.

> <snip>
> > > the reason that dependency was added originally was
> > >    because sugov was the only place where util clamps where taken into
> > >    accounts. But that is no longer true -- we check them in the capacity
> > >    aware wake-up path as well, which is entirely independent from the
> > >    currently running governor;
> >
> > But this is done under the assumption that the governor will also take
> > the clamps into account, isn't it?
>
> Even if that was correct, it's not clear a compile-time dependency makes
> that assumption true, right?

It indicates that the functional dependency is there if you will.

Otherwise it would be kind of hidden and likely to become confusing.

> For governors and the like, if the option is =n, then you can hard-rely
> on it not being used. But if it is =y, you cannot assume anything
> what-so-ever. EAS does a run-time check for that exact reason -- a
> sole Kconfig dependency typically doesn't work for that.

Obviously Kconfig dependencies cannot replace runtime checks, but OTOH
the latter are guaranteed to fail if the given piece of code is not
there in the kernel even.

> > Otherwise you can see your "low util" tasks running at high
> > frequencies and "high util" ones running slowly.  Surely, that's not
> > desirable?
> >
> > IIUC, the task placement needs to be consistent with the governor's
> > decisions for things to work as expected.
>
> Sure, but, say, the 'performance' governor could give you some of that too.

Well, kind of, and the 'powersave' governor can do that too in theory.

> That is, you could use uclamp.min on some tasks to ensure they are
> biased to bigger CPUs, and just stick the frequency to max. I wouldn't
> be surprised to see setups like that on non-battery-powered devices for
> instance. And yes, there are non-battery-powered devices that use big
> little out there (TVs and such, often because the only SOCs matching
> their requirements are mobile SOCs).

I would not conflate the use cases for uclamps and big-little.

Anyway, there are some cases in which using uclamps without schedutil
might make theoretical sense, but I'm not sure how useful that would
be in practice.

> > >  - because of the above, it is (now) largely useless: a compile time
> > >    dependency doesn't guarantee we are actually running with schedutil
> > >    at all;
> > >  - it is artificial: there are no actual compilation dependencies
> > >    between sugov and uclamp, everything will compile just fine without
> > >    that 'depends on';
> >
> > That actually is the case, but it doesn't mean that there is no
> > dependency in there.
>
> Sure, and the dependency did make sense when uclamp was first introduced.
> At the time, the clamp values where used _only_ in schedutil. So, it
> was fair to say "if schedutil is =n, there is no way the clamps will ever
> be useful to anything else, so the uclamp code can be safely compiled
> out". That is no longer true, and if you want to make uclamp work only
> with schedutil (which I would advise against for the above reason), then
> a Kconfig dependency doesn't seem to be the right tool for that anyway.

Still, IMO it would be fair to say that if uclamps are used, schedutil
is very likely to be preferred.

Kconfig can be made select schedutil when enabling uclamps or similar
to express that preference.

> > > Or maybe you were thinking of something else?
> > >
> > > > > That of course is only true if we can
> > > > > agree on a reasonable set of exported symbols, so I'll give others some
> > > > > time to complain and see if I can post a v2 addressing these issues!
> > > >
> > > > This isn't just about exported symbols, it is about what is regarded
> > > > as essential and what isn't.
> > >
> > > Right, the exported symbols are, IMO, quite interesting because they
> > > show how 'core' the governor is. But what exactly do you mean by
> > > 'essential' here? Essential in what sense?
> >
> > IMO the question is how much value there is in making it possible to
> > avoid loading a particular piece of kernel code into memory.
> >
> > You've demonstrated that it can be done with schedutil, but does that
> > matter that it needs to be done?
> >
> > I thought that the original idea was to make it closely integrated
> > with the scheduler, so it could access the scheduler's data structures
> > (that we specifically didn't want to expose to the *other* governors)
> > and so as to avoid forgetting about any dependencies when making
> > changes to either the scheduler or schedutil.  Allowing it to be build
> > as a module would make make us have to worry about those things again,
> > so is it really worth it?
>
> Right, so, if there is a strong technical reason to keep schedutil a
> bool option (such as accessing data structures we really don't want to
> export),

The bool option is the status quo and there needs to be a strong
technical reason to allow it to be modular (as that would cause the
complexity to increase).

> then sure, I'll have no choice but to accept it. Now, assuming
> that I fix the usage of 'runqueues', is there anything in particular
> that you think is wrong in the series?

Well, define "wrong". :-)

Some pieces of the series look like general improvements, but some of
them don't.

And the fact that something can be done alone should not be regarded
as a good enough reason for doing it in my view.

> Note that if one day keeping schedutil modular becomes a blocker for a
> new feature, then we'll have the option to make it bool again. But is
> there something like that already?

Putting it this way isn't entirely fair IMO.

What you are proposing is basically to add complexity and the reason
for doing that seems to be convenience (and that's not the users'
convenience for that matter) which is not really super-convincing.

Cheers!
Quentin Perret May 12, 2020, 1:58 p.m. UTC | #20
On Tuesday 12 May 2020 at 12:25:17 (+0200), Rafael J. Wysocki wrote:
> Still, IMO it would be fair to say that if uclamps are used, schedutil
> is very likely to be preferred.
> 
> Kconfig can be made select schedutil when enabling uclamps or similar
> to express that preference.

Right, fair enough. Making schedutil default to y when uclamp is
compiled in should do the trick (and avoid using 'select'). Would that
work for you?

> What you are proposing is basically to add complexity and the reason
> for doing that seems to be convenience (and that's not the users'
> convenience for that matter) which is not really super-convincing.

Forcing our users to build in their products something they don't want
to use tends to be a very real problem for what we're trying to achieve,
so it's certainly not just convenience from our perspective. I can
understand that yours might be different, though.

Thanks,
Quentin
Rafael J. Wysocki May 12, 2020, 2:08 p.m. UTC | #21
On Tue, May 12, 2020 at 3:58 PM Quentin Perret <qperret@google.com> wrote:
>
> On Tuesday 12 May 2020 at 12:25:17 (+0200), Rafael J. Wysocki wrote:
> > Still, IMO it would be fair to say that if uclamps are used, schedutil
> > is very likely to be preferred.
> >
> > Kconfig can be made select schedutil when enabling uclamps or similar
> > to express that preference.
>
> Right, fair enough. Making schedutil default to y when uclamp is
> compiled in should do the trick (and avoid using 'select'). Would that
> work for you?

I think so.

> > What you are proposing is basically to add complexity and the reason
> > for doing that seems to be convenience (and that's not the users'
> > convenience for that matter) which is not really super-convincing.
>
> Forcing our users to build in their products something they don't want
> to use tends to be a very real problem for what we're trying to achieve,
> so it's certainly not just convenience from our perspective. I can
> understand that yours might be different, though.

I would like to understand the nature of the problem here.

If some piece of kernel code is modular, it still needs to be build.
The difference is when and how it gets loaded, so can you possibly
elaborate here?

Cheers!
Quentin Perret May 12, 2020, 3:11 p.m. UTC | #22
On Tuesday 12 May 2020 at 16:08:56 (+0200), Rafael J. Wysocki wrote:
> If some piece of kernel code is modular, it still needs to be build.
> The difference is when and how it gets loaded, so can you possibly
> elaborate here?

Sure thing, sorry if that wasn't clear.

The end goal with GKI is the following: Google will release a single
binary kernel image (signed, etc etc) that all devices using a given
Android version will be required to use. That image is however going to
be only for the core of the kernel (no drivers or anything of the sort).
Vendors and OEMs, on their end, will be responsible to build and ship
GKI-compatible modules for their respective devices. So, Android devices
will eventually ship with a Google-issued GKI, plus a bunch of
vendor-provided modules loaded during boot.

This is a significant shift from the current model where vendors
completely own the kernel, and are largely free to use the kernel config
they want. Today, those who don't use schedutil are free to turn the
config off, for example.

But GKI changes that. The 'core' GKI config is effectively imposed to
the entire ecosystem. As of now, because it is 'bool' we have no choice
but to compile schedutil in the core GKI as some (most) partners use it.
But as you can imagine, that is not the preferred option of those who
_don't_ use schedutil. Modularizing avoids any potential friction since
the vendors who want to use it will be able load the module, and the
others will simply not. That really is the reason for that series.

Then there is an important question: why should upstream care about all
that stuff? That's obviously debatable, but my biased opinion is that
GKI is a good thing(TM). It's our opportunity to put some order in the
android ecosystem and to reduce the delta with mainline. That'll
definitely take time, and there will be Android-specific churn in GKI in
the beginning, but we'd like to keep that as small as possible, and to
converge to 0 looking forwards.

I hope that helps!

Thanks,
Quentin
Rafael J. Wysocki May 12, 2020, 3:30 p.m. UTC | #23
On Tue, May 12, 2020 at 5:11 PM Quentin Perret <qperret@google.com> wrote:
>
> On Tuesday 12 May 2020 at 16:08:56 (+0200), Rafael J. Wysocki wrote:
> > If some piece of kernel code is modular, it still needs to be build.
> > The difference is when and how it gets loaded, so can you possibly
> > elaborate here?
>
> Sure thing, sorry if that wasn't clear.

No worries.

> The end goal with GKI is the following: Google will release a single
> binary kernel image (signed, etc etc) that all devices using a given
> Android version will be required to use. That image is however going to
> be only for the core of the kernel (no drivers or anything of the sort).
> Vendors and OEMs, on their end, will be responsible to build and ship
> GKI-compatible modules for their respective devices. So, Android devices
> will eventually ship with a Google-issued GKI, plus a bunch of
> vendor-provided modules loaded during boot.

If that is the case, then I absolutely think that schedutil should be
part of the GKI.

Moreover, that would have been my opinion even if it had been modular
in the first place.

> This is a significant shift from the current model where vendors
> completely own the kernel, and are largely free to use the kernel config
> they want. Today, those who don't use schedutil are free to turn the
> config off, for example.

So why is this regarded as a good thing?

> But GKI changes that. The 'core' GKI config is effectively imposed to
> the entire ecosystem. As of now, because it is 'bool' we have no choice
> but to compile schedutil in the core GKI as some (most) partners use it.
> But as you can imagine, that is not the preferred option of those who
> _don't_ use schedutil.

OTOH, it may as well be an incentive for them to switch over and
report problems with it that they see.

I absolutely would like to make schedutil the clearly preferred option
and IMO avoiding to use it, especially for non-technical reasons,
should be clearly less attractive.

> Modularizing avoids any potential friction since
> the vendors who want to use it will be able load the module, and the
> others will simply not. That really is the reason for that series.

If the long-term target is for everyone to use schedutil, then I don't
quite see why making it easy to not include it in one's system is
going to help.

> Then there is an important question: why should upstream care about all
> that stuff? That's obviously debatable, but my biased opinion is that
> GKI is a good thing(TM). It's our opportunity to put some order in the
> android ecosystem and to reduce the delta with mainline. That'll
> definitely take time, and there will be Android-specific churn in GKI in
> the beginning, but we'd like to keep that as small as possible, and to
> converge to 0 looking forwards.

That's a good goal, but I'm not sure if the least resistance path to
it is the right one. :-)

Cheers!
Joel Fernandes May 12, 2020, 3:49 p.m. UTC | #24
On Tue, May 12, 2020 at 11:30 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
[...]
> > The end goal with GKI is the following: Google will release a single
> > binary kernel image (signed, etc etc) that all devices using a given
> > Android version will be required to use. That image is however going to
> > be only for the core of the kernel (no drivers or anything of the sort).
> > Vendors and OEMs, on their end, will be responsible to build and ship
> > GKI-compatible modules for their respective devices. So, Android devices
> > will eventually ship with a Google-issued GKI, plus a bunch of
> > vendor-provided modules loaded during boot.
>
> If that is the case, then I absolutely think that schedutil should be
> part of the GKI.
>
> Moreover, that would have been my opinion even if it had been modular
> in the first place.
>
> > This is a significant shift from the current model where vendors
> > completely own the kernel, and are largely free to use the kernel config
> > they want. Today, those who don't use schedutil are free to turn the
> > config off, for example.
>
> So why is this regarded as a good thing?
>
> > But GKI changes that. The 'core' GKI config is effectively imposed to
> > the entire ecosystem. As of now, because it is 'bool' we have no choice
> > but to compile schedutil in the core GKI as some (most) partners use it.
> > But as you can imagine, that is not the preferred option of those who
> > _don't_ use schedutil.
>
> OTOH, it may as well be an incentive for them to switch over and
> report problems with it that they see.
>
> I absolutely would like to make schedutil the clearly preferred option
> and IMO avoiding to use it, especially for non-technical reasons,
> should be clearly less attractive.

Also, does this series make it easier for vendors / oems / whoever to
carry out-of-tree schedutil hacks saying that's "Ok" because that's
not part of the core GKI? That would definitely be a bad thing to
encourage as well. schedutil should pretty much be considered a part
of the core GKI if the goal is to encourage everyone to move to it,
IMO.

- Joel
Quentin Perret May 12, 2020, 4:26 p.m. UTC | #25
On Tuesday 12 May 2020 at 17:30:36 (+0200), Rafael J. Wysocki wrote:
> On Tue, May 12, 2020 at 5:11 PM Quentin Perret <qperret@google.com> wrote:
> > The end goal with GKI is the following: Google will release a single
> > binary kernel image (signed, etc etc) that all devices using a given
> > Android version will be required to use. That image is however going to
> > be only for the core of the kernel (no drivers or anything of the sort).
> > Vendors and OEMs, on their end, will be responsible to build and ship
> > GKI-compatible modules for their respective devices. So, Android devices
> > will eventually ship with a Google-issued GKI, plus a bunch of
> > vendor-provided modules loaded during boot.
> 
> If that is the case, then I absolutely think that schedutil should be
> part of the GKI.
> 
> Moreover, that would have been my opinion even if it had been modular
> in the first place.

I definitely understand the feeling. Heck I contributed to schedutil, so
I'd love to see the entire world run it :-)

But my personal preference doesn't seem to matter in this world, sadly.
The truth is, we cannot afford to be arbitrary in our decisions in GKI.
Switching governors and such is a fully supported feature upstream, and
it has been for a long time. Taking that away from partners is not the
goal, nor the intention, of GKI. They will be able to choose whatever
governor they want, because there are no *objective* reasons to not let
them do that.

> > This is a significant shift from the current model where vendors
> > completely own the kernel, and are largely free to use the kernel config
> > they want. Today, those who don't use schedutil are free to turn the
> > config off, for example.
> 
> So why is this regarded as a good thing?

You mean using something else than schedutil? It is not seen as a good
thing at all, at least not by me. But we have the same problem as
upstream. We cannot remove the other governors or the governor API for a
simple reason: they have users :/

Thanks,
Quentin
Rafael J. Wysocki May 12, 2020, 5:30 p.m. UTC | #26
On Tue, May 12, 2020 at 6:26 PM Quentin Perret <qperret@google.com> wrote:
>
> On Tuesday 12 May 2020 at 17:30:36 (+0200), Rafael J. Wysocki wrote:
> > On Tue, May 12, 2020 at 5:11 PM Quentin Perret <qperret@google.com> wrote:
> > > The end goal with GKI is the following: Google will release a single
> > > binary kernel image (signed, etc etc) that all devices using a given
> > > Android version will be required to use. That image is however going to
> > > be only for the core of the kernel (no drivers or anything of the sort).
> > > Vendors and OEMs, on their end, will be responsible to build and ship
> > > GKI-compatible modules for their respective devices. So, Android devices
> > > will eventually ship with a Google-issued GKI, plus a bunch of
> > > vendor-provided modules loaded during boot.
> >
> > If that is the case, then I absolutely think that schedutil should be
> > part of the GKI.
> >
> > Moreover, that would have been my opinion even if it had been modular
> > in the first place.
>
> I definitely understand the feeling. Heck I contributed to schedutil, so
> I'd love to see the entire world run it :-)
>
> But my personal preference doesn't seem to matter in this world, sadly.
> The truth is, we cannot afford to be arbitrary in our decisions in GKI.
> Switching governors and such is a fully supported feature upstream, and
> it has been for a long time. Taking that away from partners is not the
> goal, nor the intention, of GKI.

It still will be possible with schedutil built-in, however.

> They will be able to choose whatever
> governor they want, because there are no *objective* reasons to not let
> them do that.

Which, again, is still possible with non-modular schedutil AFAICS.

I don't see any technical reason for making schedutil modular in the
context of GKI other than to make the GKI image smaller, but I don't
expect that to be significant enough.

Is there anything else I am missing?

> > > This is a significant shift from the current model where vendors
> > > completely own the kernel, and are largely free to use the kernel config
> > > they want. Today, those who don't use schedutil are free to turn the
> > > config off, for example.
> >
> > So why is this regarded as a good thing?
>
> You mean using something else than schedutil?

I mean why allowing people to compile schedutil out is regarded as a good thing.

> It is not seen as a good
> thing at all, at least not by me. But we have the same problem as
> upstream. We cannot remove the other governors or the governor API for a
> simple reason: they have users :/

I'm not saying about removing any of that.  I'm just trying to
understand why you need schedutil to be modular so as to make those
things possible.

Cheers!
Quentin Perret May 13, 2020, 8:57 a.m. UTC | #27
Hey Joel,

On Tuesday 12 May 2020 at 11:49:32 (-0400), Joel Fernandes wrote:
> Also, does this series make it easier for vendors / oems / whoever to
> carry out-of-tree schedutil hacks saying that's "Ok" because that's
> not part of the core GKI? That would definitely be a bad thing to
> encourage as well. schedutil should pretty much be considered a part
> of the core GKI if the goal is to encourage everyone to move to it,
> IMO.

Sure, but I don't think the series makes it easier to carry out-of-tree
stuff. Vendors will have the choice to load the governor they want. Some
will use schedutil, some will use other upstream governors, and some will
use their out of tree crap. And that is orthogonal to schedutil being a
module or not.

The only thing that will happen is that they will complain about GKI,
and find examples of things like schedutil that is being forced on
them. Realistically, having schedutil built-in is unlikely to change
their mind about the governor they want to use, it is just likely to
give them reasons not to do the right thing and be GKI compliant.

Thanks,
Quentin
Quentin Perret May 13, 2020, 9:41 a.m. UTC | #28
Hi Rafael,

On Tuesday 12 May 2020 at 19:30:52 (+0200), Rafael J. Wysocki wrote:
> I don't see any technical reason for making schedutil modular in the
> context of GKI other than to make the GKI image smaller, but I don't
> expect that to be significant enough.

The fact that we can make the image smaller, and we give vendors one
less reason to not-want GKI _is_ desirable IMO.

  $ size vmlinux.*
     text	   data	    bss	    dec	    hex	filename
  19225963	9601976	 491084	29319023	1bf5f6f	vmlinux.after
  19230599	9603236	 491084	29324919	1bf7677	vmlinux.before

^ that's with the series applied. 'before' means sugov is =y, and
'after' is sugov =m. So modularizing saves just over 4K on text, and a
bit of data too. Is it significant? Maybe not. But it's quite likely
that those who don't use schedutil will find any unnecessary byte to be
one too many.

I just checked the size of modules in the default arm64 defconfig, and
the median is ~4K of text. The average is a little bigger, but mostly
because of a small number of really large modules (nouveau being the
prime the example). So all in all, the sugov module is not particularly
small by comparison with other things that have been modularized. A lot
of small things can lead to significant savings at the end.

Thanks,
Quentin
Greg KH May 13, 2020, 10:02 a.m. UTC | #29
On Wed, May 13, 2020 at 10:41:17AM +0100, Quentin Perret wrote:
> Hi Rafael,
> 
> On Tuesday 12 May 2020 at 19:30:52 (+0200), Rafael J. Wysocki wrote:
> > I don't see any technical reason for making schedutil modular in the
> > context of GKI other than to make the GKI image smaller, but I don't
> > expect that to be significant enough.
> 
> The fact that we can make the image smaller, and we give vendors one
> less reason to not-want GKI _is_ desirable IMO.
> 
>   $ size vmlinux.*
>      text	   data	    bss	    dec	    hex	filename
>   19225963	9601976	 491084	29319023	1bf5f6f	vmlinux.after
>   19230599	9603236	 491084	29324919	1bf7677	vmlinux.before
> 
> ^ that's with the series applied. 'before' means sugov is =y, and
> 'after' is sugov =m. So modularizing saves just over 4K on text, and a
> bit of data too. Is it significant? Maybe not. But it's quite likely
> that those who don't use schedutil will find any unnecessary byte to be
> one too many.

It's not significant at all, just always build it in, no one will notice
it, it's just a page or two.  Serial port drivers are way bigger :)

thanks,

greg k-h
Quentin Perret May 13, 2020, 10:06 a.m. UTC | #30
On Wednesday 13 May 2020 at 12:02:13 (+0200), Greg KH wrote:
> It's not significant at all, just always build it in, no one will notice
> it, it's just a page or two.  Serial port drivers are way bigger :)

Alright, I give up :)

When partners will complain (and I think they will) I'll point them here
and say we tried ;)

Cheers,
Quentin
Greg KH May 13, 2020, 10:24 a.m. UTC | #31
On Wed, May 13, 2020 at 11:06:11AM +0100, Quentin Perret wrote:
> On Wednesday 13 May 2020 at 12:02:13 (+0200), Greg KH wrote:
> > It's not significant at all, just always build it in, no one will notice
> > it, it's just a page or two.  Serial port drivers are way bigger :)
> 
> Alright, I give up :)
> 
> When partners will complain (and I think they will) I'll point them here
> and say we tried ;)

No problem, that's what lore.kernel.org archives are for :)