Thread

1. Replace pg_atomic_flag with pg_atomic_bool

   Heikki Linnakangas <hlinnaka@iki.fi> — 2026-07-05T22:27:45Z

   Hi,
   
   I've always found the pg_atomic_flag functions counterintuitive. I 
   remember by now pretty well how the pg_atomic_u32/u64 functions work, 
   but any time I have to look at the pg_atomic_flag functions, I feel 
   not-like I'm in a foreign land.
   
   I think that's because I think of pg_atomic_flag as an atomic version of 
   "bool". I'd assume there to be functions similar to the u32/u64 
   functions, like pg_atomic_read_bool() and pg_atomic_write_bool(). But 
   alas, neither of those functions exist. To read the value, you have to 
   use pg_atomic_unlocked_test_flag(), and remember that when the flag is 
   set, it returns *false*, which feels completely backwards to me. It 
   makes more sense with pg_atomic_test_set_flag(), which returns 'false' 
   if the flag was already set and hence was not set again, but it 
   nevertheless just feels wrong to me.
   
   I propose that we replace pg_atomic_flag with pg_atomic_bool, which 
   behaves more like pg_atomic_u32/u64.
   
   We currently only use pg_atomic_bool in two places: in the shared memory 
   structs of pg_stash_advise and autovacuum. I actually considered just 
   removing pg_test_flag altogether and replacing those two uses directly 
   with pg_atomic_u32, but it's still nice to have a distinct boolean type. 
   It's worth noting that the current usage is not very performance 
   critical (not that I'd expect the u32 functions to be any slower).
   
   
   Thomas's patches to switch to standard C <stdatomic.h> at [1] is also 
   touching this. They keep the the application-facing functions unchanged, 
   but changes the implementation to use the same atomic-exchange 
   instructions as for u32/u64. This is complementary and an independent 
   topic to discuss, although the patches will conflict.
   
   [1] 
   https://www.postgresql.org/message-id/CA%2BhUKGKFvu3zyvv3aaj5hHs9VtWcjFAmisOwOc7aOZNc5AF3NA%40mail.gmail.com
   
   - Heikki
   

2. Re: Replace pg_atomic_flag with pg_atomic_bool

   Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-07-06T15:05:04Z

   On Mon, Jul 6, 2026 at 3:57 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
   
   >
   > I think that's because I think of pg_atomic_flag as an atomic version of
   > "bool". I'd assume there to be functions similar to the u32/u64
   > functions, like pg_atomic_read_bool() and pg_atomic_write_bool(). But
   > alas, neither of those functions exist. To read the value, you have to
   > use pg_atomic_unlocked_test_flag(), and remember that when the flag is
   > set, it returns *false*, which feels completely backwards to me. It
   > makes more sense with pg_atomic_test_set_flag(), which returns 'false'
   > if the flag was already set and hence was not set again, but it
   > nevertheless just feels wrong to me.
   >
   
   I am using pg_atomic_test_set_flag() as a non-wait locking, lighter
   than LWLock mechanism in shared buffer resizing patches. That might
   change in future. But I think there's use for TAS kind of semantics.
   But I have also got confused when looking at it as pg_atomic_bool. I
   think we need both.
   
   -- 
   Best Wishes,
   Ashutosh Bapat
   
   
   
   

3. Re: Replace pg_atomic_flag with pg_atomic_bool

   Heikki Linnakangas <hlinnaka@iki.fi> — 2026-07-06T16:01:25Z

   On 06/07/2026 18:05, Ashutosh Bapat wrote:
   > On Mon, Jul 6, 2026 at 3:57 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
   > 
   >>
   >> I think that's because I think of pg_atomic_flag as an atomic version of
   >> "bool". I'd assume there to be functions similar to the u32/u64
   >> functions, like pg_atomic_read_bool() and pg_atomic_write_bool(). But
   >> alas, neither of those functions exist. To read the value, you have to
   >> use pg_atomic_unlocked_test_flag(), and remember that when the flag is
   >> set, it returns *false*, which feels completely backwards to me. It
   >> makes more sense with pg_atomic_test_set_flag(), which returns 'false'
   >> if the flag was already set and hence was not set again, but it
   >> nevertheless just feels wrong to me.
   >>
   > 
   > I am using pg_atomic_test_set_flag() as a non-wait locking, lighter
   > than LWLock mechanism in shared buffer resizing patches. That might
   > change in future. But I think there's use for TAS kind of semantics.
   > But I have also got confused when looking at it as pg_atomic_bool. I
   > think we need both.
   
   The functionality is still there with my patch, as the 
   pg_atomic_exchange_bool() function. Would that work for you?
   
   - Heikki
   
   
   
   
   

4. Re: Replace pg_atomic_flag with pg_atomic_bool

   Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-07-07T05:47:04Z

   On Mon, Jul 6, 2026 at 9:31 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
   >
   > On 06/07/2026 18:05, Ashutosh Bapat wrote:
   > > On Mon, Jul 6, 2026 at 3:57 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
   > >
   > >>
   > >> I think that's because I think of pg_atomic_flag as an atomic version of
   > >> "bool". I'd assume there to be functions similar to the u32/u64
   > >> functions, like pg_atomic_read_bool() and pg_atomic_write_bool(). But
   > >> alas, neither of those functions exist. To read the value, you have to
   > >> use pg_atomic_unlocked_test_flag(), and remember that when the flag is
   > >> set, it returns *false*, which feels completely backwards to me. It
   > >> makes more sense with pg_atomic_test_set_flag(), which returns 'false'
   > >> if the flag was already set and hence was not set again, but it
   > >> nevertheless just feels wrong to me.
   > >>
   > >
   > > I am using pg_atomic_test_set_flag() as a non-wait locking, lighter
   > > than LWLock mechanism in shared buffer resizing patches. That might
   > > change in future. But I think there's use for TAS kind of semantics.
   > > But I have also got confused when looking at it as pg_atomic_bool. I
   > > think we need both.
   >
   > The functionality is still there with my patch, as the
   > pg_atomic_exchange_bool() function. Would that work for you?
   
   I think I can use it by replacing pg_atomic_test_set_flag(ab) by
   !pg_atomic_exchange_bool(ab, true). I haven't tried it. Maybe just
   define pg_atomic_test_set_flag(ab) as !pg_atomic_exchange_bool(ab,
   true) to be readable? Right now, none of the callers of
   pg_atomic_test_set_flag() check its return value (thus use it as TAS).
   So maybe we can add pg_atomic_test_set_flag() as a separate patch.
   
   -- 
   Best Wishes,
   Ashutosh Bapat