Messages in this thread | | | Subject | Re: 9908859acaa9 cpuidle/menu: add per CPU PM QoS resume latency consideration | From | Mike Galbraith <> | Date | Wed, 22 Feb 2017 16:03:43 +0100 |
| |
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);
| |