Date: Fri, 14 Jul 2017 17:38:18 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: "Li, Aubrey" <aubrey.li@linux.intel.com>
Cc: Andi Kleen <ak@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: <20170714153818.pjauqxebxyhs6ljp@hirez.programming.kicks-ass.net>
References: <20170711163422.etydkhhtgfthpfi5@hirez.programming.kicks-ass.net>
 <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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <31170ac6-9db1-f0b8-4841-f1661c8ed6e1@linux.intel.com>
User-Agent: NeoMutt/20170609 (1.8.3)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1867
Lines: 45

On Fri, Jul 14, 2017 at 11:56:33AM +0800, Li, Aubrey wrote:
> On 2017/7/14 2:28, Peter Zijlstra wrote:
> > On Thu, Jul 13, 2017 at 11:13:28PM +0800, Li, Aubrey wrote:
> >> On 2017/7/13 22:53, Peter Zijlstra wrote:
> > 
> >>> Fixing C-state selection by creating an alternative idle path sounds so
> >>> very wrong.
> >>
> >> This only happens on the arch which has multiple hardware idle cstates, like
> >> Intel's processor. As long as we want to support multiple cstates, we have to
> >> make a selection(with cost of timestamp update and computation). That's fine
> >> in the normal idle path, but if we want a fast idle switch, we can make a 
> >> tradeoff to use a low-latency one directly, that's why I proposed a fast idle
> >> path, so that we don't need to mix fast idle condition judgement in both idle
> >> entry and idle exit path.
> > 
> > That doesn't make sense. If you can decide to pick a shallow C state in
> > any way, you can fix the general selection too.
> > 
> 
> Okay, maybe something like the following make sense? Give a hint to
> cpuidle_idle_call() to indicate a fast idle.
> 
> --------------------------------------------------------
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index ef63adc..3165e99 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -152,7 +152,7 @@ static void cpuidle_idle_call(void)
> 	 */
> 	rcu_idle_enter();
>  
> -	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.