Re: [PATCH] CPU C-state breakage with PM Qos change

8 messages, 4 authors, 2012-02-07 · open the first message on its own page

Re: [PATCH] CPU C-state breakage with PM Qos change

From: Pihet-XID, Jean <hidden>
Date: 2012-02-03 14:04:46

Looping in linux-pm

On Fri, Feb 3, 2012 at 1:14 AM, Venkatesh Pallipadi [off-list ref] wrote:
Looks like change "PM QoS: Move and rename the implementation files"
made pm_qos depend on CONFIG_PM which depends on
PM_SLEEP || PM_RUNTIME

That breaks CPU C-states with kernels not having these CONFIGs, causing CPUs
to spend time in Polling loop idle instead of going into deep C-states,
consuming way way more power. This is with either acpi idle or intel idle
enabled.

Either CONFIG_PM should be enabled with any pm_qos users or
the !CONFIG_PM pm_qos_request() should return sane defaults not to break
the existing users. Here's is the patch for the latter option.
I think the real question is whether PM QoS should be functional in
all cases (as is ACPI) or whether only if certain options are set
(CONFIG_PM).
In the current code if CONFIG_PM is not enabled, a dummy PM QoS API is
provided as function stubs in order for the build to succeed.

Rafael, Mark,
What do you think? Should PM QoS be enabled in all cases? Are there
any known dependencies with CONFIG_PM?

Regards,
Jean

Re: [PATCH] CPU C-state breakage with PM Qos change

From: Rafael J. Wysocki <hidden>
Date: 2012-02-03 19:58:35

On Friday, February 03, 2012, Pihet-XID, Jean wrote:
Looping in linux-pm

On Fri, Feb 3, 2012 at 1:14 AM, Venkatesh Pallipadi [off-list ref] wrote:
quoted
Looks like change "PM QoS: Move and rename the implementation files"
made pm_qos depend on CONFIG_PM which depends on
PM_SLEEP || PM_RUNTIME

That breaks CPU C-states with kernels not having these CONFIGs, causing CPUs
to spend time in Polling loop idle instead of going into deep C-states,
consuming way way more power. This is with either acpi idle or intel idle
enabled.

Either CONFIG_PM should be enabled with any pm_qos users or
the !CONFIG_PM pm_qos_request() should return sane defaults not to break
the existing users. Here's is the patch for the latter option.
I think the real question is whether PM QoS should be functional in
all cases (as is ACPI) or whether only if certain options are set
(CONFIG_PM).
In the current code if CONFIG_PM is not enabled, a dummy PM QoS API is
provided as function stubs in order for the build to succeed.

Rafael, Mark,
What do you think? Should PM QoS be enabled in all cases? Are there
any known dependencies with CONFIG_PM?
At least we should keep the current behavior to avoid breaking things
for now.  We can change that in the next cycle, however, if everyone
agrees, but more carefully.

The patch has been applied to linux-pm/linux-next and will be pushed to Linus
early next week.

Thanks,
Rafael

Re: [PATCH] CPU C-state breakage with PM Qos change

From: mark gross <hidden>
Date: 2012-02-05 03:51:03

On Fri, Feb 03, 2012 at 03:04:43PM +0100, Pihet-XID, Jean wrote:
Looping in linux-pm

On Fri, Feb 3, 2012 at 1:14 AM, Venkatesh Pallipadi [off-list ref] wrote:
quoted
Looks like change "PM QoS: Move and rename the implementation files"
made pm_qos depend on CONFIG_PM which depends on
PM_SLEEP || PM_RUNTIME

That breaks CPU C-states with kernels not having these CONFIGs, causing CPUs
to spend time in Polling loop idle instead of going into deep C-states,
consuming way way more power. This is with either acpi idle or intel idle
enabled.

Either CONFIG_PM should be enabled with any pm_qos users or
the !CONFIG_PM pm_qos_request() should return sane defaults not to break
the existing users. Here's is the patch for the latter option.
I think the real question is whether PM QoS should be functional in
all cases (as is ACPI) or whether only if certain options are set
(CONFIG_PM).
In the current code if CONFIG_PM is not enabled, a dummy PM QoS API is
provided as function stubs in order for the build to succeed.

Rafael, Mark,
What do you think? Should PM QoS be enabled in all cases? Are there
any known dependencies with CONFIG_PM?
Yes I do think pm_qos interfaces should be enabled all the time and be
independent of CONFIG_PM.  Also, I still am not a fan of the renaming
patch but, as the argument for and against renaming cannot be based on
quantifiable things I've chosen not to let it bother me.

