From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751818AbcF0BN7 (ORCPT ); Sun, 26 Jun 2016 21:13:59 -0400 Received: from mail-yw0-f169.google.com ([209.85.161.169]:33144 "EHLO mail-yw0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751758AbcF0BN4 (ORCPT ); Sun, 26 Jun 2016 21:13:56 -0400 MIME-Version: 1.0 In-Reply-To: References: <1466154816-17900-1-git-send-email-zhaoyang.huang@spreadtrum.com> From: Zhaoyang Huang Date: Mon, 27 Jun 2016 09:13:55 +0800 Message-ID: Subject: Re: [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select To: "Rafael J. Wysocki" Cc: Linux Kernel Mailing List , "linux-pm@vger.kernel.org" , Ingo Molnar , Peter Zijlstra , =?UTF-8?B?Wmhhb3lhbmcgSHVhbmcgKOm7hOacnemYsyk=?= , Thomas Gleixner , Geng Ren , Alex Wang Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 25 June 2016 at 09:09, Rafael J. Wysocki wrote: > On Fri, Jun 17, 2016 at 11:13 AM, Zhaoyang Huang > wrote: >> In previous version, cpu_pm_enter is invoked > > By whom? Not by the core surely? > >> after the governor select the state, which cause the executing time of cpu_pm_enter >> is included in the idle time. Moving it before the state selection. >> >> Signed-off-by: Zhaoyang Huang >> --- >> kernel/sched/idle.c | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c >> index bd12c6c..929da2e 100644 >> --- a/kernel/sched/idle.c >> +++ b/kernel/sched/idle.c >> @@ -5,6 +5,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -130,6 +131,7 @@ static void cpuidle_idle_call(void) >> struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices); >> struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); >> int next_state, entered_state; >> + int ret; >> >> /* >> * Check if the idle task must be rescheduled. If it is the >> @@ -174,12 +176,16 @@ static void cpuidle_idle_call(void) >> /* >> * Ask the cpuidle framework to choose a convenient idle state. >> */ >> - next_state = cpuidle_select(drv, dev); >> - entered_state = call_cpuidle(drv, dev, next_state); >> - /* >> - * Give the governor an opportunity to reflect on the outcome >> - */ >> - cpuidle_reflect(dev, entered_state); >> + ret = cpu_pm_enter(); > > "To move" usually means "take it away from there and put it here" as > far as kernel patches are concerned, but I only see it added here. > The API is provided by kernel, while invoked by cpuidle driver now. We have ever move 'CPUFREQ_PRECHANGE' from cpufreq driver to framework. I do the same thing here. >> + if (!ret) { >> + next_state = cpuidle_select(drv, dev); >> + entered_state = call_cpuidle(drv, dev, next_state); >> + cpu_pm_exit(); >> + /* >> + * Give the governor an opportunity to reflect on the outcome >> + */ >> + cpuidle_reflect(dev, entered_state); >> + } >> } >> >> exit_idle: >> -- > > No way I will agree to add that notification stuff to the core. > It is not add, just move. > Thanks, > Rafael