LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Rafael J. Wysocki" <rafael@kernel.org>
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 10:13:38 +0200
Message-ID: <CAJZ5v0j5mEaQSAiW4ynitM1ee=sqkpc8G_tZ40RYUgyavXXhqQ@mail.gmail.com> (raw)
In-Reply-To: <4198010.6ArFqS34NK@aspire.rjw.lan>

On Wed, Mar 28, 2018 at 12:10 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> 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.

Overall, this is an interesting conundrum, because the case in
question is when the tick should never be stopped at all during the
workload and the code's behavior in that case should not change, so
the change was not intentional.

Now, from walking through the code, as long as can_stop_idle_tick()
returns 'true' all should be fine or at least I don't see why there is
any difference in behavior in that case.

However, if can_stop_idle_tick() returns 'false' (for example, because
need_resched() returns 'true' when it is evaluated), the behavior *is*
different in a couple of ways.  I sort of know how that can be
addressed, but I'd like to reproduce your results here.

Are you still using the same workload as before to trigger this behavior?

  reply index

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
2018-03-28  8:13       ` Rafael J. Wysocki [this message]
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 publically 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='CAJZ5v0j5mEaQSAiW4ynitM1ee=sqkpc8G_tZ40RYUgyavXXhqQ@mail.gmail.com' \
    --to=rafael@kernel.org \
    --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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox