Date: Fri, 11 Sep 2026 13:02:44 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Yura Sokolov <y.sokolov@postgrespro.ru>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: convert various variables to atomics
Message-ID: <aqRCRGBaAYFQFKWA@nathan>
References: <amDAZAVob4DJjUfW@nathan>
 <ycvruij7554tlhw6w7bg4lqtibd52qguo52iojyou7ejdv5jrm@qyyr5f3hdsue>
 <amDGRZxnlmTVjkCe@nathan>
 <amJx4Lwx4nuuExYT@nathan>
 <207c0bfb-6e06-4358-bb2f-c961915efc36@eisentraut.org>
 <hi4rsxwu3ioas5rmuwfnu2gisqb2rd6g2uq56r3pvzk7clismo@oyndzrrylnir>
 <3856d1cf-53a8-414b-98d9-829d5a455a86@iki.fi>
 <aqA_P7Uwub-MDXOO@nathan>
 <aqBmuOO26ZtG_BgX@nathan>
 <55e6a615-ec9d-4a57-8f36-7fbd1674da2c@postgrespro.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <55e6a615-ec9d-4a57-8f36-7fbd1674da2c@postgrespro.ru>
Archived-At: <https://www.postgresql.org/message-id/aqRCRGBaAYFQFKWA%40nathan>
Precedence: bulk

On Fri, Sep 11, 2026 at 05:43:22PM +0300, Yura Sokolov wrote:
> Personally, I don't like current implementation of
> pg_atomic_read_membarrier_u32 because it writes into shared variable.

I think your dislike of the membarrier implementation is misguided.  The
write is important and helps reduce the cognitive load of reading the code.

A spinlock guarantees that whoever takes the lock sees everything the
previous holder did before releasing the lock.  The membarrier functions
keep that guarantee because every access is a read-modify-write, i.e.,
whoever touches the variable second must read what the first one wrote.
Take the following example:

    /* thread A */
    x = 1;
    z = pg_atomic_read_membarrier_u32(&y);

    /* thread B */
    pg_atomic_write_membarrier_u32(&y, 1);
    x = 2;

Let's say thread A's read of "y" returns 0.  That must mean that thread A
wrote "x" before thread B did, which is same as what you'd get with a
spinlock.  If the read was just a plain load behind a barrier, we can't
know the order of the writes to "x" on non-TSO architectures.

> That is why in [1] (thread [2]) I used explicit pg_memory_barrier before
> and pg_read_barrier after reading segP->maxMsgNum. (pg_memory_barrier
> writes onto stack - process's private memory, and pg_read_barrier does
> nothing on x86_64).

My patch is intended to be a straightforward spinlock-to-atomics
conversion, so I'd like to keep the membarrier accessors for now.  Further
optimizations should be handled in their own threads.  Two that come to
mind are an x86-specific implementation of pg_atomic_read_membarrier_u32()
(since it _is_ a TSO architecture), and something like your patch for
sinvaladt.c, i.e., using explicit barriers for that code.

-- 
nathan