From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932970AbdBVPEZ (ORCPT ); Wed, 22 Feb 2017 10:04:25 -0500 Received: from mout.gmx.net ([212.227.17.20]:60167 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932754AbdBVPER (ORCPT ); Wed, 22 Feb 2017 10:04:17 -0500 Message-ID: <1487775823.4487.23.camel@gmx.de> Subject: Re: 9908859acaa9 cpuidle/menu: add per CPU PM QoS resume latency consideration From: Mike Galbraith To: Alex Shi , Peter Zijlstra Cc: LKML , "rafael.j.wysocki@intel.com" Date: Wed, 22 Feb 2017 16:03:43 +0100 In-Reply-To: <08cafc1b-3e0f-ab48-da45-8ba591379635@linaro.org> References: <1487768197.27533.5.camel@gmx.de> <20170222131218.GS6515@twins.programming.kicks-ass.net> <08cafc1b-3e0f-ab48-da45-8ba591379635@linaro.org> Content-Type: text/plain; charset="us-ascii" X-Mailer: Evolution 3.16.5 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:wRL4e0eObjFtjMfpE6qNWIH0SUg19CtpERhW1oV8h8pj+wHsAH8 hNz/zr3M4/ke8gVpgk7nQ7zCvTDBzt4LIckmCW+t0f+JTKEKRo0cVn2/5TGOn5vhrAUtA9c iZxWlrWQkzaCT5LCc2gy+wCzTwZX+i234Uc9Baq4NM6XdxdQx0AzqAbl9lSjuIFuT377nSO XXq7GY67aIg++L/60dQAA== X-UI-Out-Filterresults: notjunk:1;V01:K0:NufoLMS9MpE=:uxMaa9JJqbj5mfpIq8XFB9 PBw92a7lI6Hm+sW7jWj5HBFWk4PsC/SXQxohV6ujAoGqrsVUIOtZ99RRxSBu/c7Tkw+LSYSf9 +EZr6w/KMDYpeb0KoFGsUByon0qn/dK1cAJbatyT9PTuWdZJ8eG+ikDuFk2K4Daa2u6HxIOv4 Q2/LzGqRcuACHFp67r5E48VquYk2qXpioFvk5/nNrfzksJ9tp4DryUeEgo0LnR4KzBPhFC/GA fFjxRGz0+NHeDwlRSkllhmyCyQR/Y9ibA3NiC7LrlgQXs4925cPI3EeQr5B1kvM7y5dDigif8 gbBDygvkNL0CgNAqw8TJ1+pHMZnHdCORpbf/wrAJI+TPGYlwmxIjiEPpaMl2kt7AbKgPAc9Xf qWiEqPIdkBRjgIr2tNZ0P8CWiROo0Chcza/JjlFZhJP+0X96DXGDwdUdzvV3Ce0KZOrrf4Zum jkEtlEy6lbdCAdlGjaO05sydTEe5zgGeCto/7UR0X14S2pfifZud/hgj/hfh8/JVeam2WJxLg +Mkk50t7KEgckMIXKoehfd81GcOncsiyjB90S4SnamCiWHChQp0k45Bg9K73aKTeULV4suwyl x0BlE/tdPYnnp7Lp60keBzKzKwAiHpvWTNSrOyvBVYfnrrriiBCVR5ZGavQQRqmBV+zaST5+c qvDA39NZQM/QlQJNBKD7/FGVajKVQNGSrDZoXjsUXasB4P6dgavF1WRH1H9A+/EKLQBj4QyjH xea0gFYuBtDGEuPWh4s9VMAJOSNnTQtrDoelUsjUDwpTt2VFN3pf+gPGKcD/Me7ac7M98U9u3 6+4X7/0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-02-22 at 22:53 +0800, Alex Shi wrote: > 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? That's what I was gonna do, but then figured RT users will take full control when it really matters, so took the zero added cycles option for RT instead. > =========== > 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);