From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752645AbcAAWln (ORCPT ); Fri, 1 Jan 2016 17:41:43 -0500 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:37206 "EHLO e23smtp09.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752611AbcAAWl1 (ORCPT ); Fri, 1 Jan 2016 17:41:27 -0500 X-IBM-Helo: d23dlp03.au.ibm.com X-IBM-MailFrom: shilpa.bhat@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Message-ID: <5687005E.602@linux.vnet.ibm.com> Date: Sat, 02 Jan 2016 04:10:30 +0530 From: Shilpasri G Bhat User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Paul Clarke , linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org CC: viresh.kumar@linaro.org, rjw@rjwysocki.net Subject: Re: [PATCH] cpufreq: powernv: Redesign the presentation of throttle notification References: <1450030657-9121-1-git-send-email-shilpa.bhat@linux.vnet.ibm.com> <566F34A6.4060709@us.ibm.com> In-Reply-To: <566F34A6.4060709@us.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16010122-0033-0000-0000-000002AA586E Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 12/15/2015 02:59 AM, Paul Clarke wrote: > On 12/13/2015 12:17 PM, Shilpasri G Bhat wrote: >> Replace the throttling event console messages to perf trace event >> "power:powernv_throttle" and throttle counter stats which are >> exported in sysfs. The newly added sysfs files are as follows: >> >> 1)/sys/devices/system/node/node0/throttle_frequencies >> This gives the throttle stats for each of the available frequencies. >> The throttle stat of a frequency is the total number of times the max >> frequency was reduced to that frequency. >> # cat /sys/devices/system/node/node0/throttle_frequencies >> 4023000 0 >> 3990000 0 >> 3956000 1 >> 3923000 0 >> 3890000 0 >> 3857000 2 >> 3823000 0 >> 3790000 0 >> 3757000 2 >> 3724000 1 >> 3690000 1 >> ... > > Is this data useful? It seems like "elapsed time" at each frequency might be > more useful, if any. > Yes elapsed time is more useful data here. But the concern here is with the accuracy of measurement/observation of elapsed time by the kernel. OCC can throttle/unthrottle the frequency at the granularity of 250us. Although OCC updates the throttle status to HOMER region immediately there may be a delay in propagating this message by the opal-poller to the driver. So instead we might want OCC to give us the throttled elapsed time stat for each frequency and opal-poller/driver can take the snapshot of this info every n seconds. >> 2)/sys/devices/system/node/node0/throttle_reasons >> This gives the stats for each of the supported throttle reasons. >> This gives the total number of times the frequency was throttled due >> to each of the reasons. >> # cat /sys/devices/system/node/node0/throttle_reasons >> No throttling 7 >> Power Cap 0 >> Processor Over Temperature 7 >> Power Supply Failure 0 >> Over Current 0 >> OCC Reset 0 >> >> 3)/sys/devices/system/node/node0/throttle_stat >> This gives the total number of throttle events occurred in turbo >> range of frequencies and non-turbo(below nominal) range of >> frequencies. > > non-turbo should read "at or below nominal". Maybe "sub-turbo" is a better term(?) > >> # cat /sys/devices/system/node/node0/throttle_stat >> Turbo 7 >> Nominal 0 > > Should this read "Non-turbo" or "Sub-turbo" instead of "Nominal", since the > events could well occur when already operating below nominal. > Agree. Applied 'sub-turbo' in v2 >> Signed-off-by: Shilpasri G Bhat >> --- >> drivers/cpufreq/powernv-cpufreq.c | 186 +++++++++++++++++++++++++++++--------- >> include/trace/events/power.h | 22 +++++ >> 2 files changed, 166 insertions(+), 42 deletions(-) >> >> diff --git a/drivers/cpufreq/powernv-cpufreq.c >> b/drivers/cpufreq/powernv-cpufreq.c >> index cb50138..bdde9d6 100644 >> --- a/drivers/cpufreq/powernv-cpufreq.c >> +++ b/drivers/cpufreq/powernv-cpufreq.c >> @@ -28,6 +28,9 @@ >> #include >> #include >> #include >> +#include >> +#include >> +#include >> >> #include >> #include >> @@ -43,12 +46,27 @@ >> static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1]; >> static bool rebooting, throttled, occ_reset; >> >> +static char throttle_reason[][30] = { >> + "No throttling", >> + "Power Cap", >> + "Processor Over Temperature", >> + "Power Supply Failure", >> + "Over Current", >> + "OCC Reset" >> + }; > > I'm curious if this would be slightly more efficiently implemented as: > static const char *throttle_reason[] = { ... }; > > Do you need 30 characters per string for a reason? > > Regardless, it should be const. Modified the declaration in v2 version of the patch. > > [...] > -- > PC Thanks and Regards, Shilpa