From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932824AbdBVOxm (ORCPT ); Wed, 22 Feb 2017 09:53:42 -0500 Received: from mail-pg0-f54.google.com ([74.125.83.54]:35164 "EHLO mail-pg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932369AbdBVOxe (ORCPT ); Wed, 22 Feb 2017 09:53:34 -0500 Subject: Re: 9908859acaa9 cpuidle/menu: add per CPU PM QoS resume latency consideration To: Peter Zijlstra , Mike Galbraith References: <1487768197.27533.5.camel@gmx.de> <20170222131218.GS6515@twins.programming.kicks-ass.net> From: Alex Shi Cc: LKML , "rafael.j.wysocki@intel.com" Message-ID: <08cafc1b-3e0f-ab48-da45-8ba591379635@linaro.org> Date: Wed, 22 Feb 2017 22:53:29 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20170222131218.GS6515@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org cc Rafael. On 02/22/2017 09:12 PM, Peter Zijlstra wrote: > On Wed, Feb 22, 2017 at 01:56:37PM +0100, Mike Galbraith wrote: >> Hi, >> >> Do we really need a spinlock for that in the idle loop? > > Urgh, that's broken on RT, you cannot schedule the idle loop. > > Also, yeah, reading a s32 should not need no locking, but there's a > bunch of pointer chases in between :/ Do you mean s/should not/should/ ? :) Yes, the dev_pm_qos_read_value() using a power.lock, that is right for normal device. But as to this cpu here, the lock isn't necessary. Hi Rafael, Is this fix ok? =========== diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 8d6d25c..957c56d 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_this_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_this_cpu_resume_latency(dev->cpu); if (data->needs_update) { menu_update(drv, dev);