linux-kernel - Re: 9908859acaa9 cpuidle/menu: add per CPU PM QoS resume latency consideration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e8261472-4338-eab2-98e1-5dd6039899f7@linaro.org>
Date:   Thu, 23 Feb 2017 21:55:17 +0800
From:   Alex Shi <alex.shi@...aro.org>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     Mike Galbraith <efault@....de>,
        LKML <linux-kernel@...r.kernel.org>,
        Rik van Riel <riel@...hat.com>
Subject: Re: 9908859acaa9 cpuidle/menu: add per CPU PM QoS resume latency
 consideration



On 02/23/2017 08:15 PM, Rafael J. Wysocki wrote:
> On Wednesday, February 22, 2017 10:55:04 PM Alex Shi wrote:
>>>
>>> Its not hard; spinlock_t ends up being a mutex, and this is ran from the
>>> idle thread. What thread do you think we ought to run when we block
>>> idle?
>>>
>>
>> Straight right.
>> Thanks for explanations! :)
> 
> I overlooked that, sorry.
> 
> Shall we revert?
> 
> I don't want RT to be broken because of this.
> 

The RT kernel had a fix already. :)
This feature is very useful to save power of cpu. We could have a better fix
to keep this feature and good for RT.


>From cfe82c555628f7197ecd91d3b8092a98b34a371a Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@...aro.org>
Date: Thu, 23 Feb 2017 21:27:09 +0800
Subject: [PATCH] cpuidle/menu: skip lock in per cpu resume latency reading

dev_pm_qos_read_value using a lock to proctect the concurrent device
latency reading, that is useful for multiple cpu access a global device.
But it's not necessary for a per cpu data reading here. And furthermore,
RT kernel using mutex to replace spinlock causes fake panic here.

So, skip the lock using is nice for this per cpu value reading.

Signed-off-by: Alex Shi <alex.shi@...aro.org>
---
 drivers/cpuidle/governors/menu.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 8d6d25c..b852d99 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -273,6 +273,14 @@ static unsigned int get_typical_interval(struct menu_device *data)
 	goto again;
 }
 
+int read_per_cpu_resume_latency(int cpu)
+{
+	struct device *dev = get_cpu_device(cpu);
+
+	return IS_ERR_OR_NULL(dev->power.qos) ?
+		0 : pm_qos_read_value(&dev->power.qos->resume_latency);
+}
+
 /**
  * menu_select - selects the next idle state to enter
  * @drv: cpuidle driver containing state data
@@ -281,13 +289,12 @@ static unsigned int get_typical_interval(struct menu_device *data)
 static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
 {
 	struct menu_device *data = this_cpu_ptr(&menu_devices);
-	struct device *device = get_cpu_device(dev->cpu);
 	int latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
 	int i;
 	unsigned int interactivity_req;
 	unsigned int expected_interval;
 	unsigned long nr_iowaiters, cpu_load;
-	int resume_latency = dev_pm_qos_read_value(device);
+	int resume_latency = read_per_cpu_resume_latency(dev->cpu);
 
 	if (data->needs_update) {
 		menu_update(drv, dev);
-- 
2.8.1.101.g72d917a