Re: [PATCH V3 2/6] sched: idle: cpuidle: Check the latency req before idle

Daniel Lezcano
2014-11-10T15:19:50Z

On 11/10/2014 01:41 PM, Peter Zijlstra wrote:
> On Fri, Nov 07, 2014 at 03:31:23PM +0100, Daniel Lezcano wrote:
>> @@ -216,19 +219,26 @@ static void cpu_idle_loop(void)
>> local_irq_disable();
>> arch_cpu_idle_enter();
>>
>> + latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY);
>> +
>> /*
>> * In poll mode we reenable interrupts and spin.
>> *
>> + * If the latency req is zero, we don't want to
>> + * enter any idle state and we jump to the poll
>> + * function directly
>> + *
>> * Also if we detected in the wakeup from idle
>> * path that the tick broadcast device expired
>> * for us, we don't want to go deep idle as we
>> * know that the IPI is going to arrive right
>> * away
>> */
>> - if (cpu_idle_force_poll || tick_check_broadcast_expired())
>> + if (!latency_req || cpu_idle_force_poll ||
>> + tick_check_broadcast_expired())
>> cpu_idle_poll();
>
> Is this why you wanted that weak poll function?

Not only there, it will be added in the next patchset in the
cpu_idle_call function.

> Should we not instead allow an arch to deal with !latency_req and only
> fall back to this polling if there is no actual way for it to implement
> this better?

All this is to remove the poll idle state from the x86 cpuidle driver in
order to remove the CPUIDLE_DRIVER_STATE_START (this one forces to write
always ugly code in the cpuidle framework).

This poll state introduces the CPUIDLE_DRIVER_STATE_START macro. If you
look at the different governors and the code, you can checkout what kind
of tricks this macro introduces and how that makes the code ugly.

For the sake of what ? Just a small optimization in the menu governor.

I suppose that has been introduce and then evolved on a wrong basis. So
now we have to deal with that.

This patchset provides a first round of cleanup around the poll
function, the next patchset will move the 5us timer optimization from
the menu governor and the last patchset will remove the
CPUIDLE_DRIVER_STATE_START ugly macro.

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |