RE: [RFC/RFT][PATCH 0/7] sched/cpuidle: Idle loop rework

RE: [RFC/RFT][PATCH 0/7] sched/cpuidle: Idle loop rework

[Doug Smythies]

On 2018.03.04 14:22 Rafael J. Wysocki wrote:

...[snip]...

> I have tested these patches on a couple of machines, including the very laptop
> I'm sending them from, without any obvious issues, but please give them a go
> if you can, especially if you have an easy way to reproduce the problem they
> are targeting.  The patches are on top of 4.16-rc3 (if you need a git branch
> with them for easier testing, please let me know).

I am having some troubles with the patch set.
My test server takes anywhere between nominal (about 30 seconds) and 10X nominal
(about 5 minutes) time to boot.
I have booted 5 times:

1. took about 5 minutes to boot. No test data acquired. Kern.log excerpt:
	[    5.760740] Btrfs loaded, crc32c=crc32c-intel
	[    5.779522] random: crng init done
	[  336.185559] EXT4-fs (sda1): mounted filesystem with ordered data mode. Opts: (null)
	[  346.823627] Loading iSCSI transport class v2.0-870.

2. took about 5 minutes to boot. Minimal test data acquired. Kern.log excerpt:
	[    8.963959] async_tx: api initialized (async)
	[    9.001124] Btrfs loaded, crc32c=crc32c-intel
	[    9.024652] random: crng init done
	[  332.768025] clocksource: timekeeping watchdog on
			 CPU3: Marking clocksource 'tsc' as unstable because the
			 skew is too large:
	[  332.768128] clocksource: 'hpet' wd_now: 1c082869 wd_last:
			 a5053b0 mask: ffffffff
	[  332.768129] clocksource: 'tsc' cs_now: 11a0d3a34be cs_last:
			 1b534f1252 mask: ffffffffffffffff
	[  332.768129] tsc: Marking TSC unstable due to clocksource watchdog
	[  332.768145] TSC found unstable after boot, most likely due to
			 broken BIOS. Use 'tsc=unstable'.	

	Observe 25.14 average processor package watts. 100% load on CPU 7.
	Test Duration 2.5 hours.
	Reference: K4.16-rc3: Not done yet.
	Reference: K4.15-rc1: 27.34 Watts
	Reference: K4.15-rc1, idle states 0-3 disabled: 23.92 Watts
	Reference: This kernel, idle states 0-3 disabled: ~23.65 Watts

3. crashed during boot. No useful log info.

4. booted kernel 4.16-rc3 stock. Booted normally in about 30 seconds.

5. took about 30 seconds to boot. Minimal test data acquired.
	Observe 24.77 average processor package watts. 100% load on CPU 7.
	Test duration 15 hours. Peak power 32.8 watts.
	There is a graph. I'll send it off-list.

Oh, I see a version 2 of the patch set. I'll move on to version 2.

... Doug

[RFC/RFT][PATCH 0/7] sched/cpuidle: Idle loop rework

[Rafael J. Wysocki]

Hi All,

The problem is that if we stop the sched tick in
tick_nohz_idle_enter() and then the idle governor predicts short idle
duration, we lose regardless of whether or not it is right.

If it is right, we've lost already, because we stopped the tick
unnecessarily.  If it is not right, we'll lose going forward, because
the idle state selected by the governor is going to be too shallow and
we'll draw too much power (that has been reported recently to actually
happen often enough for people to care).

This patch series is an attempt to improve the situation and the idea
here is to make the decision whether or not to stop the tick deeper in
the idle loop and in particular after running the idle state selection
in the path where the idle governor is invoked.  This way the problem
can be avoided, because the idle duration predicted by the idle governor
can be used to decide whether or not to stop the tick so that the tick
is only stopped if that value is large enough (and, consequently, the
idle state selected by the governor is deep enough).

The series tires to avoid adding too much new code, rather reorder the
existing code and make it more fine-grained.

Patch 1 prepares the tick-sched code for the subsequent modifications and it
doesn't change the code's functionality (at least not intentionally).

Patch 2 starts pushing the tick stopping decision deeper into the idle
loop, but it is limited to do_idle() and tick_nohz_irq_exit().

Patch 3 makes cpuidle_idle_call() decide whether or not to stop the tick
and sets the stage for the changes in patch 6.

Patch 4 splits menu_select() into idle duration prediction and idle state
selection parts.

Patch 5 adds the ->predict callback to struct cpuidle_governor and hooks
it up to the menu governor's idle duration prediction routine introduced
by patch 4.  It also changes cpuidle_select() to return the expected idle
duration in addition to the target state index.

Patch 6 reorders idle duration prediction by the governor and idle state
selection with respect to the stopping of the tick and causes the predicted
idle duration to be used for deciding whether or not to stop the tick.

Patch 7 cleans up the code to avoid running one piece of it twice in a row
in some cases.

I have tested these patches on a couple of machines, including the very laptop
I'm sending them from, without any obvious issues, but please give them a go
if you can, especially if you have an easy way to reproduce the problem they
are targeting.  The patches are on top of 4.16-rc3 (if you need a git branch
with them for easier testing, please let me know).

The above said, this is just RFC, so no pets close to the machines running it,
please, and I'm kind of expecting Peter and Thomas to tear it into pieces. :-)

Thanks,
Rafael