pgsql: Add pg_atomic_unlocked_write_u64

On Wed, Dec 03, 2025 at 11:40:47PM +0000, Andres Freund wrote:

Add pg_atomic_unlocked_write_u64

The 64bit equivalent of pg_atomic_unlocked_write_u32(), to be used in an
upcoming patch converting BufferDesc.state into a 64bit atomic.

I noticed that this new function was defined as

ptr->value = val;

and couldn't figure out why that was safe. Above
pg_atomic_unlocked_write_u32(), I see this comment:

* The write is guaranteed to succeed as a whole, i.e. it's not possible to
* observe a partial write for any reader. ...

But the new 64-bit version doesn't seem to be surrounded by a check for
PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY, and I found no discussion about it in
the commit message or the linked thread. Am I missing something here?

--
nathan

Hi,

On 2025-12-04 09:51:10 -0600, Nathan Bossart wrote:

On Wed, Dec 03, 2025 at 11:40:47PM +0000, Andres Freund wrote:

Add pg_atomic_unlocked_write_u64

The 64bit equivalent of pg_atomic_unlocked_write_u32(), to be used in an
upcoming patch converting BufferDesc.state into a 64bit atomic.

I noticed that this new function was defined as

ptr->value = val;

and couldn't figure out why that was safe. Above
pg_atomic_unlocked_write_u32(), I see this comment:

* The write is guaranteed to succeed as a whole, i.e. it's not possible to
* observe a partial write for any reader. ...

But the new 64-bit version doesn't seem to be surrounded by a check for
PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY, and I found no discussion about it in
the commit message or the linked thread. Am I missing something here?

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.

Greetings,

Andres Freund

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.

--
nathan

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

Andres Freund <andres@anarazel.de> writes:

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

grammar police: s/nor to/nor is this/

* 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.
*/

LGTM otherwise.

regards, tom lane

On Thu, Dec 04, 2025 at 01:15:55PM -0500, Tom Lane wrote:

LGTM otherwise.

+1

--
nathan

Hi,

On 2025-12-04 12:41:26 -0600, Nathan Bossart wrote:

On Thu, Dec 04, 2025 at 01:15:55PM -0500, Tom Lane wrote:

LGTM otherwise.

+1

Thanks for the reviews Tom and Nathan. Pushed with minor further tweaks
(s/due to/via/, s/concurent/concurrent/).

Greetings,

Andres Freund