On Tue, May 23, 2023 at 6:26 PM Michael Paquier <michael@paquier.xyz> wrote:
> Spinlocks provide a full memory barrier, which may not the case with
> add_u64() or read_u64(). Could memory ordering be a problem in these
> code paths?
It could be a huge problem if what you say were true, but unless I'm
missing something, the comments in atomics.h say that it isn't. The
comments for the 64-bit functions say to look at the 32-bit functions,
and there it says:
/*
* pg_atomic_add_fetch_u32 - atomically add to variable
*
* Returns the value of ptr after the arithmetic operation.
*
* Full barrier semantics.
*/
Which is probably a good thing, because I'm not sure what good it
would be to have an operation that both reads and modifies an atomic
variable but has no barrier semantics.
That's not to say that I entirely understand the point of this patch.
It seems like a generally reasonable thing to do AFAICT but somehow I
wonder if there's more to the story here, because it doesn't feel like
it would be easy to measure the benefit of this patch in isolation.
That's not a reason to reject it, though, just something that makes me
curious.
--
Robert Haas
EDB: http://www.enterprisedb.com