Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1552061 > unrolled thread

[PATCH 2/3] cpu: expose pm_qos_resume_latency for each cpu

Started byAlex Shi <alex.shi@linaro.org>
First post2017-01-05 16:40 +0100
Last post2017-01-12 14:20 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 2/3] cpu: expose pm_qos_resume_latency for each cpu Alex Shi <alex.shi@linaro.org> - 2017-01-05 16:40 +0100
    Re: [PATCH 2/3] cpu: expose pm_qos_resume_latency for each cpu Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-01-11 09:20 +0100
      Re: [PATCH 2/3] cpu: expose pm_qos_resume_latency for each cpu Alex Shi <alex.shi@linaro.org> - 2017-01-11 15:40 +0100
        Re: [PATCH 2/3] cpu: expose pm_qos_resume_latency for each cpu "Rafael J. Wysocki" <rafael@kernel.org> - 2017-01-11 19:50 +0100
          Re: [PATCH 2/3] cpu: expose pm_qos_resume_latency for each cpu Alex Shi <alex.shi@linaro.org> - 2017-01-12 14:20 +0100

#1552061 — [PATCH 2/3] cpu: expose pm_qos_resume_latency for each cpu

FromAlex Shi <alex.shi@linaro.org>
Date2017-01-05 16:40 +0100
Subject[PATCH 2/3] cpu: expose pm_qos_resume_latency for each cpu
Message-ID<sWij0-4TE-23@gated-at.bofh.it>
The cpu-dma PM QoS constraint impacts all the cpus in the system. There
is no way to let the user to choose a PM QoS constraint per cpu.

The following patch exposes to the userspace a per cpu based sysfs file
in order to let the userspace to change the value of the PM QoS latency
constraint.

This change is inoperative in its form and the cpuidle governors have to
take into account the per cpu latency constraint in addition to the
global cpu-dma latency constraint in order to operate properly.

BTW
The pm_qos_resume_latency usage defined in
Documentation/ABI/testing/sysfs-devices-power
The /sys/devices/.../power/pm_qos_resume_latency_us attribute
contains the PM QoS resume latency limit for the given device,
which is the maximum allowed time it can take to resume the
device, after it has been suspended at run time, from a resume
request to the moment the device will be ready to process I/O,
in microseconds.  If it is equal to 0, however, this means that
the PM QoS resume latency may be arbitrary.

Signed-off-by: Alex Shi <alex.shi@linaro.org>
To: linux-kernel@vger.kernel.org
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
---
 drivers/base/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 4c28e1a..29cf3459 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/cpufeature.h>
 #include <linux/tick.h>
+#include <linux/pm_qos.h>
 
 #include "base.h"
 
@@ -376,6 +377,9 @@ int register_cpu(struct cpu *cpu, int num)
 
 	per_cpu(cpu_sys_devices, num) = &cpu->dev;
 	register_cpu_under_node(num, cpu_to_node(num));
+#ifdef CONFIG_CPU_IDLE_GOV_MENU
+	dev_pm_qos_expose_latency_limit(&cpu->dev, 0);
+#endif
 
 	return 0;
 }
-- 
2.8.1.101.g72d917a

[toc] | [next] | [standalone]


#1556268

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2017-01-11 09:20 +0100
Message-ID<sYmit-4wT-7@gated-at.bofh.it>
In reply to#1552061
On Thu, Jan 05, 2017 at 11:29:46PM +0800, Alex Shi wrote:
> The cpu-dma PM QoS constraint impacts all the cpus in the system. There
> is no way to let the user to choose a PM QoS constraint per cpu.
> 
> The following patch exposes to the userspace a per cpu based sysfs file
> in order to let the userspace to change the value of the PM QoS latency
> constraint.
> 
> This change is inoperative in its form and the cpuidle governors have to
> take into account the per cpu latency constraint in addition to the
> global cpu-dma latency constraint in order to operate properly.
> 
> BTW
> The pm_qos_resume_latency usage defined in
> Documentation/ABI/testing/sysfs-devices-power
> The /sys/devices/.../power/pm_qos_resume_latency_us attribute
> contains the PM QoS resume latency limit for the given device,
> which is the maximum allowed time it can take to resume the
> device, after it has been suspended at run time, from a resume
> request to the moment the device will be ready to process I/O,
> in microseconds.  If it is equal to 0, however, this means that
> the PM QoS resume latency may be arbitrary.
> 
> Signed-off-by: Alex Shi <alex.shi@linaro.org>
> To: linux-kernel@vger.kernel.org
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> ---
>  drivers/base/cpu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 4c28e1a..29cf3459 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -17,6 +17,7 @@
>  #include <linux/of.h>
>  #include <linux/cpufeature.h>
>  #include <linux/tick.h>
> +#include <linux/pm_qos.h>
>  
>  #include "base.h"
>  
> @@ -376,6 +377,9 @@ int register_cpu(struct cpu *cpu, int num)
>  
>  	per_cpu(cpu_sys_devices, num) = &cpu->dev;
>  	register_cpu_under_node(num, cpu_to_node(num));
> +#ifdef CONFIG_CPU_IDLE_GOV_MENU
> +	dev_pm_qos_expose_latency_limit(&cpu->dev, 0);
> +#endif

