Date: Fri, 14 Jul 2017 08:53:56 -0700
From: Andi Kleen <ak@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Li, Aubrey" <aubrey.li@linux.intel.com>,
        Frederic Weisbecker <fweisbec@gmail.com>,
        Christoph Lameter <cl@linux.com>, Aubrey Li <aubrey.li@intel.com>,
        tglx@linutronix.de, len.brown@intel.com, rjw@rjwysocki.net,
        tim.c.chen@linux.intel.com, arjan@linux.intel.com,
        paulmck@linux.vnet.ibm.com, yang.zhang.wz@gmail.com, x86@kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods
Message-ID: <20170714155356.GH3441@tassilo.jf.intel.com>
References: <496d4921-5768-cd1e-654b-38630b7d2e13@linux.intel.com>
 <20170712083410.ualmvnvzoohyami5@hirez.programming.kicks-ass.net>
 <20170712213240.GE3441@tassilo.jf.intel.com>
 <20170713083649.febfflfl5hafkko5@hirez.programming.kicks-ass.net>
 <16e12e23-6b28-f174-7c4b-4d719225cd3b@linux.intel.com>
 <20170713145311.z4zxlyd2dospeoqg@hirez.programming.kicks-ass.net>
 <4a577bd6-20b1-abb6-2153-f9870f0a721e@linux.intel.com>
 <20170713182820.sn3fjitnd3mca27p@hirez.programming.kicks-ass.net>
 <31170ac6-9db1-f0b8-4841-f1661c8ed6e1@linux.intel.com>
 <20170714153818.pjauqxebxyhs6ljp@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170714153818.pjauqxebxyhs6ljp@hirez.programming.kicks-ass.net>
User-Agent: Mutt/1.8.0 (2017-02-23)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1245
Lines: 33

> > -	if (cpuidle_not_available(drv, dev)) {
> > +	if (cpuidle_not_available(drv, dev) || this_is_a_fast_idle) {
> > 		default_idle_call();
> > 		goto exit_idle;
> > 	}
> 
> No, that's wrong. We want to fix the normal C state selection process to
> pick the right C state.
> 
> The fast-idle criteria could cut off a whole bunch of available C
> states. We need to understand why our current C state pick is wrong and
> amend the algorithm to do better. Not just bolt something on the side.

Fast idle uses the same predictor as the current C state governor.

The only difference is that it uses a different threshold for C1.
Likely that's the cause. If it was using the same threshold the
decision would be the same.

The thresholds are coming either from the tables in intel idle,
or from ACPI (let's assume the first)

That means either: the intel idle C1 threshold on the system Aubrey
tested on is too high, or the fast idle threshold is too low.

But that would be only true for the workload he tested.
It may well be that it's not that great for another.

The numbers in the standard intel_idle are reasonably tested
with many workloads, so I guess it would be safer to pick that one.
Unless someone wants to revisit these tables.

-Andi