Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 6 Oct 2020 15:57:13 +0200 | Subject | Re: [PATCH] cpufreq: stats: Add memory barrier to store_reset() |
| |
On Tue, Oct 6, 2020 at 1:59 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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 <rafael.j.wysocki@intel.com> > --- > > 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; > > >
| |