LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
@ 2021-11-25  9:14 Maulik Shah
  2021-11-25  9:51 ` Peter Zijlstra
  0 siblings, 1 reply; 9+ messages in thread
From: Maulik Shah @ 2021-11-25  9:14 UTC (permalink / raw)
  To: bjorn.andersson, rafael, daniel.lezcano
  Cc: linux-arm-msm, linux-pm, linux-kernel, ulf.hansson, quic_lsrao,
	rnayak, Maulik Shah, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot

Export cpu_idle_poll_ctrl() so that module drivers can use same.

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/idle.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index d17b0a5..7fbc07f 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -33,6 +33,7 @@ void cpu_idle_poll_ctrl(bool enable)
 		WARN_ON_ONCE(cpu_idle_force_poll < 0);
 	}
 }
+EXPORT_SYMBOL(cpu_idle_poll_ctrl);
 
 #ifdef CONFIG_GENERIC_IDLE_POLL_SETUP
 static int __init cpu_idle_poll_setup(char *__unused)
-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
  2021-11-25  9:14 [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol Maulik Shah
@ 2021-11-25  9:51 ` Peter Zijlstra
  2021-11-25 14:13   ` Maulik Shah
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2021-11-25  9:51 UTC (permalink / raw)
  To: Maulik Shah
  Cc: bjorn.andersson, rafael, daniel.lezcano, linux-arm-msm, linux-pm,
	linux-kernel, ulf.hansson, quic_lsrao, rnayak, Ingo Molnar,
	Juri Lelli, Vincent Guittot

On Thu, Nov 25, 2021 at 02:44:36PM +0530, Maulik Shah wrote:
> Export cpu_idle_poll_ctrl() so that module drivers can use same.

This does not seem like a really safe interface to expose to the
world.

Surely the better solution is to rework things to not rely on this. I'm
fairly sure it's not hard to write a cpuidle driver that does much the
same.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
  2021-11-25  9:51 ` Peter Zijlstra
@ 2021-11-25 14:13   ` Maulik Shah
  2021-11-25 17:26     ` Daniel Lezcano
  2021-11-26 12:05     ` Ulf Hansson
  0 siblings, 2 replies; 9+ messages in thread
From: Maulik Shah @ 2021-11-25 14:13 UTC (permalink / raw)
  To: Peter Zijlstra, Rafael J. Wysocki, Daniel Lezcano
  Cc: bjorn.andersson, rafael, daniel.lezcano, linux-arm-msm, linux-pm,
	linux-kernel, ulf.hansson, quic_lsrao, rnayak, Ingo Molnar,
	Juri Lelli, Vincent Guittot

Hi Peter,

On 11/25/2021 3:21 PM, Peter Zijlstra wrote:
> On Thu, Nov 25, 2021 at 02:44:36PM +0530, Maulik Shah wrote:
>> Export cpu_idle_poll_ctrl() so that module drivers can use same.
> This does not seem like a really safe interface to expose to the
> world.

Thanks for the review.

Keeping the cpuidle enabled from boot up may delay/increase the boot up 
time.
Below is our use case to force cpuidle to stay in cpu_idle_poll().

We keep cpuidle disabled from boot up using "nohlt" option of kernel 
command line which internally sets cpu_idle_force_poll = 1;
and once the device bootup reaches till certain point (for example the 
android homescreen is up) userspace may notify a
vendor module driver which can invoke cpu_idle_poll_ctrl(false); to come 
out of poll mode.
So vendor module driver needs cpu_idle_poll_ctrl() exported symbol.

We can not take PM-QoS from driver to prevent deep cpuidle since all the 
vendor modules are kept in a separate partition and will be loaded only 
after kernel boot up is done
and by this time kernel already starts executing deep cpuidle modes.
>
> Surely the better solution is to rework things to not rely on this. I'm
> fairly sure it's not hard to write a cpuidle driver that does much the
> same.
The other option i think is to pass cpuidle.off=1 in kernel command line 
and then add enable_cpuidle() in drivers/cpuidle/cpuidle.c
something similar as below which can be called by vendor module.

void enable_cpuidle(void)
{
         off = 0;
}
EXPORT_SYMBOL_GPL(enable_cpuidle);

This may be a good option since we have already disable_cpuidle() but 
not enable_cpuidle().

void disable_cpuidle(void)
{
         off = 1;
}

Hi Rafael/Daniel, can you please let me know your suggestion on 
this/similar implementation?

Thanks,
Maulik

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
  2021-11-25 14:13   ` Maulik Shah
