linux-kernel - [PATCH] cpufreq: stats: Do not populate stats when policy->cur has no exact match

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1384561243-30391-1-git-send-email-nm@ti.com>
Date:	Fri, 15 Nov 2013 18:20:43 -0600
From:	Nishanth Menon <nm@...com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Viresh Kumar <viresh.kumar@...aro.org>
CC:	<cpufreq@...r.kernel.org>, <linux-pm@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <ceh@...com>,
	Nishanth Menon <nm@...com>,
	Tobias Diedrich <ranma+xen@...edrich.de>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Dave Jones <davej@...hat.com>
Subject: [PATCH] cpufreq: stats: Do not populate stats when policy->cur has no exact match

commit 46a310b ([CPUFREQ] Don't set stat->last_index to -1 if the
pol->cur has incorrect value.) tries to handle case where policy->cur
does not match any entry in freq_table.

As indicated in the above commit, the exact match search of
freq_table_get index will return a -1 which is stored in
stat->last_index. However, as a result of the above commit,
cpufreq_stat_notifier_trans which updates the statistics, fails
to update any *further* valid transitions that take place as
stat->last_index is -1 as the condition occurs at boot and never
solved.

So, instead of having a statistics information that never ever
reflects valid data in the mentioned case and scratching our heads, we
instead, refuse to populate any of the statistics entries and note in
kernel log the error condition for developers to fix. The only useable
information are the available frequencies which is already available
in other cpufreq sysfs entries.

Cc: Tobias Diedrich <ranma+xen@...edrich.de>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Cc: Dave Jones <davej@...hat.com>
Reported-by: Carlos Hernandez <ceh@...com>
Signed-off-by: Nishanth Menon <nm@...com>
---

Patch based on v3.12 tag

Reported by Carlos on TI vendor kernel (v3.12tag based):

OMAP5uEVM: http://pastebin.mozilla.org/3612196
AM335x-evm: http://pastebin.mozilla.org/3612220

As can be seen, the translation table are present, however none of the
transition information is ever updated.
With the current patch, this becomes: http://pastebin.mozilla.org/3612256

Original commit thread seems to be:
https://lkml.org/lkml/2011/6/16/760 but I dont see much information if
this was discussed further.

An alternate approach possible is to try and recover from this case by
using an 'atmost' match to find a closest match and then giveup when
we cant find one, but that does not really indicate a proper statistic
(if freq1 < cur_freq < freq2, was the intent to be at freq1 or freq2?
stats should not make that policy decision)
	http://pastebin.mozilla.org/3612179 (alternate approach 1)

Yet another option might be to update last_index using the first
transition into a valid index, but then, we wont have statistics
information 100% correct as we did not store the very first frequency
stat.
	 http://pastebin.mozilla.org/3612241 (alternate approach 2)

 drivers/cpufreq/cpufreq_stats.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index 4cf0d28..4c8a501 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -251,6 +251,14 @@ static int cpufreq_stats_create_table(struct cpufreq_policy *policy,
 	stat->last_time = get_jiffies_64();
 	stat->last_index = freq_table_get_index(stat, policy->cur);
 	spin_unlock(&cpufreq_stats_lock);
+
+	if (stat->last_index == -1) {
+		pr_err("%s: No match for current freq %u in table. Disabled!\n",
+		       __func__, policy->cur);
+		ret = -EINVAL;
+		goto error_out;
+	}
+
 	cpufreq_cpu_put(current_policy);
 	return 0;
 error_out:
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/