From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752531AbcEJT5N (ORCPT ); Tue, 10 May 2016 15:57:13 -0400 Received: from mga03.intel.com ([134.134.136.65]:17261 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751293AbcEJT5L (ORCPT ); Tue, 10 May 2016 15:57:11 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,606,1455004800"; d="scan'208";a="803335162" Message-ID: <1462910284.28729.116.camel@linux.intel.com> Subject: Re: [PATCH 1/3] intel_pstate: Clarify average performance computation From: Srinivas Pandruvada To: "Rafael J. Wysocki" Cc: "Rafael J. Wysocki" , Linux PM list , Linux Kernel Mailing List Date: Tue, 10 May 2016 12:58:04 -0700 In-Reply-To: References: <2638272.Bq3MnQEe4U@vostro.rjw.lan> <1880504.e9G30eDLBM@vostro.rjw.lan> <1462843116.4224.16.camel@linux.intel.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2 (3.18.5.2-1.fc23) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2016-05-10 at 21:21 +0200, Rafael J. Wysocki wrote: > On Tue, May 10, 2016 at 3:18 AM, Srinivas Pandruvada > wrote: > > > > On Sat, 2016-05-07 at 01:44 +0200, Rafael J. Wysocki wrote: > > > > > > From: Rafael J. Wysocki > > > > > > The core_pct_busy field of struct sample actually contains the > > > average performace during the last sampling period (in percent) > > > and not the utilization of the core as suggested by its name > > > which is confusing. > > > > > > For this reason, change the name of that field to core_avg_perf > > > and rename the function that computes its value accordingly. > > > > > Makes perfect sense. > > > > > > > > Also notice that it would be more useful if it was a "raw" > > > fraction > > > rather than percentage, so change its meaning too and update the > > > code using it accordingly (it is better to change the name of > > > the field along with its meaning in one go than to make those > > > two changes separately, as that would likely lead to more > > > confusion). > > Due to the calculation the results from old and new method will be > > similar but not same. For example in one scenario the > > get_avg_frequency difference is 4.3KHz (printed side by side using > > both > > old style using pct and new using fraction) > > Frequency with old calc: 2996093 Hz > I guess the above is the new one? > > > > > Frequency with old calc: 3000460 Hz > So the relative difference is of the order of 0.1% and that number is > not what is used in PID computations.  That's what is printed, but > I'm > not sure if that's really that important. :-) This difference will appear in cpufreq sysfs as their granularity in KHz for current frequency. But the difference is very small. So I guess no one will notice. Thanks, Srinivas > > Here, the sample.aperf bits lost because the 100 was moved away from > intel_pstate_calc_busy() would be multiplied by a relatively large > number to produce the difference that looks significant, but the > numbers actually used in computations are a few orders of magnitude > smaller. > > > > > How much do you think the performance gain changing fraction vs > > pct? > I'm more concerned about latency than about performance.  On HWP, for > example, the costly multiplication removed by this from the hot path > is of the order of the half of the work done. > > That said, I can do something to retain the bits in question for as > long as possible, although the patch will be slightly more > complicated > then. :-) The