Commit efad4240 authored by Rafael J. Wysocki's avatar Rafael J. Wysocki

cpufreq: stats: Add memory barrier to store_reset()

There is nothing to prevent the CPU or the compiler from reordering
the writes to stats->reset_time and stats->reset_pending in
store_reset(), in which case the readers of stats->reset_time may see
a stale value.  Moreover, on 32-bit arches the write to reset_time
cannot be completed in one go, so the readers of it may see a
partially updated value in that case.

To prevent that from happening, add a write memory barrier between
the writes to stats->reset_time and stats->reset_pending in
store_reset() and corresponding read memory barrier in the
readers of stats->reset_time.

Fixes: 40c3bd4c ("cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()")
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: default avatarViresh Kumar <viresh.kumar@linaro.org>
parent 86836bac
...@@ -47,6 +47,11 @@ static void cpufreq_stats_reset_table(struct cpufreq_stats *stats) ...@@ -47,6 +47,11 @@ static void cpufreq_stats_reset_table(struct cpufreq_stats *stats)
/* Adjust for the time elapsed since reset was requested */ /* Adjust for the time elapsed since reset was requested */
WRITE_ONCE(stats->reset_pending, 0); WRITE_ONCE(stats->reset_pending, 0);
/*
* Prevent the reset_time read from being reordered before the
* reset_pending accesses in cpufreq_stats_record_transition().
*/
smp_rmb();
cpufreq_stats_update(stats, READ_ONCE(stats->reset_time)); cpufreq_stats_update(stats, READ_ONCE(stats->reset_time));
} }
...@@ -71,10 +76,16 @@ static ssize_t show_time_in_state(struct cpufreq_policy *policy, char *buf) ...@@ -71,10 +76,16 @@ static ssize_t show_time_in_state(struct cpufreq_policy *policy, char *buf)
for (i = 0; i < stats->state_num; i++) { for (i = 0; i < stats->state_num; i++) {
if (pending) { if (pending) {
if (i == stats->last_index) if (i == stats->last_index) {
/*
* Prevent the reset_time read from occurring
* before the reset_pending read above.
*/
smp_rmb();
time = get_jiffies_64() - READ_ONCE(stats->reset_time); time = get_jiffies_64() - READ_ONCE(stats->reset_time);
else } else {
time = 0; time = 0;
}
} else { } else {
time = stats->time_in_state[i]; time = stats->time_in_state[i];
if (i == stats->last_index) if (i == stats->last_index)
...@@ -99,6 +110,11 @@ static ssize_t store_reset(struct cpufreq_policy *policy, const char *buf, ...@@ -99,6 +110,11 @@ static ssize_t store_reset(struct cpufreq_policy *policy, const char *buf,
* avoid races. * avoid races.
*/ */
WRITE_ONCE(stats->reset_time, get_jiffies_64()); WRITE_ONCE(stats->reset_time, get_jiffies_64());
/*
* The memory barrier below is to prevent the readers of reset_time from
* seeing a stale or partially updated value.
*/
smp_wmb();
WRITE_ONCE(stats->reset_pending, 1); WRITE_ONCE(stats->reset_pending, 1);
return count; return count;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment