From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751550AbdISSyy (ORCPT ); Tue, 19 Sep 2017 14:54:54 -0400 Received: from mail-pf0-f180.google.com ([209.85.192.180]:51273 "EHLO mail-pf0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750895AbdISSyx (ORCPT ); Tue, 19 Sep 2017 14:54:53 -0400 X-Google-Smtp-Source: AOwi7QCIV/OVIDwQ+vbpBBYV89cgDK8AguL3rJwUwiDS4QPOGKGN+FrfKnFWD4nOsI6+JVruDjjKEw== Date: Tue, 19 Sep 2017 11:54:47 -0700 From: Viresh Kumar To: Bo Yan Cc: rjw@rjwysocki.net, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] cpufreq: cpufreq_stats: make last_index signed int Message-ID: <20170919185447.GC30848@ubuntu> References: <1505506402-11497-1-git-send-email-byan@nvidia.com> <20170918015017.GC17030@ubuntu> <961bce0a-5162-5e8d-3474-5b2161cf92c4@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <961bce0a-5162-5e8d-3474-5b2161cf92c4@nvidia.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18-09-17, 10:39, Bo Yan wrote: > Currently, the "last_index" is being checked before > cpufreq_stats_update(stats) inside function > "cpufreq_stats_record_transition", so it's taken care of. > > However, the function "show_time_in_state" also calls cpufreq_stats_update, > the similar check should be done there too, like this: Yeah, that's what I suggested. > diff --git a/drivers/cpufreq/cpufreq_stats.c > b/drivers/cpufreq/cpufreq_stats.c > index e75880eb037d..15305b5ec322 100644 > --- a/drivers/cpufreq/cpufreq_stats.c > +++ b/drivers/cpufreq/cpufreq_stats.c > @@ -62,7 +62,8 @@ static ssize_t show_time_in_state(struct cpufreq_policy > *policy, char *buf) > if (policy->fast_switch_enabled) > return 0; > > - cpufreq_stats_update(stats); > + if ((int)stats->last_index >= 0) You can rather do: if (stats->last_index != -1) > + cpufreq_stats_update(stats); > for (i = 0; i < stats->state_num; i++) { > len += sprintf(buf + len, "%u %llu\n", stats->freq_table[i], > (unsigned long long) > > > This is only needed when policy->cur is not in frequency table when stats > table is created, in which case, stats->last_index will get -1, then user > does a "cat time_in_state" before any frequency transition. > > Does this make sense? -- viresh