* [PATCH] CPU C-state breakage with PM Qos change @ 2012-02-02 23:57 Venkatesh Pallipadi 2012-02-03 0:14 ` Venkatesh Pallipadi 0 siblings, 1 reply; 10+ messages in thread From: Venkatesh Pallipadi @ 2012-02-02 23:57 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Kevin Hilman, Jean Pihet, markgross, linux-kernel, Len Brown, Venkatesh Pallipadi 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. Signed-off-by: Venkatesh Pallipadi <venki@google.com> --- include/linux/pm_qos.h | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h index 83b0ea3..84c99f4 100644 --- a/include/linux/pm_qos.h +++ b/include/linux/pm_qos.h @@ -107,7 +107,19 @@ static inline void pm_qos_remove_request(struct pm_qos_request *req) { return; } static inline int pm_qos_request(int pm_qos_class) - { return 0; } +{ + switch (pm_qos_class) { + case PM_QOS_CPU_DMA_LATENCY: + return PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; + case PM_QOS_NETWORK_LATENCY: + return PM_QOS_NETWORK_LAT_DEFAULT_VALUE; + case PM_QOS_NETWORK_LATENCY: + return PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE; + default: + return PM_QOS_DEFAULT_VALUE; + } +} + static inline int pm_qos_add_notifier(int pm_qos_class, struct notifier_block *notifier) { return 0; } -- 1.7.7.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] CPU C-state breakage with PM Qos change 2012-02-02 23:57 [PATCH] CPU C-state breakage with PM Qos change Venkatesh Pallipadi @ 2012-02-03 0:14 ` Venkatesh Pallipadi 2012-02-03 14:04 ` Pihet-XID, Jean 0 siblings, 1 reply; 10+ messages in thread From: Venkatesh Pallipadi @ 2012-02-03 0:14 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Kevin Hilman, Jean Pihet, markgross, linux-kernel, Len Brown, Venkatesh Pallipadi 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. Signed-off-by: Venkatesh Pallipadi <venki@google.com> --- include/linux/pm_qos.h | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h index 83b0ea3..8a0ede4 100644 --- a/include/linux/pm_qos.h +++ b/include/linux/pm_qos.h @@ -107,7 +107,19 @@ static inline void pm_qos_remove_request(struct pm_qos_request *req) { return; } static inline int pm_qos_request(int pm_qos_class) - { return 0; } +{ + switch (pm_qos_class) { + case PM_QOS_CPU_DMA_LATENCY: + return PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; + case PM_QOS_NETWORK_LATENCY: + return PM_QOS_NETWORK_LAT_DEFAULT_VALUE; + case PM_QOS_NETWORK_THROUGHPUT: + return PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE; + default: + return PM_QOS_DEFAULT_VALUE; + } +} + static inline int pm_qos_add_notifier(int pm_qos_class, struct notifier_block *notifier) { return 0; } -- 1.7.7.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] CPU C-state breakage with PM Qos change 2012-02-03 0:14 ` Venkatesh Pallipadi @ 2012-02-03 14:04 ` Pihet-XID, Jean 2012-02-03 20:02 ` Rafael J. Wysocki 2012-02-05 3:50 ` mark gross 0 siblings, 2 replies; 10+ messages in thread From: Pihet-XID, Jean @ 2012-02-03 14:04 UTC (permalink / raw) To: Venkatesh Pallipadi, Rafael J. Wysocki, markgross Cc: Kevin Hilman, linux-kernel, Len Brown, Linux PM mailing list Looping in linux-pm On Fri, Feb 3, 2012 at 1:14 AM, Venkatesh Pallipadi <venki@google.com> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] CPU C-state breakage with PM Qos change 2012-02-03 14:04 ` Pihet-XID, Jean @ 2012-02-03 20:02 ` Rafael J. Wysocki 2012-02-05 3:50 ` mark gross 1 sibling, 0 replies; 10+ messages in thread From: Rafael J. Wysocki @ 2012-02-03 20:02 UTC (permalink / raw) To: Pihet-XID, Jean Cc: Venkatesh Pallipadi, markgross, Kevin Hilman, linux-kernel, Len Brown, Linux PM mailing list On Friday, February 03, 2012, Pihet-XID, Jean wrote: > Looping in linux-pm > > On Fri, Feb 3, 2012 at 1:14 AM, Venkatesh Pallipadi <venki@google.com> 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? 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] CPU C-state breakage with PM Qos change 2012-02-03 14:04 ` Pihet-XID, Jean 2012-02-03 20:02 ` Rafael J. Wysocki @ 2012-02-05 3:50 ` mark gross 2012-02-05 11:04 ` Rafael J. Wysocki 1 sibling, 1 reply; 10+ messages in thread From: mark gross @ 2012-02-05 3:50 UTC (permalink / raw) To: Pihet-XID, Jean Cc: Venkatesh Pallipadi, Rafael J. Wysocki, markgross, Kevin Hilman, linux-kernel, Len Brown, Linux PM mailing list 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 <venki@google.com> 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? 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] CPU C-state breakage with PM Qos change 2012-02-05 3:50 ` mark gross @ 2012-02-05 11:04 ` Rafael J. Wysocki 2012-02-06 10:18 ` [linux-pm] " Jean Pihet 0 siblings, 1 reply; 10+ messages in thread From: Rafael J. Wysocki @ 2012-02-05 11:04 UTC (permalink / raw) To: markgross Cc: Pihet-XID, Jean, Venkatesh Pallipadi, Kevin Hilman, linux-kernel, Len Brown, Linux PM mailing list On Sunday, February 05, 2012, mark gross wrote: > 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 <venki@google.com> 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? > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [linux-pm] [PATCH] CPU C-state breakage with PM Qos change 2012-02-05 11:04 ` Rafael J. Wysocki @ 2012-02-06 10:18 ` Jean Pihet 2012-02-06 16:42 ` Jean Pihet 0 siblings, 1 reply; 10+ messages in thread From: Jean Pihet @ 2012-02-06 10:18 UTC (permalink / raw) To: Rafael J. Wysocki, markgross Cc: Kevin Hilman, Venkatesh Pallipadi, linux-kernel, Linux PM mailing list, Pihet-XID, Jean Hi Rafael, Mark, On Sun, Feb 5, 2012 at 12:04 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Sunday, February 05, 2012, mark gross wrote: >> 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 <venki@google.com> 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? >> >> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [linux-pm] [PATCH] CPU C-state breakage with PM Qos change 2012-02-06 10:18 ` [linux-pm] " Jean Pihet @ 2012-02-06 16:42 ` Jean Pihet 2012-02-06 20:16 ` Rafael J. Wysocki 0 siblings, 1 reply; 10+ messages in thread From: Jean Pihet @ 2012-02-06 16:42 UTC (permalink / raw) To: Rafael J. Wysocki Cc: markgross, Kevin Hilman, Venkatesh Pallipadi, linux-kernel, Linux PM mailing list, Pihet-XID, Jean Rafael, On Mon, Feb 6, 2012 at 11:18 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote: > Hi Rafael, Mark, > > On Sun, Feb 5, 2012 at 12:04 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: ... >> 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. 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 >> >> Thanks, >> Rafael ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [linux-pm] [PATCH] CPU C-state breakage with PM Qos change 2012-02-06 16:42 ` Jean Pihet @ 2012-02-06 20:16 ` Rafael J. Wysocki 2012-02-07 8:39 ` Jean Pihet 0 siblings, 1 reply; 10+ messages in thread From: Rafael J. Wysocki @ 2012-02-06 20:16 UTC (permalink / raw) To: Jean Pihet Cc: markgross, Kevin Hilman, Venkatesh Pallipadi, linux-kernel, Linux PM mailing list, Pihet-XID, Jean On Monday, February 06, 2012, Jean Pihet wrote: > Rafael, > > On Mon, Feb 6, 2012 at 11:18 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote: > > Hi Rafael, Mark, > > > > On Sun, Feb 5, 2012 at 12:04 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > ... > > >> 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. > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [linux-pm] [PATCH] CPU C-state breakage with PM Qos change 2012-02-06 20:16 ` Rafael J. Wysocki @ 2012-02-07 8:39 ` Jean Pihet 0 siblings, 0 replies; 10+ messages in thread From: Jean Pihet @ 2012-02-07 8:39 UTC (permalink / raw) To: Rafael J. Wysocki, Venkatesh Pallipadi Cc: markgross, Kevin Hilman, linux-kernel, Linux PM mailing list, Pihet-XID, Jean On Mon, Feb 6, 2012 at 9:16 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Monday, February 06, 2012, Jean Pihet wrote: >> Rafael, >> >> On Mon, Feb 6, 2012 at 11:18 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote: >> > Hi Rafael, Mark, >> > >> > On Sun, Feb 5, 2012 at 12:04 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> ... >> >> >> 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. >> >> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-02-07 8:39 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-02-02 23:57 [PATCH] CPU C-state breakage with PM Qos change Venkatesh Pallipadi 2012-02-03 0:14 ` Venkatesh Pallipadi 2012-02-03 14:04 ` Pihet-XID, Jean 2012-02-03 20:02 ` Rafael J. Wysocki 2012-02-05 3:50 ` mark gross 2012-02-05 11:04 ` Rafael J. Wysocki 2012-02-06 10:18 ` [linux-pm] " Jean Pihet 2012-02-06 16:42 ` Jean Pihet 2012-02-06 20:16 ` Rafael J. Wysocki 2012-02-07 8:39 ` Jean Pihet
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).