Re: valgrind versus pg_atomic_init() - Mailing list pgsql-hackers

From Noah Misch
Subject Re: valgrind versus pg_atomic_init()
Date
Msg-id 20200617032429.GB2916904@rfd.leadboat.com
Whole thread Raw
In response to Re: valgrind versus pg_atomic_init()  (Andres Freund <andres@anarazel.de>)
Responses Re: valgrind versus pg_atomic_init()
List pgsql-hackers
On Sun, Jun 14, 2020 at 09:16:20PM -0700, Andres Freund wrote:
> On 2020-06-14 18:55:27 -0700, Noah Misch wrote:
> > Does something guarantee the write will be globally-visible by the time the
> > first concurrent accessor shows up?
> 
> The function comments say:
> 
>  *
>  * Has to be done before any concurrent usage..
>  *
>  * No barrier semantics.
> 
> 
> > (If not, one could (a) do an unlocked ptr->value=0, then the atomic
> > write, or (b) revert and improve the suppression.)  I don't doubt it's
> > fine for the ways PostgreSQL uses atomics today, which generally
> > initialize an atomic before the concurrent-accessor processes even
> > exist.
> 
> I think it's unlikely that there are cases where you could safely
> initialize the atomic without needing some form of synchronization
> before it can be used. If a barrier were needed, what'd guarantee the
> concurrent access happened after the initialization in the first place?

Suppose the initializing process does:

  pg_atomic_init_u64(&somestruct->atomic, 123);
  somestruct->atomic_ready = true;

In released versions, any process observing atomic_ready==true will observe
the results of the pg_atomic_init_u64().  After the commit from this thread,
that's no longer assured.  Having said that, I agree with a special case of
Tom's assertion about spinlocks, namely that this has same problem:

  /* somestruct->lock already happens to contain value 0 */
  SpinLockInit(&somestruct->lock);
  somestruct->lock_ready = true;



pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: Review for GetWALAvailability()
Next
From: Noah Misch
Date:
Subject: Remove dead forceSync parameter of XactLogCommitRecord()