@ 2021-11-25 17:26     ` Daniel Lezcano
  2021-11-26  5:56       ` Maulik Shah
  2021-11-26 12:05     ` Ulf Hansson
  1 sibling, 1 reply; 9+ messages in thread
From: Daniel Lezcano @ 2021-11-25 17:26 UTC (permalink / raw)
  To: Maulik Shah, Peter Zijlstra, Rafael J. Wysocki
  Cc: bjorn.andersson, linux-arm-msm, linux-pm, linux-kernel,
	ulf.hansson, quic_lsrao, rnayak, Ingo Molnar, Juri Lelli,
	Vincent Guittot

On 25/11/2021 15:13, Maulik Shah wrote:
> Hi Peter,
> 
> On 11/25/2021 3:21 PM, Peter Zijlstra wrote:
>> On Thu, Nov 25, 2021 at 02:44:36PM +0530, Maulik Shah wrote:
>>> Export cpu_idle_poll_ctrl() so that module drivers can use same.
>> This does not seem like a really safe interface to expose to the
>> world.
> 
> Thanks for the review.
> 
> Keeping the cpuidle enabled from boot up may delay/increase the boot up
> time.
> Below is our use case to force cpuidle to stay in cpu_idle_poll().
> 
> We keep cpuidle disabled from boot up using "nohlt" option of kernel
> command line which internally sets cpu_idle_force_poll = 1;
> and once the device bootup reaches till certain point (for example the
> android homescreen is up) userspace may notify a
> vendor module driver which can invoke cpu_idle_poll_ctrl(false); to come
> out of poll mode.
> So vendor module driver needs cpu_idle_poll_ctrl() exported symbol.
> 
> We can not take PM-QoS from driver to prevent deep cpuidle since all the
> vendor modules are kept in a separate partition and will be loaded only
> after kernel boot up is done
> and by this time kernel already starts executing deep cpuidle modes.
>>
>> Surely the better solution is to rework things to not rely on this. I'm
>> fairly sure it's not hard to write a cpuidle driver that does much the
>> same.
> The other option i think is to pass cpuidle.off=1 in kernel command line
> and then add enable_cpuidle() in drivers/cpuidle/cpuidle.c
> something similar as below which can be called by vendor module.
> 
> void enable_cpuidle(void)
> {
>         off = 0;
> }
> EXPORT_SYMBOL_GPL(enable_cpuidle);
> 
> This may be a good option since we have already disable_cpuidle() but
> not enable_cpuidle().
> 
> void disable_cpuidle(void)
> {
>         off = 1;
> }
> 
> Hi Rafael/Daniel, can you please let me know your suggestion on
> this/similar implementation?

Did you try to use the QoS latency? Sounds like it is exactly for this
purpose.

Set it to zero to force cpuidle to choose the shallowest idle state and
then INT_MAX to disable the constraint.

 cpu_latency_qos_add_request();

Hope that helps

  -- Daniel

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
  2021-11-25 17:26     ` Daniel Lezcano
@ 2021-11-26  5:56       ` Maulik Shah
  2021-11-26 11:22         ` Daniel Lezcano
  0 siblings, 1 reply; 9+ messages in thread
From: Maulik Shah @ 2021-11-26  5:56 UTC (permalink / raw)
  To: Daniel Lezcano, Peter Zijlstra, Rafael J. Wysocki
  Cc: bjorn.andersson, linux-arm-msm, linux-pm, linux-kernel,
	ulf.hansson, quic_lsrao, rnayak, Ingo Molnar, Juri Lelli,
	Vincent Guittot

Hi Daniel,

On 11/25/2021 10:56 PM, Daniel Lezcano wrote:
> On 25/11/2021 15:13, Maulik Shah wrote:
>> Hi Peter,
>>
>> On 11/25/2021 3:21 PM, Peter Zijlstra wrote:
>>> On Thu, Nov 25, 2021 at 02:44:36PM +0530, Maulik Shah wrote:
>>>> Export cpu_idle_poll_ctrl() so that module drivers can use same.
>>> This does not seem like a really safe interface to expose to the
>>> world.
>> Thanks for the review.
>>
>> Keeping the cpuidle enabled from boot up may delay/increase the boot up
>> time.
>> Below is our use case to force cpuidle to stay in cpu_idle_poll().
>>
>> We keep cpuidle disabled from boot up using "nohlt" option of kernel
>> command line which internally sets cpu_idle_force_poll = 1;
>> and once the device bootup reaches till certain point (for example the
>> android homescreen is up) userspace may notify a
>> vendor module driver which can invoke cpu_idle_poll_ctrl(false); to come
>> out of poll mode.
>> So vendor module driver needs cpu_idle_poll_ctrl() exported symbol.
>>
>> We can not take PM-QoS from driver to prevent deep cpuidle since all the
>> vendor modules are kept in a separate partition and will be loaded only
>> after kernel boot up is done
>> and by this time kernel already starts executing deep cpuidle modes.
>>> Surely the better solution is to rework things to not rely on this. I'm
>>> fairly sure it's not hard to write a cpuidle driver that does much the
>>> same.
>> The other option i think is to pass cpuidle.off=1 in kernel command line
>> and then add enable_cpuidle() in drivers/cpuidle/cpuidle.c
>> something similar as below which can be called by vendor module.
>>
>> void enable_cpuidle(void)
>> {
>>          off = 0;
>> }
>> EXPORT_SYMBOL_GPL(enable_cpuidle);
>>
>> This may be a good option since we have already disable_cpuidle() but
>> not enable_cpuidle().
>>
>> void disable_cpuidle(void)
>> {
>>          off = 1;
>> }
>>
>> Hi Rafael/Daniel, can you please let me know your suggestion on
>> this/similar implementation?
> Did you try to use the QoS latency? Sounds like it is exactly for this
> purpose.
>
> Set it to zero to force cpuidle to choose the shallowest idle state and
> then INT_MAX to disable the constraint.
>
>   cpu_latency_qos_add_request();
>
> Hope that helps
>
>    -- Daniel
The PM-QoS is not helping here since all the vendor drivers are kept in 
a separate partition
and will be loaded only after kernel boot up is done and by the time 
vendor kernel modules are inserted
takes QoS, kernel/menu governor already starts executing deep cpuidle modes.

kernel start (t0)---------Menu governor loads (t1)----------vendor 
modules loaded (t2)----------Usespace ready(t3)

Untill (t2), its only core kernel/android kernel which don't have any 
vendor driver which can take QoS.
If we take QoS, it can be taken only from point (t2) but CPUs still 
enter deep idle state between (t1) to (t2).

So to prevent this passing "cpuidle.off=1" or "nohlt" in kernel command 
line can keep deep cpuidle states disabled from boot up and
once vendor modules are ready at (t2) or (t3), it can either invoke 
newly added enable_cpuidle() or cpu_idle_poll_ctrl(false);
to comeout of polling mode and start executing deep low power modes.

Thanks,
Maulik

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
  2021-11-26  5:56       ` Maulik Shah
@ 2021-11-26 11:22         ` Daniel Lezcano
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Lezcano @ 2021-11-26 11:22 UTC (permalink / raw)
  To: Maulik Shah, Peter Zijlstra, Rafael J. Wysocki
  Cc: bjorn.andersson, linux-arm-msm, linux-pm, linux-kernel,
	ulf.hansson, quic_lsrao, rnayak, Ingo Molnar, Juri Lelli,
	Vincent Guittot

On 26/11/2021 06:56, Maulik Shah wrote:
> Hi Daniel,
> 
> On 11/25/2021 10:56 PM, Daniel Lezcano wrote:
>> On 25/11/2021 15:13, Maulik Shah wrote:
>>> Hi Peter,
>>>
>>> On 11/25/2021 3:21 PM, Peter Zijlstra wrote:
>>>> On Thu, Nov 25, 2021 at 02:44:36PM +0530, Maulik Shah wrote:
>>>>> Export cpu_idle_poll_ctrl() so that module drivers can use same.
>>>> This does not seem like a really safe interface to expose to the
>>>> world.
>>> Thanks for the review.
>>>
>>> Keeping the cpuidle enabled from boot up may delay/increase the boot up
>>> time.
>>> Below is our use case to force cpuidle to stay in cpu_idle_poll().
>>>
>>> We keep cpuidle disabled from boot up using "nohlt" option of kernel
>>> command line which internally sets cpu_idle_force_poll = 1;
>>> and once the device bootup reaches till certain point (for example the
>>> android homescreen is up) userspace may notify a
>>> vendor module driver which can invoke cpu_idle_poll_ctrl(false); to come
>>> out of poll mode.
>>> So vendor module driver needs cpu_idle_poll_ctrl() exported symbol.
>>>
>>> We can not take PM-QoS from driver to prevent deep cpuidle since all the
>>> vendor modules are kept in a separate partition and will be loaded only
>>> after kernel boot up is done
>>> and by this time kernel already starts executing deep cpuidle modes.
>>>> Surely the better solution is to rework things to not rely on this. I'm
>>>> fairly sure it's not hard to write a cpuidle driver that does much the
>>>> same.
>>> The other option i think is to pass cpuidle.off=1 in kernel command line
>>> and then add enable_cpuidle() in drivers/cpuidle/cpuidle.c
>>> something similar as below which can be called by vendor module.
>>>
>>> void enable_cpuidle(void)
>>> {
>>>          off = 0;
>>> }
>>> EXPORT_SYMBOL_GPL(enable_cpuidle);
>>>
>>> This may be a good option since we have already disable_cpuidle() but
>>> not enable_cpuidle().
>>>
>>> void disable_cpuidle(void)
>>> {
>>>          off = 1;
>>> }
>>>
>>> Hi Rafael/Daniel, can you please let me know your suggestion on
>>> this/similar implementation?
>> Did you try to use the QoS latency? Sounds like it is exactly for this
>> purpose.
>>
>> Set it to zero to force cpuidle to choose the shallowest idle state and
>> then INT_MAX to disable the constraint.
>>
>>   cpu_latency_qos_add_request();
>>
>> Hope that helps
>>
>>    -- Daniel
> The PM-QoS is not helping here since all the vendor drivers are kept in
> a separate partition
> and will be loaded only after kernel boot up is done and by the time
> vendor kernel modules are inserted
> takes QoS, kernel/menu governor already starts executing deep cpuidle
> modes.
> 
> kernel start (t0)---------Menu governor loads (t1)----------vendor
> modules loaded (t2)----------Usespace ready(t3)
> 
> Untill (t2), its only core kernel/android kernel which don't have any
> vendor driver which can take QoS.
> If we take QoS, it can be taken only from point (t2) but CPUs still
> enter deep idle state between (t1) to (t2).
> 
> So to prevent this passing "cpuidle.off=1" or "nohlt" in kernel command
> line can keep deep cpuidle states disabled from boot up and
> once vendor modules are ready at (t2) or (t3), it can either invoke
> newly added enable_cpuidle() or cpu_idle_poll_ctrl(false);
> to comeout of polling mode and start executing deep low power modes.

I understand. The approach is valid but I'm wondering if it should fall
under a more global feature with a perf <-> power cursor

If the goal is to be as fast as possible at boot time, the cursor should
be set to 'perf and then changed to 'power' after the system has booted.

So not only cpuidle but also any other subsystems can skip the PM path
or go the highest performance state (assuming the PM code is compiled-in
for the devices).




-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
  2021-11-25 14:13   ` Maulik Shah
  2021-11-25 17:26     ` Daniel Lezcano
