From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751658AbcFYPJu (ORCPT ); Sat, 25 Jun 2016 11:09:50 -0400 Received: from mga11.intel.com ([192.55.52.93]:12864 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751557AbcFYPJs (ORCPT ); Sat, 25 Jun 2016 11:09:48 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,527,1459839600"; d="scan'208";a="1009524506" From: "Pandruvada, Srinivas" To: "jszhang@marvell.com" , "viresh.kumar@linaro.org" , "rjw@rjwysocki.net" CC: "linux-kernel@vger.kernel.org" , "linux-pm@vger.kernel.org" , "rafael@kernel.org" Subject: Re: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early") Thread-Topic: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early") Thread-Index: AQHRznXx3nR6bSPkW0+B8XeVCQBXr5/6v0mA Date: Sat, 25 Jun 2016 15:09:46 +0000 Message-ID: <1466867309.4272.32.camel@intel.com> References: <20160617163023.5bb374f8@xhacker> <393b63b7caa7474a97baf248c4891b72@SC-EXCH02.marvell.com> <24e20dd7b2b047a19515ff52ad105c37@SC-EXCH02.marvell.com> <2958182.eDEME9b2CX@vostro.rjw.lan> In-Reply-To: <2958182.eDEME9b2CX@vostro.rjw.lan> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.255.230.61] Content-Type: text/plain; charset="utf-8" Content-ID: <3FB35EB0840DDC4E82B15BE4C53A0DC7@intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u5PF9sWb029619 On Sat, 2016-06-25 at 02:14 +0200, Rafael J. Wysocki wrote: > On Friday, June 17, 2016 04:09:33 PM Jisheng Zhang wrote: > > Dear all, > > > > If using acpi-cpufreq instead, v4.6, v4.6-rc3, v4.7-rc3 can't > > reproduce the issue. It seems > > only intel_pstate is impacted. > > Which is quite obvious, since the commit your bisection led to was > intel_pstate-specific. :-) > We should also check why the set_policy callback is getting called quite often. May be some thermal zone is tripping quite often. echo 'file thermal_core.c +p' > /sys/kernel/debug/dynamic_debug/control may give us some clue. Thanks, Srinivas > If the issue is what I'm thinking it is, the patch below should help, > so > can you please test it? > > Thanks, > Rafael > > --- > From: Rafael J. Wysocki > Subject: [PATCH] intel_pstate: Do not clear utilization update hooks > on policy changes > > intel_pstate_set_policy() is invoked by the cpufreq core during > driver initialization, on changes of policy attributes (minimim and > maximum frequency, for example) via sysfs and via CPU notifications > from the platform firmware.  On some platforms the latter may occur > relatively often. > > Commit bb6ab52f2bef (intel_pstate: Do not set utilization update hook > too early) made intel_pstate_set_policy() clear the CPU's utilization > update hook before updating the policy attributes for it (and set the > hook again after doind that), but that involves invoking > synchronize_sched() and adds overhead to the CPU notifications > mentioned above and to the sched-RCU handling in general. > > That extra overhead is arguably not necessary, because updating > policy attributes when the CPU's utilization update hook is active > should not lead to any adverse effects, so drop the clearing of > the hook from intel_pstate_set_policy() and make it check if > the hook has been set already when attempting to set it. > > Fixes: bb6ab52f2bef (intel_pstate: Do not set utilization update hook > too early) > Reported-by: Jisheng Zhang > Signed-off-by: Rafael J. Wysocki > --- >  drivers/cpufreq/intel_pstate.c |    5 +++-- >  1 file changed, 3 insertions(+), 2 deletions(-) > > Index: linux-pm/drivers/cpufreq/intel_pstate.c > =================================================================== > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c > +++ linux-pm/drivers/cpufreq/intel_pstate.c > @@ -1440,6 +1440,9 @@ static void intel_pstate_set_update_util >  { >   struct cpudata *cpu = all_cpu_data[cpu_num]; >   > + if (cpu->update_util_set) > + return; > + >   /* Prevent intel_pstate_update_util() from using stale data. > */ >   cpu->sample.time = 0; >   cpufreq_add_update_util_hook(cpu_num, &cpu->update_util, > @@ -1480,8 +1483,6 @@ static int intel_pstate_set_policy(struc >   if (!policy->cpuinfo.max_freq) >   return -ENODEV; >   > - intel_pstate_clear_update_util_hook(policy->cpu); > - >   pr_debug("set_policy cpuinfo.max %u policy->max %u\n", >    policy->cpuinfo.max_freq, policy->max); >   >