From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Thomas Ilsche <thomas.ilsche@tu-dresden.de>
Cc: Peter Zijlstra <peterz@infradead.org>,
Linux PM <linux-pm@vger.kernel.org>,
Frederic Weisbecker <fweisbec@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>,
Paul McKenney <paulmck@linux.vnet.ibm.com>,
Doug Smythies <dsmythies@telus.net>,
Rik van Riel <riel@surriel.com>,
Aubrey Li <aubrey.li@linux.intel.com>,
Mike Galbraith <mgalbraith@suse.de>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFT][PATCH v7 6/8] sched: idle: Select idle state before stopping the tick
Date: Wed, 28 Mar 2018 00:10:37 +0200 [thread overview]
Message-ID: <4198010.6ArFqS34NK@aspire.rjw.lan> (raw)
In-Reply-To: <6462e44a-e207-6b97-22bf-ad4aed69afc2@tu-dresden.de>
On Tuesday, March 27, 2018 11:50:02 PM CEST Thomas Ilsche wrote:
> On 2018-03-20 16:45, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > In order to address the issue with short idle duration predictions
> > by the idle governor after the tick has been stopped, reorder the
> > code in cpuidle_idle_call() so that the governor idle state selection
> > runs before tick_nohz_idle_go_idle() and use the "nohz" hint returned
> > by cpuidle_select() to decide whether or not to stop the tick.
> >
> > This isn't straightforward, because menu_select() invokes
> > tick_nohz_get_sleep_length() to get the time to the next timer
> > event and the number returned by the latter comes from
> > __tick_nohz_idle_enter(). Fortunately, however, it is possible
> > to compute that number without actually stopping the tick and with
> > the help of the existing code.
>
> I think something is wrong with the new tick_nohz_get_sleep_length.
> It seems to return a value that is too large, ignoring immanent
> non-sched timer.
That's a very useful hint, let me have a look.
> I tested idle-loop-v7.3. It looks very similar to my previous results
> on the first idle-loop-git-version [1]. Idle and traditional synthetic
> powernightmares are mostly good.
OK
> But it selects too deep C-states for short idle periods, which is bad
> for power consumption [2].
That still needs to be improved, then.
> I tracked this down with additional tests using
> __attribute__((optimize("O0"))) menu_select
> and perf probe. With this the behavior seems slightly different, but it
> shows that data->next_timer_us is:
> v4.16-rc6: the expected ~500 us [3]
> idle-loop-v7.3: many milliseconds to minutes [4].
> This leads to the governor to wrongly selecting C6.
>
> Checking with 372be9e and 6ea0577, I can confirm that the change is
> introduced by this patch.
Yes, that's where the most intrusive reordering happens.
Thanks for the feedback!
next prev parent reply other threads:[~2018-03-27 22:10 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-20 15:12 [RFT][PATCH v7 0/8] sched/cpuidle: Idle loop rework Rafael J. Wysocki
2018-03-20 15:13 ` [RFT][PATCH v7 1/8] time: tick-sched: Reorganize idle tick management code Rafael J. Wysocki
2018-03-20 15:15 ` [RFT][PATCH v7 2/8] sched: idle: Do not stop the tick upfront in the idle loop Rafael J. Wysocki
2018-03-20 15:15 ` [RFT][PATCH v7 3/8] " Rafael J. Wysocki
2018-03-20 18:00 ` [Correction][RFT][PATCH v7 3/8] sched: idle: Do not stop the tick before cpuidle_idle_call() Rafael J. Wysocki
2018-03-20 15:16 ` [RFT][PATCH v7 4/8] jiffies: Introduce USER_TICK_USEC and redefine TICK_USEC Rafael J. Wysocki
2018-03-20 15:45 ` [RFT][PATCH v7 5/8] cpuidle: Return nohz hint from cpuidle_select() Rafael J. Wysocki
2018-03-21 6:48 ` [RFT][PATCH v7.1 " Rafael J. Wysocki
2018-03-21 11:52 ` Rafael J. Wysocki
2018-03-21 13:03 ` [RFT][PATCH v7.2 " Rafael J. Wysocki
2018-03-21 14:36 ` [RFT][PATCH v7 " Rafael J. Wysocki
2018-03-21 17:59 ` Thomas Ilsche
2018-03-21 22:15 ` Rafael J. Wysocki
2018-03-22 13:18 ` Thomas Ilsche
2018-03-22 17:23 ` Rafael J. Wysocki
2018-03-22 6:24 ` Doug Smythies
2018-03-22 15:41 ` Doug Smythies
2018-03-22 17:21 ` Rafael J. Wysocki
2018-03-21 18:23 ` Doug Smythies
2018-03-22 17:40 ` [RFT][PATCH v7.3 " Rafael J. Wysocki
2018-03-28 9:14 ` Thomas Ilsche
2018-03-30 9:39 ` Rafael J. Wysocki
2018-04-10 15:22 ` Thomas Ilsche
2018-03-22 20:46 ` Doug Smythies
2018-03-20 15:45 ` [RFT][PATCH v7 6/8] sched: idle: Select idle state before stopping the tick Rafael J. Wysocki
2018-03-27 21:50 ` Thomas Ilsche
2018-03-27 22:10 ` Rafael J. Wysocki [this message]
2018-03-28 8:13 ` Rafael J. Wysocki
2018-03-28 8:38 ` Thomas Ilsche
2018-03-28 10:37 ` Rafael J. Wysocki
2018-03-28 10:56 ` Rafael J. Wysocki
2018-03-28 15:15 ` Thomas Ilsche
2018-03-28 20:41 ` Doug Smythies
2018-03-28 23:11 ` Rafael J. Wysocki
2018-03-20 15:46 ` [RFT][PATCH v7 7/8] cpuidle: menu: Refine idle state selection for running tick Rafael J. Wysocki
2018-03-20 15:47 ` [RFT][PATCH v7 8/8] cpuidle: menu: Avoid selecting shallow states with stopped tick Rafael J. Wysocki
2018-03-20 17:52 ` [RFT][PATCH v7 3/8] sched: idle: Do not stop the tick upfront in the idle loop Doug Smythies
2018-03-20 18:01 ` Rafael J. Wysocki
2018-03-21 12:31 ` [RFT][PATCH v7 0/8] sched/cpuidle: Idle loop rework Rik van Riel
2018-03-21 13:55 ` Rafael J. Wysocki
2018-03-21 14:53 ` Rik van Riel
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=4198010.6ArFqS34NK@aspire.rjw.lan \
--to=rjw@rjwysocki.net \
--cc=aubrey.li@linux.intel.com \
--cc=dsmythies@telus.net \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mgalbraith@suse.de \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=riel@surriel.com \
--cc=tglx@linutronix.de \
--cc=thomas.ilsche@tu-dresden.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).