Hi,

On 2025-12-04 10:03:22 -0600, Nathan Bossart wrote:
> On Thu, Dec 04, 2025 at 10:56:12AM -0500, Andres Freund wrote:
> > The whole point of the _unlocked_ function is to use it for modifying an
> > atomic that doesn't need to actually be atomic when modified by that
> > function. The current use-case for it is to to modify BufferDesc->state for
> > temporary table buffers. Those obviously can't be shared across processes 
> > and
> > therefore don't need an atomic operation to be modified. In the referenced
> > thread I'm working on converting BufferDesc->state to be a 64bit atomic, 
> > hence
> > the need for pg_atomic_unlocked_write_u64().
> > 
> > I didn't notice that the comment for pg_atomic_unlocked_write_u32() makes 
> > that
> > claim about partial writes not being visible. I think we should just remove
> > that claim.
> 
> +1 to updating the comment with this context.

Hm. Aside from the above issue, the reference to atomics emulation in the
comment is also obsolete since 81385261362.

How about:

/*
 * pg_atomic_unlocked_write_u32 - unlocked write to atomic variable.
 *
 * Write to an atomic variable, without atomicity guarantees. I.e. it is not
 * guaranteed that a concurent reader will not see a torn value, nor to
 * guaranteed to correctly interact with concurrent read-modify-write
 * operations like pg_atomic_compare_exchange_u32.  This should only be used
 * in cases where minor performance regressions due to atomic operations are
 * unacceptable and where exclusive access is guaranteed due to some external
 * means.
 *
 * No barrier semantics.
 */


We could actually guarantee, in the 32bit case, that a concurrent reader would
not see a torn value, but ISTM that any such user should not use _unlocked_,
and this way we don't need separate documentation for the 64bit case.

Greetings,

Andres Freund


Reply via email to