linux-kernel - Re: [PATCH 2/3] cpuidle,menu: use interactivity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPKp9ubg3V0979dGTGi+kjXGiBczCG2+QBGgfrZBKjMgSuhy-A@mail.gmail.com>
Date:	Wed, 13 Jan 2016 17:27:11 +0000
From:	Sudeep Holla <sudeep.holla@....com>
To:	riel@...hat.com, rafael.j.wysocki@...el.com
Cc:	Linux PM list <linux-pm@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>,
	arjan@...ux.intel.com, Len Brown <len.brown@...el.com>,
	daniel.lezcano@...aro.org, Sudeep Holla <sudeep.holla@....com>
Subject: Re: [PATCH 2/3] cpuidle,menu: use interactivity_req to disable polling

Hi Rik,

This change break idle on ARM64(may be on other ARM?) platform.
Sorry for reporting late, but missed to check cpuidle in -next.

On Tue, Nov 3, 2015 at 10:34 PM,  <riel@...hat.com> wrote:
> From: Rik van Riel <riel@...hat.com>
>
> The menu governor carefully figures out how much time we typically
> sleep for an estimated sleep interval, or whether there is a repeating
> pattern going on, and corrects that estimate for the CPU load.
>
> Then it proceeds to ignore that information when determining whether
> or not to consider polling. This is not a big deal on most x86 CPUs,
> which have very low C1 latencies, and the patch should not have any
> effect on those CPUs.
>
> However, certain CPUs (eg. Atom) have much higher C1 latencies, and
> it would be good to not waste performance and power on those CPUs if
> we are expecting a very low wakeup latency.
>
> Disable polling based on the estimated interactivity requirement, not
> on the time to the next timer interrupt.
>
> Signed-off-by: Rik van Riel <riel@...hat.com>
> ---
>  drivers/cpuidle/governors/menu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index ecc242a586c9..b1a55731f921 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -330,7 +330,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>          * We want to default to C1 (hlt), not to busy polling
>          * unless the timer is happening really really soon.
>          */
> -       if (data->next_timer_us > 20 &&
> +       if (interactivity_req > 20 &&

I found that data->predicted_us is gets overwritten in get_typical_interval
when avg computed = 0 which is the case initially on boot when the past
intervals are not yet accumulated.

I just tried a hack and that seem to work and proved what I anticipated
(i.e. interactivity_req = 0). Let me know if you have any clues on how to
solve it ? I can help you getting the change tested.

Regards,
Sudeep

--->8

diff --git i/drivers/cpuidle/governors/menu.c w/drivers/cpuidle/governors/menu.c
index 7b0971d97cc3..7c7f4059705a 100644
--- i/drivers/cpuidle/governors/menu.c
+++ w/drivers/cpuidle/governors/menu.c
@@ -330,7 +330,8 @@ static int menu_select(struct cpuidle_driver *drv,
struct cpuidle_device *dev)
         * We want to default to C1 (hlt), not to busy polling
         * unless the timer is happening really really soon.
         */
-       if (interactivity_req > 20 &&
+       if (((interactivity_req && interactivity_req > 20) ||
+               data->next_timer_us > 20) &&
            !drv->states[CPUIDLE_DRIVER_STATE_START].disabled &&
                dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable == 0)
                data->last_state_idx = CPUIDLE_DRIVER_STATE_START;