From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751832AbcF0GNK (ORCPT ); Mon, 27 Jun 2016 02:13:10 -0400 Received: from mx0b-0016f401.pphosted.com ([67.231.156.173]:59220 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751644AbcF0GNH (ORCPT ); Mon, 27 Jun 2016 02:13:07 -0400 Date: Mon, 27 Jun 2016 14:08:58 +0800 From: Jisheng Zhang To: "Rafael J. Wysocki" CC: Viresh Kumar , "Rafael J. Wysocki" , "linux-pm@vger.kernel.org" , Linux Kernel Mailing List , "Srinivas Pandruvada" Subject: Re: regression caused by bb6ab52f2bef ("intel_pstate: Do not set utilization update hook too early") Message-ID: <20160627140858.0c7686f0@xhacker> In-Reply-To: <2958182.eDEME9b2CX@vostro.rjw.lan> References: <20160617163023.5bb374f8@xhacker> <393b63b7caa7474a97baf248c4891b72@SC-EXCH02.marvell.com> <24e20dd7b2b047a19515ff52ad105c37@SC-EXCH02.marvell.com> <2958182.eDEME9b2CX@vostro.rjw.lan> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-06-27_03:,, signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1606270070 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dear Rafael, On Sat, 25 Jun 2016 02:14:28 +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. :-) > > 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. This patch works! on a 32 snb cores server: 80 wakeups/s w/o the patch 7 wakeups/s w/ the patch on a 4 snb cores laptop: 22 wakeups/s w/o the patch 6 wakeups/s w/ the patch Thank you so much for fixing it ;) > > Fixes: bb6ab52f2bef (intel_pstate: Do not set utilization update hook too early) > Reported-by: Jisheng Zhang So feel free to add Tested-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); > >