@ 2021-11-26 12:05     ` Ulf Hansson
  1 sibling, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2021-11-26 12:05 UTC (permalink / raw)
  To: Maulik Shah
  Cc: Peter Zijlstra, Rafael J. Wysocki, Daniel Lezcano,
	bjorn.andersson, linux-arm-msm, linux-pm, linux-kernel,
	quic_lsrao, rnayak, Ingo Molnar, Juri Lelli, Vincent Guittot

On Thu, 25 Nov 2021 at 15:13, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> Hi Peter,
>
> On 11/25/2021 3:21 PM, Peter Zijlstra wrote:
> > On Thu, Nov 25, 2021 at 02:44:36PM +0530, Maulik Shah wrote:
> >> Export cpu_idle_poll_ctrl() so that module drivers can use same.
> > This does not seem like a really safe interface to expose to the
> > world.
>
> Thanks for the review.
>
> Keeping the cpuidle enabled from boot up may delay/increase the boot up
> time.
> Below is our use case to force cpuidle to stay in cpu_idle_poll().
>
> We keep cpuidle disabled from boot up using "nohlt" option of kernel
> command line which internally sets cpu_idle_force_poll = 1;
> and once the device bootup reaches till certain point (for example the
> android homescreen is up) userspace may notify a
> vendor module driver which can invoke cpu_idle_poll_ctrl(false); to come
> out of poll mode.
> So vendor module driver needs cpu_idle_poll_ctrl() exported symbol.

Waiting for the homescreen can be considered as rather late, from the
kernel boot progress point of view.

That said, I am wondering if a similar improvement can be achieved by
just allowing WFI (thus no deeper idle states) until homescreen? If
so, that can be quite easily achieved by modularizing the cpuidle-psci
driver.

[...]

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
  2021-11-25  9:18 Maulik Shah
@ 2021-11-26  9:15 ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2021-11-26  9:15 UTC (permalink / raw)
  To: Maulik Shah
  Cc: bjorn.andersson, rafael, daniel.lezcano, linux-arm-msm, linux-pm,
	linux-kernel, ulf.hansson, quic_lsrao, rnayak, Ingo Molnar,
	Peter Zijlstra, Juri Lelli, Vincent Guittot

On Thu, Nov 25, 2021 at 02:48:53PM +0530, Maulik Shah wrote:
> Export cpu_idle_poll_ctrl() so that module drivers can use same.

Independ of the fact that this looks like a module that should not be
exported at all:  please always post exports with the actual module
using them, otherwise they are unreviewable and unacceptable.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol
@ 2021-11-25  9:18 Maulik Shah
  2021-11-26  9:15 ` Christoph Hellwig
  0 siblings, 1 reply; 9+ messages in thread
From: Maulik Shah @ 2021-11-25  9:18 UTC (permalink / raw)
  To: bjorn.andersson, rafael, daniel.lezcano
  Cc: linux-arm-msm, linux-pm, linux-kernel, ulf.hansson, quic_lsrao,
	rnayak, Maulik Shah, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot

Export cpu_idle_poll_ctrl() so that module drivers can use same.

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/idle.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index d17b0a5..7fbc07f 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -33,6 +33,7 @@ void cpu_idle_poll_ctrl(bool enable)
 		WARN_ON_ONCE(cpu_idle_force_poll < 0);
 	}
 }
+EXPORT_SYMBOL(cpu_idle_poll_ctrl);
 
 #ifdef CONFIG_GENERIC_IDLE_POLL_SETUP
 static int __init cpu_idle_poll_setup(char *__unused)
-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-11-26 12:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25  9:14 [PATCH] sched/idle: Export cpu_idle_poll_ctrl() symbol Maulik Shah
2021-11-25  9:51 ` Peter Zijlstra
2021-11-25 14:13   ` Maulik Shah
2021-11-25 17:26     ` Daniel Lezcano
2021-11-26  5:56       ` Maulik Shah
2021-11-26 11:22         ` Daniel Lezcano
2021-11-26 12:05     ` Ulf Hansson
2021-11-25  9:18 Maulik Shah
2021-11-26  9:15 ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).