Re: [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()

[Rafael J. Wysocki]

On Fri, Sep 11, 2020 at 1:36 PM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> On 11-09-20, 12:11, peterz@xxxxxxxxxxxxx wrote:
> > On Wed, Sep 02, 2020 at 12:54:41PM +0530, Viresh Kumar wrote:
> > > +   atomic_t reset_pending;
> >
> > > +   atomic_set(&stats->reset_pending, 0);
> > > +   if (atomic_read(&stats->reset_pending))
> > > +   bool pending = atomic_read(&stats->reset_pending);
> > > +   atomic_set(&stats->reset_pending, 1);
> > > +   bool pending = atomic_read(&stats->reset_pending);
> > > +   if (atomic_read(&stats->reset_pending))
> >
> > What do you think atomic_t is doing for you?
>
> I was trying to avoid races while two writes are going in parallel,
> but obviously as this isn't a RMW operation, it won't result in
> anything for me.
>
> Maybe what I should be doing is just READ_ONCE()/WRITE_ONCE()? So the
> other side doesn't see any intermediate value that was never meant to
> be set/read ?

If the value in question is a pointer or an int (or equivalent),
READ_ONCE()/WRITE_ONCE() should be sufficient, and should be used at
least as a matter of annotation of the sensitive code IMO.

IIRC, atomic_set() and atomic_read() are pretty much the same as
WRITE_ONCE() and READ_ONCE(), respectively, anyway.

Cheers!