No way to do this without the #ifdef?  That's really not recommended for
.c code :(

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]


#1556527

FromAlex Shi <alex.shi@linaro.org>
Date2017-01-11 15:40 +0100
Message-ID<sYsed-82P-7@gated-at.bofh.it>
In reply to#1556268
>> >  #include "base.h"
>> >  
>> > @@ -376,6 +377,9 @@ int register_cpu(struct cpu *cpu, int num)
>> >  
>> >  	per_cpu(cpu_sys_devices, num) = &cpu->dev;
>> >  	register_cpu_under_node(num, cpu_to_node(num));
>> > +#ifdef CONFIG_CPU_IDLE_GOV_MENU
>> > +	dev_pm_qos_expose_latency_limit(&cpu->dev, 0);
>> > +#endif
> No way to do this without the #ifdef?  That's really not recommended for
> .c code :(
> 

Hi Greg,

Thanks for comments!

The function dev_pm_qos_expose_latency_limit() is null if no CONFIG_PM.
So when CONFIG_PM enabled, may we could consider the cpu idle is also
wanted. In this assumption the #ifdef could be removed. If user want to
use this feature, she/he should understand the feature only work on menu
gov only currently. So consider this, I guess we could remove this
#ifdef. :)

Any different concerns on this?

Regards
Alex

BTW,
Although I did try this patch on other platform, but it clearly other
multi core system, like x86 could also get the same benefit.

[toc] | [prev] | [next] | [standalone]


#1556832

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2017-01-11 19:50 +0100
Message-ID<sYw89-1XP-3@gated-at.bofh.it>
In reply to#1556527
On Wed, Jan 11, 2017 at 3:33 PM, Alex Shi <alex.shi@linaro.org> wrote:
>
>>> >  #include "base.h"
>>> >
>>> > @@ -376,6 +377,9 @@ int register_cpu(struct cpu *cpu, int num)
>>> >
>>> >    per_cpu(cpu_sys_devices, num) = &cpu->dev;
>>> >    register_cpu_under_node(num, cpu_to_node(num));
>>> > +#ifdef CONFIG_CPU_IDLE_GOV_MENU
>>> > +  dev_pm_qos_expose_latency_limit(&cpu->dev, 0);
>>> > +#endif
>> No way to do this without the #ifdef?  That's really not recommended for
>> .c code :(
>>
>
> Hi Greg,
>
> Thanks for comments!
>
> The function dev_pm_qos_expose_latency_limit() is null if no CONFIG_PM.
> So when CONFIG_PM enabled, may we could consider the cpu idle is also
> wanted. In this assumption the #ifdef could be removed. If user want to
> use this feature, she/he should understand the feature only work on menu
> gov only currently. So consider this, I guess we could remove this
> #ifdef. :)

But instead of putting the #ifdef into the function body, you can use
a wrapper function defined to be empty for CONFIG_PM unset.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1557436

FromAlex Shi <alex.shi@linaro.org>
Date2017-01-12 14:20 +0100
Message-ID<sYNsm-4qh-39@gated-at.bofh.it>
In reply to#1556832

On 01/12/2017 02:40 AM, Rafael J. Wysocki wrote:
>> >
>> > Hi Greg,
>> >
>> > Thanks for comments!
>> >
>> > The function dev_pm_qos_expose_latency_limit() is null if no CONFIG_PM.
>> > So when CONFIG_PM enabled, may we could consider the cpu idle is also
>> > wanted. In this assumption the #ifdef could be removed. If user want to
>> > use this feature, she/he should understand the feature only work on menu
>> > gov only currently. So consider this, I guess we could remove this
>> > #ifdef. :)
> But instead of putting the #ifdef into the function body, you can use
> a wrapper function defined to be empty for CONFIG_PM unset.

Thanks Rafael!

The function dev_pm_qos_expose_latency_limit() is empty now when
CONFIG_PM disabled. :)
I will resend the patch without the #ifdef.

Thanks!
Alex

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web