I think Venki's change is a band aid and we should fix it right by not
having a dependency on config_pm for the interface to behave.

I'll take a look at why there is now a dependency before I have more to
say.

--mark

Re: [PATCH] CPU C-state breakage with PM Qos change

From: Rafael J. Wysocki <hidden>
Date: 2012-02-05 11:00:55

On Sunday, February 05, 2012, mark gross wrote:
On Fri, Feb 03, 2012 at 03:04:43PM +0100, Pihet-XID, Jean wrote:
quoted
Looping in linux-pm

On Fri, Feb 3, 2012 at 1:14 AM, Venkatesh Pallipadi [off-list ref] wrote:
quoted
Looks like change "PM QoS: Move and rename the implementation files"
made pm_qos depend on CONFIG_PM which depends on
PM_SLEEP || PM_RUNTIME

That breaks CPU C-states with kernels not having these CONFIGs, causing CPUs
to spend time in Polling loop idle instead of going into deep C-states,
consuming way way more power. This is with either acpi idle or intel idle
enabled.

Either CONFIG_PM should be enabled with any pm_qos users or
the !CONFIG_PM pm_qos_request() should return sane defaults not to break
the existing users. Here's is the patch for the latter option.
I think the real question is whether PM QoS should be functional in
all cases (as is ACPI) or whether only if certain options are set
(CONFIG_PM).
In the current code if CONFIG_PM is not enabled, a dummy PM QoS API is
provided as function stubs in order for the build to succeed.

Rafael, Mark,
What do you think? Should PM QoS be enabled in all cases? Are there
any known dependencies with CONFIG_PM?
Yes I do think pm_qos interfaces should be enabled all the time and be
independent of CONFIG_PM.  Also, I still am not a fan of the renaming
patch but, as the argument for and against renaming cannot be based on
quantifiable things I've chosen not to let it bother me.

I think Venki's change is a band aid and we should fix it right by not
having a dependency on config_pm for the interface to behave.

I'll take a look at why there is now a dependency before I have more to
say.
In kernel/power/Makefile:

obj-$(CONFIG_PM)                += main.o qos.o

I guess that explains things. :-)

It's quite easy to make qos.o be independent of CONFIG_PM, in which case the
code added by Venki can be removed, so patches welcome (for 3.4, though).

Thanks,
Rafael

Re: [linux-pm] [PATCH] CPU C-state breakage with PM Qos change

From: Jean Pihet <hidden>
Date: 2012-02-06 10:18:35

Hi Rafael, Mark,

On Sun, Feb 5, 2012 at 12:04 PM, Rafael J. Wysocki [off-list ref] wrote:
On Sunday, February 05, 2012, mark gross wrote:
quoted
On Fri, Feb 03, 2012 at 03:04:43PM +0100, Pihet-XID, Jean wrote:
quoted
Looping in linux-pm

On Fri, Feb 3, 2012 at 1:14 AM, Venkatesh Pallipadi [off-list ref] wrote:
quoted
Looks like change "PM QoS: Move and rename the implementation files"
made pm_qos depend on CONFIG_PM which depends on
PM_SLEEP || PM_RUNTIME

That breaks CPU C-states with kernels not having these CONFIGs, causing CPUs
to spend time in Polling loop idle instead of going into deep C-states,
consuming way way more power. This is with either acpi idle or intel idle
enabled.

Either CONFIG_PM should be enabled with any pm_qos users or
the !CONFIG_PM pm_qos_request() should return sane defaults not to break
the existing users. Here's is the patch for the latter option.
I think the real question is whether PM QoS should be functional in
all cases (as is ACPI) or whether only if certain options are set
(CONFIG_PM).
In the current code if CONFIG_PM is not enabled, a dummy PM QoS API is
provided as function stubs in order for the build to succeed.

Rafael, Mark,
What do you think? Should PM QoS be enabled in all cases? Are there
any known dependencies with CONFIG_PM?
Yes I do think pm_qos interfaces should be enabled all the time and be
independent of CONFIG_PM.  Also, I still am not a fan of the renaming
patch but, as the argument for and against renaming cannot be based on
quantifiable things I've chosen not to let it bother me.

I think Venki's change is a band aid and we should fix it right by not
having a dependency on config_pm for the interface to behave.

I'll take a look at why there is now a dependency before I have more to
say.
In kernel/power/Makefile:

obj-$(CONFIG_PM)                += main.o qos.o

I guess that explains things. :-)
Initially I thought we should have a way of disabling the feature on
some (minimal) kernels and so thought CONFIG_PM was the option to use.
It's quite easy to make qos.o be independent of CONFIG_PM, in which case the
code added by Venki can be removed, so patches welcome (for 3.4, though).
I am working on it, more to come soon.
Thanks,
Rafael
Thanks,
Jean
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

Re: [linux-pm] [PATCH] CPU C-state breakage with PM Qos change

From: Jean Pihet <hidden>
Date: 2012-02-06 16:42:03

Rafael,

On Mon, Feb 6, 2012 at 11:18 AM, Jean Pihet [off-list ref] wrote:
Hi Rafael, Mark,

On Sun, Feb 5, 2012 at 12:04 PM, Rafael J. Wysocki [off-list ref] wrote:
...
quoted
In kernel/power/Makefile:

obj-$(CONFIG_PM)                += main.o qos.o

I guess that explains things. :-)
Initially I thought we should have a way of disabling the feature on
some (minimal) kernels and so thought CONFIG_PM was the option to use.
quoted
It's quite easy to make qos.o be independent of CONFIG_PM, in which case the
code added by Venki can be removed, so patches welcome (for 3.4, though).
I am working on it, more to come soon.
I have a couple of patches ready, to be applied on 3.3-rc1 (so without
Venki's patch applied).
The first one is on PM QoS, the second one on per-device PM QoS. Is
the latter needed?

Please let me know and I will send them asap.

Regards,
Jean
quoted
Thanks,
Rafael

Re: [linux-pm] [PATCH] CPU C-state breakage with PM Qos change

From: Rafael J. Wysocki <hidden>
Date: 2012-02-06 20:12:32

On Monday, February 06, 2012, Jean Pihet wrote:
Rafael,

On Mon, Feb 6, 2012 at 11:18 AM, Jean Pihet [off-list ref] wrote:
quoted
Hi Rafael, Mark,

On Sun, Feb 5, 2012 at 12:04 PM, Rafael J. Wysocki [off-list ref] wrote:
...
quoted
quoted
In kernel/power/Makefile:

obj-$(CONFIG_PM)                += main.o qos.o

I guess that explains things. :-)
Initially I thought we should have a way of disabling the feature on
some (minimal) kernels and so thought CONFIG_PM was the option to use.
quoted
It's quite easy to make qos.o be independent of CONFIG_PM, in which case the
code added by Venki can be removed, so patches welcome (for 3.4, though).
I am working on it, more to come soon.
I have a couple of patches ready, to be applied on 3.3-rc1 (so without
Venki's patch applied).
The first one is on PM QoS, the second one on per-device PM QoS. Is
the latter needed?
I'm not sure without looking. :-)

Thanks,
Rafael

Re: [linux-pm] [PATCH] CPU C-state breakage with PM Qos change

From: Jean Pihet <hidden>
Date: 2012-02-07 08:39:21

On Mon, Feb 6, 2012 at 9:16 PM, Rafael J. Wysocki [off-list ref] wrote:
On Monday, February 06, 2012, Jean Pihet wrote:
quoted
Rafael,

On Mon, Feb 6, 2012 at 11:18 AM, Jean Pihet [off-list ref] wrote:
quoted
Hi Rafael, Mark,

On Sun, Feb 5, 2012 at 12:04 PM, Rafael J. Wysocki [off-list ref] wrote:
...
quoted
quoted
In kernel/power/Makefile:

obj-$(CONFIG_PM)                += main.o qos.o

I guess that explains things. :-)
Initially I thought we should have a way of disabling the feature on
some (minimal) kernels and so thought CONFIG_PM was the option to use.
quoted
It's quite easy to make qos.o be independent of CONFIG_PM, in which case the
code added by Venki can be removed, so patches welcome (for 3.4, though).
I am working on it, more to come soon.
I have a couple of patches ready, to be applied on 3.3-rc1 (so without
Venki's patch applied).
The first one is on PM QoS, the second one on per-device PM QoS. Is
the latter needed?
I'm not sure without looking. :-)
That makes sense ;p

Just sent out the patch set as '[PATCH 0/2] PM / QoS: unconditionally
build the feature'.
It has been compile tested only using the i386 defconfig, with and
without CONFIG_PM.

It requires some more testing on x86.
Venki, can you check it out if this fixes the initial problem?
Thanks,
Rafael
Regards,
Jean
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help