On Tue, Oct 6, 2020 at 1:59 PM Rafael J. Wysocki <[email protected]> wrote: > > From: Rafael J. Wysocki <[email protected]> > > 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(). > > Fixes: 40c3bd4cfa6f ("cpufreq: stats: Defer stats update to > cpufreq_stats_record_transition()") > Signed-off-by: Rafael J. Wysocki <[email protected]> > --- > > I couldn't convince myself that it was OK to leave the code as it was. > > linux-next material. > > --- > drivers/cpufreq/cpufreq_stats.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > Index: linux-pm/drivers/cpufreq/cpufreq_stats.c > =================================================================== > --- linux-pm.orig/drivers/cpufreq/cpufreq_stats.c > +++ linux-pm/drivers/cpufreq/cpufreq_stats.c > @@ -99,6 +99,13 @@ static ssize_t store_reset(struct cpufre > * avoid races. > */ > 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. Note that they both > access > + * reset_time only if reset_pending is 1, so corresponding read > barriers > + * are not needed.
I'm taking this back after double-checking memory-barriers.txt. Will send a v2. > + */ > + smp_wmb(); > WRITE_ONCE(stats->reset_pending, 1); > > return count; > > >
