[PATCH 0/4] Allow cpuidle governors to be compiled as modules

[PATCH 0/4] Allow cpuidle governors to be compiled as modules

[Maulik Shah]

This series makes changes to allow cpuidle governors menu, ladder and teo
to compiled as modules when building with allmodconfig.

Patch 3 of the series is taken from [1] which brings back the change
removed in Commit 83788c0caed3 ("cpuidle: remove unused exports").

[1] https://lore.kernel.org/all/010101746fc98add-45e77496-d2d6-4bc1-a1ce-0692599a9a7a-000000@us-west-2.amazonses.com/

Lina Iyer (1):
  cpuidle: governor: export cpuidle governor functions

Maulik Shah (3):
  tick/sched: Export symbols used by cpuidle governors
  sched/core: Export symbols used by cpuidle governors
  cpuidle: governors: Allow the governors to be compiled as modules

 drivers/cpuidle/Kconfig            | 6 +++---
 drivers/cpuidle/governor.c         | 2 ++
 drivers/cpuidle/governors/ladder.c | 4 ++++
 drivers/cpuidle/governors/menu.c   | 4 ++++
 drivers/cpuidle/governors/teo.c    | 4 ++++
 kernel/sched/core.c                | 1 +
 kernel/time/tick-sched.c           | 5 +++++
 7 files changed, 23 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 1/4] tick/sched: Export symbols used by cpuidle governors

[Maulik Shah]

Export symbols that are used by cpuidle menu, ladder and teo governors
in preparation to allow cpuidle governors to be compiled as modules.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 kernel/time/tick-sched.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 6bffe5af..7cc6619 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -573,6 +573,8 @@ void __init tick_nohz_init(void)
  * NO HZ enabled ?
  */
 bool tick_nohz_enabled __read_mostly  = true;
+EXPORT_SYMBOL(tick_nohz_enabled);
+
 unsigned long tick_nohz_active  __read_mostly;
 /*
  * Enable / Disable tickless mode
@@ -590,6 +592,7 @@ bool tick_nohz_tick_stopped(void)
 
 	return ts->tick_stopped;
 }
+EXPORT_SYMBOL(tick_nohz_tick_stopped);
 
 bool tick_nohz_tick_stopped_cpu(int cpu)
 {
@@ -1147,6 +1150,7 @@ bool tick_nohz_idle_got_tick(void)
 	}
 	return false;
 }
+EXPORT_SYMBOL(tick_nohz_idle_got_tick);
 
 /**
  * tick_nohz_get_next_hrtimer - return the next expiration time for the hrtimer
@@ -1202,6 +1206,7 @@ ktime_t tick_nohz_get_sleep_length(ktime_t *delta_next)
 
 	return ktime_sub(next_event, now);
 }
+EXPORT_SYMBOL(tick_nohz_get_sleep_length);
 
 /**
  * tick_nohz_get_idle_calls_cpu - return the current idle calls counter value
-- 
2.7.4

[PATCH 2/4] sched/core: Export symbols used by cpuidle governors

[Maulik Shah]

Export symbols that are used by cpuidle menu governor in preparation
to allow cpuidle governors to be compiled as modules.

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 kernel/sched/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8cffe31..1d031e0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5047,6 +5047,7 @@ unsigned int nr_iowait_cpu(int cpu)
 {
 	return atomic_read(&cpu_rq(cpu)->nr_iowait);
 }
+EXPORT_SYMBOL(nr_iowait_cpu);
 
 /*
  * IO-wait accounting, and how it's mostly bollocks (on SMP).
-- 
2.7.4

[PATCH 3/4] cpuidle: governor: export cpuidle governor functions

[Maulik Shah]

From: Lina Iyer <ilina@codeaurora.org>

Commit 83788c0caed3 ("cpuidle: remove unused exports") removed
capability of registering cpuidle governors, which was unused at that
time. By exporting the symbol, let's allow platform specific modules to
register cpuidle governors and use cpuidle_governor_latency_req() to get
the QoS for the CPU.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 drivers/cpuidle/governor.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cpuidle/governor.c b/drivers/cpuidle/governor.c
index 29acaf4..0e51ed2 100644
--- a/drivers/cpuidle/governor.c
+++ b/drivers/cpuidle/governor.c
@@ -102,6 +102,7 @@ int cpuidle_register_governor(struct cpuidle_governor *gov)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(cpuidle_register_governor);
 
 /**
  * cpuidle_governor_latency_req - Compute a latency constraint for CPU
@@ -118,3 +119,4 @@ s64 cpuidle_governor_latency_req(unsigned int cpu)
 
 	return (s64)device_req * NSEC_PER_USEC;
 }
+EXPORT_SYMBOL_GPL(cpuidle_governor_latency_req);
-- 
2.7.4

[PATCH 4/4] cpuidle: governors: Allow the governors to be compiled as modules

[Maulik Shah]

Allow the menu, ladder and teo governors to be compiled as modules
when building allmodconfig.

Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 drivers/cpuidle/Kconfig            | 6 +++---
 drivers/cpuidle/governors/ladder.c | 4 ++++
 drivers/cpuidle/governors/menu.c   | 4 ++++
 drivers/cpuidle/governors/teo.c    | 4 ++++
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index c0aeedd..d71e3e4 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -19,13 +19,13 @@ config CPU_IDLE_MULTIPLE_DRIVERS
 	bool
 
 config CPU_IDLE_GOV_LADDER
-	bool "Ladder governor (for periodic timer tick)"
+	tristate "Ladder governor (for periodic timer tick)"
 
 config CPU_IDLE_GOV_MENU
-	bool "Menu governor (for tickless system)"
+	tristate "Menu governor (for tickless system)"
 
 config CPU_IDLE_GOV_TEO
-	bool "Timer events oriented (TEO) governor (for tickless systems)"
+	tristate "Timer events oriented (TEO) governor (for tickless systems)"
 	help
 	  This governor implements a simplified idle state selection method
 	  focused on timer events and does not do any interactivity boosting.
diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
index 8e9058c..4de5b3d 100644
--- a/drivers/cpuidle/governors/ladder.c
+++ b/drivers/cpuidle/governors/ladder.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/cpuidle.h>
 #include <linux/jiffies.h>
+#include <linux/module.h>
 #include <linux/tick.h>
 
 #include <asm/io.h>
@@ -195,3 +196,6 @@ static int __init init_ladder(void)
 }
 
 postcore_initcall(init_ladder);
+
+MODULE_DESCRIPTION("CPUidle Ladder governor (for periodic timer tick)");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 2e56704..2ef7cfc 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -13,6 +13,7 @@
 #include <linux/time.h>
 #include <linux/ktime.h>
 #include <linux/hrtimer.h>
+#include <linux/module.h>
 #include <linux/tick.h>
 #include <linux/sched.h>
 #include <linux/sched/loadavg.h>
@@ -577,3 +578,6 @@ static int __init init_menu(void)
 }
 
 postcore_initcall(init_menu);
+
+MODULE_DESCRIPTION("CPUidle Menu governor (for tickless system)");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
index d9262db..1a1d9ef 100644
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -104,6 +104,7 @@
 #include <linux/cpuidle.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/sched/clock.h>
 #include <linux/tick.h>
 
@@ -532,3 +533,6 @@ static int __init teo_governor_init(void)
 }
 
 postcore_initcall(teo_governor_init);
+
+MODULE_DESCRIPTION("CPUidle Timer events oriented (TEO) governor (for tickless systems)");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH 2/4] sched/core: Export symbols used by cpuidle governors

[Peter Zijlstra]

On Thu, Nov 25, 2021 at 02:24:39PM +0530, Maulik Shah wrote:
> Export symbols that are used by cpuidle menu governor in preparation
> to allow cpuidle governors to be compiled as modules.
> 
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> ---
>  kernel/sched/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 8cffe31..1d031e0 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -5047,6 +5047,7 @@ unsigned int nr_iowait_cpu(int cpu)
>  {
>  	return atomic_read(&cpu_rq(cpu)->nr_iowait);
>  }
> +EXPORT_SYMBOL(nr_iowait_cpu);

NACK, that function is batshit insane, exporting it serves nobody.

Re: [PATCH 0/4] Allow cpuidle governors to be compiled as modules

[Rafael J. Wysocki]

On Thu, Nov 25, 2021 at 9:55 AM Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> This series makes changes to allow cpuidle governors menu, ladder and teo
> to compiled as modules when building with allmodconfig.

What's the purpose of this?

Re: [PATCH 0/4] Allow cpuidle governors to be compiled as modules

[Maulik Shah]

Hi Rafael,

On 11/25/2021 5:57 PM, Rafael J. Wysocki wrote:
> On Thu, Nov 25, 2021 at 9:55 AM Maulik Shah <quic_mkshah@quicinc.com> wrote:
>> This series makes changes to allow cpuidle governors menu, ladder and teo
>> to compiled as modules when building with allmodconfig.
> What's the purpose of this?

There are two purposes of this series.

1. The series enables cpuidle governors to be allowed to compiled as 
modules.
     This is something similar to what cpufreq/devfreq governors do 
today as they can be be compiled as modules or built-in depending on the 
build config.

2. The series will also enable custom cpuidle governor to be able to 
register with cpuidle framework by using cpuidle_register_governor() API.
     This will be already achieved by (1) since it will export the 
required APIs for menu/ladder/teo governors to be compiled as module.

Thanks,
Maulik

Re: [PATCH 2/4] sched/core: Export symbols used by cpuidle governors

[Maulik Shah]

Hi Peter,

On 11/25/2021 3:05 PM, Peter Zijlstra wrote:
> On Thu, Nov 25, 2021 at 02:24:39PM +0530, Maulik Shah wrote:
>> Export symbols that are used by cpuidle menu governor in preparation
>> to allow cpuidle governors to be compiled as modules.
>>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Juri Lelli <juri.lelli@redhat.com>
>> Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
>> ---
>>   kernel/sched/core.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 8cffe31..1d031e0 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -5047,6 +5047,7 @@ unsigned int nr_iowait_cpu(int cpu)
>>   {
>>   	return atomic_read(&cpu_rq(cpu)->nr_iowait);
>>   }
>> +EXPORT_SYMBOL(nr_iowait_cpu);
> NACK, that function is batshit insane, exporting it serves nobody.
Thanks for the review.
Exporting is to serve cpuidle menu governor when its compiled as module 
(last patch in this series).

otherwise we get below error during compilation,
ERROR: modpost: "nr_iowait_cpu" [drivers/cpuidle/governors/menu.ko] 
undefined!

Do you suggest to use something else instead of this?
I think making this function inline and moving to linux/sched/stat.h 
(along with dependent data structures) may help but yet to try.

Thanks,
Maulik

Re: [PATCH 0/4] Allow cpuidle governors to be compiled as modules

[Rafael J. Wysocki]

On Thu, Nov 25, 2021 at 2:18 PM Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> Hi Rafael,
>
> On 11/25/2021 5:57 PM, Rafael J. Wysocki wrote:
> > On Thu, Nov 25, 2021 at 9:55 AM Maulik Shah <quic_mkshah@quicinc.com> wrote:
> >> This series makes changes to allow cpuidle governors menu, ladder and teo
> >> to compiled as modules when building with allmodconfig.
> > What's the purpose of this?
>
> There are two purposes of this series.
>
> 1. The series enables cpuidle governors to be allowed to compiled as
> modules.
>      This is something similar to what cpufreq/devfreq governors do
> today as they can be be compiled as modules or built-in depending on the
> build config.

Which is not the case for all of them, though, and I don't see why
this would imply that making cpuidle governors modular would be
useful.

> 2. The series will also enable custom cpuidle governor to be able to
> register with cpuidle framework by using cpuidle_register_governor() API.
>      This will be already achieved by (1) since it will export the
> required APIs for menu/ladder/teo governors to be compiled as module.

No custom cpuidle governors in the mainline, please.  If you have one
you want to be included, please submit it.

So from the mainline perspective this series doesn't serve any useful
purpose at all.

Sorry about that.

RE: [PATCH 0/4] Allow cpuidle governors to be compiled as modules

[Doug Smythies]

Hi All,

I realize that previous replies render this one useless
but sending anyhow.

On 2021.11.25 00:55 Maulik Shah wrote:

> This series makes changes to allow cpuidle governors
> menu, ladder and teo to compiled as modules when
> building with allmodconfig.

One current issue with governors being available as modules
is that they don't appear on the available governors list unless
they are loaded.

Example with this patch set, all done as modules:

~$ grep . /sys/devices/system/cpu/cpuidle/*
/sys/devices/system/cpu/cpuidle/current_driver:none
/sys/devices/system/cpu/cpuidle/current_governor:none
/sys/devices/system/cpu/cpuidle/current_governor_ro:none

However, and based on my systems power consumption,
some sort of idle must be running.

~$ echo teo | sudo tee /sys/devices/system/cpu/cpuidle/current_governor
teo
tee: /sys/devices/system/cpu/cpuidle/current_governor: Invalid argument

~$ sudo modprobe teo
~$ grep . /sys/devices/system/cpu/cpuidle/*
/sys/devices/system/cpu/cpuidle/available_governors:teo
/sys/devices/system/cpu/cpuidle/current_driver:none
/sys/devices/system/cpu/cpuidle/current_governor:teo
/sys/devices/system/cpu/cpuidle/current_governor_ro:teo

By the way, for the cpufreq stuff, while governors that
are actually available, but are modules, changing to them
without first force loading the module works:

$ grep . /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_governors
/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors:performance schedutil
/sys/devices/system/cpu/cpu10/cpufreq/scaling_available_governors:performance schedutil
...

$ echo ondemand | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
ondemand

$ grep . /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_governors
/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors:ondemand performance schedutil
/sys/devices/system/cpu/cpu10/cpufreq/scaling_available_governors:ondemand performance schedutil

... Doug

Re: [PATCH 2/4] sched/core: Export symbols used by cpuidle governors

[Peter Zijlstra]

On Thu, Nov 25, 2021 at 07:01:44PM +0530, Maulik Shah wrote:
> Hi Peter,
> 
> On 11/25/2021 3:05 PM, Peter Zijlstra wrote:
> > On Thu, Nov 25, 2021 at 02:24:39PM +0530, Maulik Shah wrote:
> > > Export symbols that are used by cpuidle menu governor in preparation
> > > to allow cpuidle governors to be compiled as modules.
> > > 
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Juri Lelli <juri.lelli@redhat.com>
> > > Cc: Vincent Guittot <vincent.guittot@linaro.org>
> > > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> > > ---
> > >   kernel/sched/core.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > > index 8cffe31..1d031e0 100644
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -5047,6 +5047,7 @@ unsigned int nr_iowait_cpu(int cpu)
> > >   {
> > >   	return atomic_read(&cpu_rq(cpu)->nr_iowait);
> > >   }
> > > +EXPORT_SYMBOL(nr_iowait_cpu);
> > NACK, that function is batshit insane, exporting it serves nobody.
> Thanks for the review.
> Exporting is to serve cpuidle menu governor when its compiled as module
> (last patch in this series).
> 
> otherwise we get below error during compilation,
> ERROR: modpost: "nr_iowait_cpu" [drivers/cpuidle/governors/menu.ko]
> undefined!
> 
> Do you suggest to use something else instead of this?

Yeah, schedutil :-)

Re: [PATCH 1/4] tick/sched: Export symbols used by cpuidle governors

[Thomas Gleixner]

On Thu, Nov 25 2021 at 14:24, Maulik Shah wrote:
>  bool tick_nohz_enabled __read_mostly  = true;
> +EXPORT_SYMBOL(tick_nohz_enabled);

If any of this gets ever exported then with EXPORT_SYMBOL_GPL(), but I
agree with Rafael that there is no real value for this, so the exports
are not required either.

Thanks,

        tglx