From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Preeti Murthy <preeti.lkml@gmail.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
Nicolas Pitre <nicolas.pitre@linaro.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Lists linaro-kernel <linaro-kernel@lists.linaro.org>,
patches@linaro.org, Preeti U Murthy <preeti@linux.vnet.ibm.com>
Subject: Re: [PATCH V2 1/5] sched: idle: cpuidle: Check the latency req before idle
Date: Tue, 28 Oct 2014 19:59:19 +0100 [thread overview]
Message-ID: <544FE787.8090108@linaro.org> (raw)
In-Reply-To: <CAM4v1pOg1GFW82WD8b6Vas5xhYQrQtdP1STGxyzYtrBNSa+-Pw@mail.gmail.com>
On 10/28/2014 04:51 AM, Preeti Murthy wrote:
> Hi Daniel,
>
> On Thu, Oct 23, 2014 at 2:31 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>> When the pmqos latency requirement is set to zero that means "poll in all the
>> cases".
>>
>> That is correctly implemented on x86 but not on the other archs.
>>
>> As how is written the code, if the latency request is zero, the governor will
>> return zero, so corresponding, for x86, to the poll function, but for the
>> others arch the default idle function. For example, on ARM this is wait-for-
>> interrupt with a latency of '1', so violating the constraint.
>
> This is not true actually. On PowerPC the idle state 0 has an exit_latency of 0.
>
>>
>> In order to fix that, do the latency requirement check *before* calling the
>> cpuidle framework in order to jump to the poll function without entering
>> cpuidle. That has several benefits:
>
> Doing so actually hurts on PowerPC. Because the idle loop defined for
> idle state 0 is different from what cpu_relax() does in cpu_idle_loop().
> The spinning is more power efficient in the former case. Moreover we also set
> certain register values which indicate an idle cpu. The ppc_runlatch bits
> do precisely this. These register values are being read by some user space
> tools. So we will end up breaking them with this patch
>
> My suggestion is very well keep the latency requirement check in
> kernel/sched/idle.c
> like your doing in this patch. But before jumping to cpu_idle_loop verify if the
> idle state 0 has an exit_latency > 0 in addition to your check on the
> latency_req == 0.
> If not, you can fall through to the regular path of calling into the
> cpuidle driver.
> The scheduler can query the cpuidle_driver structure anyway.
>
> What do you think?
Thanks for reviewing the patch and spotting this.
Wouldn't make sense to create:
void __weak_cpu_idle_poll(void) ?
and override it with your specific poll function ?
--
<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 |
<http://www.linaro.org/linaro-blog/> Blog
next prev parent reply other threads:[~2014-10-28 18:59 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-23 9:01 [PATCH V2 1/5] sched: idle: cpuidle: Check the latency req before idle Daniel Lezcano
2014-10-23 9:01 ` [PATCH V2 2/5] sched: idle: Get the next timer event and pass it the cpuidle framework Daniel Lezcano
2014-10-23 9:01 ` [PATCH V2 3/5] cpuidle: idle: menu: Don't reflect when a state selection failed Daniel Lezcano
2014-10-28 2:01 ` Len Brown
2014-10-28 19:15 ` Daniel Lezcano
2014-10-28 7:01 ` Preeti Murthy
2014-10-28 18:28 ` Daniel Lezcano
2014-10-29 1:44 ` Preeti U Murthy
2014-10-29 16:54 ` Kevin Hilman
2014-10-29 16:54 ` Kevin Hilman
2014-10-29 21:11 ` Rafael J. Wysocki
2014-10-23 9:01 ` [PATCH V2 4/5] cpuidle: menu: Fix the get_typical_interval Daniel Lezcano
2014-10-23 16:43 ` Nicolas Pitre
2014-10-28 2:48 ` Len Brown
2014-10-29 18:15 ` Daniel Lezcano
2014-10-23 9:01 ` [PATCH V2 5/5] cpuidle: menu: Move the update function before its declaration Daniel Lezcano
2014-10-23 16:47 ` Nicolas Pitre
2014-10-28 2:53 ` Len Brown
2014-10-28 3:51 ` [PATCH V2 1/5] sched: idle: cpuidle: Check the latency req before idle Preeti Murthy
2014-10-28 18:59 ` Daniel Lezcano [this message]
2014-10-29 2:01 ` Preeti U Murthy
2014-11-05 14:28 ` Daniel Lezcano
2014-11-06 4:08 ` Preeti U Murthy
2014-11-06 12:27 ` Daniel Lezcano
2014-11-07 4:23 ` Preeti U Murthy
2014-11-06 13:42 ` Daniel Lezcano
2014-11-07 4:29 ` Preeti U Murthy
2014-11-07 9:35 ` Daniel Lezcano
2014-11-05 21:57 ` Rafael J. Wysocki
2014-11-05 21:41 ` Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=544FE787.8090108@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=nicolas.pitre@linaro.org \
--cc=patches@linaro.org \
--cc=peterz@infradead.org \
--cc=preeti.lkml@gmail.com \
--cc=preeti@linux.vnet.ibm.com \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.