pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Add pg_atomic_unlocked_write_u64
8+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2025-12-09 aa749bde32 Improve documentation for pg_atomic_unlocked_write_u32()
* pgsql: Add pg_atomic_unlocked_write_u64
@ 2025-12-03 23:40 Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2025-12-03 23:40 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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.
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/7902a47c20b1d5c0b7d20071f9ada6a0185bf39b
Modified Files
--------------
src/include/port/atomics.h | 10 ++++++++++
src/include/port/atomics/generic.h | 9 +++++++++
2 files changed, 19 insertions(+)
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Add pg_atomic_unlocked_write_u64
@ 2025-12-04 15:51 Nathan Bossart <nathandbossart@gmail.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Bossart @ 2025-12-04 15:51 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Add pg_atomic_unlocked_write_u64
@ 2025-12-04 15:56 Andres Freund <andres@anarazel.de>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2025-12-04 15:56 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Add pg_atomic_unlocked_write_u64
@ 2025-12-04 16:03 Nathan Bossart <nathandbossart@gmail.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Bossart @ 2025-12-04 16:03 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Add pg_atomic_unlocked_write_u64
@ 2025-12-04 17:30 Andres Freund <andres@anarazel.de>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2025-12-04 17:30 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Add pg_atomic_unlocked_write_u64
@ 2025-12-04 18:15 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2025-12-04 18:15 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Add pg_atomic_unlocked_write_u64
@ 2025-12-04 18:41 Nathan Bossart <nathandbossart@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 8+ messages in thread
From: Nathan Bossart @ 2025-12-04 18:41 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@lists.postgresql.org
On Thu, Dec 04, 2025 at 01:15:55PM -0500, Tom Lane wrote:
> LGTM otherwise.
+1
--
nathan
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Add pg_atomic_unlocked_write_u64
@ 2025-12-09 04:12 Andres Freund <andres@anarazel.de>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 8+ messages in thread
From: Andres Freund @ 2025-12-09 04:12 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2025-12-09 04:12 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-12-03 23:40 pgsql: Add pg_atomic_unlocked_write_u64 Andres Freund <andres@anarazel.de>
2025-12-04 15:51 ` Nathan Bossart <nathandbossart@gmail.com>
2025-12-04 15:56 ` Andres Freund <andres@anarazel.de>
2025-12-04 16:03 ` Nathan Bossart <nathandbossart@gmail.com>
2025-12-04 17:30 ` Andres Freund <andres@anarazel.de>
2025-12-04 18:15 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-04 18:41 ` Nathan Bossart <nathandbossart@gmail.com>
2025-12-09 04:12 ` Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox