Thread

1. remove unnecessary volatile qualifiers

   Nathan Bossart <nathandbossart@gmail.com> — 2026-06-30T21:47:36Z

   I looked into some of these earlier [0], but ended up leaving them alone at
   the time.  Here is a new patch that removes all of the volatile markers in
   the tree that seemed obviously unnecessary to me.
   
   [0] https://postgr.es/m/aZX2oUcKf7IzHnnK%40nathan
   
   -- 
   nathan
   

2. Re: remove unnecessary volatile qualifiers

   Heikki Linnakangas <hlinnaka@iki.fi> — 2026-07-06T11:58:12Z

   On 01/07/2026 00:47, Nathan Bossart wrote:
   > I looked into some of these earlier [0], but ended up leaving them alone at
   > the time.  Here is a new patch that removes all of the volatile markers in
   > the tree that seemed obviously unnecessary to me.
   
   Thanks!
   
   > --- a/src/backend/access/transam/clog.c
   > +++ b/src/backend/access/transam/clog.c
   > @@ -450,7 +450,7 @@ static bool
   >  TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
   >  								XLogRecPtr lsn, int64 pageno)
   >  {
   > -	volatile PROC_HDR *procglobal = ProcGlobal;
   > +	PROC_HDR   *procglobal = ProcGlobal;
   >  	PGPROC	   *proc = MyProc;
   >  	uint32		nextidx;
   >  	uint32		wakeidx;
   
   You might want to get rid of the local variable altogether and just 
   refer to ProcGlobal directly..
   
   > @@ -512,7 +512,7 @@ ProcessProcSignalBarrier(void)
   >  {
   >  	uint64		local_gen;
   >  	uint64		shared_gen;
   > -	volatile uint32 flags;
   > +	uint32		flags;
   >  
   >  	Assert(MyProcSignalSlot);
   >  
   
   Are you sure about this one? 'flags' is used in the PG_TRY/CATCH block 
   that follows. It is modified in the PG_TRY(), here:
   
   > 
   > 				/*
   > 				 * To avoid an infinite loop, we must always unset the bit in
   > 				 * flags.
   > 				 */
   > 				BARRIER_CLEAR_BIT(flags, type);
   
   and read later in the PG_CATCH() block.
   
   
   The rest looks OK to me.
   
   - Heikki
   
   
   
   

3. Re: remove unnecessary volatile qualifiers

   Nathan Bossart <nathandbossart@gmail.com> — 2026-07-06T15:47:03Z

   On Mon, Jul 06, 2026 at 02:58:12PM +0300, Heikki Linnakangas wrote:
   > On 01/07/2026 00:47, Nathan Bossart wrote:
   >> I looked into some of these earlier [0], but ended up leaving them alone at
   >> the time.  Here is a new patch that removes all of the volatile markers in
   >> the tree that seemed obviously unnecessary to me.
   > 
   > Thanks!
   
   Thanks for reviewing.
   
   >> --- a/src/backend/access/transam/clog.c
   >> +++ b/src/backend/access/transam/clog.c
   >> @@ -450,7 +450,7 @@ static bool
   >>  TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
   >>  								XLogRecPtr lsn, int64 pageno)
   >>  {
   >> -	volatile PROC_HDR *procglobal = ProcGlobal;
   >> +	PROC_HDR   *procglobal = ProcGlobal;
   >>  	PGPROC	   *proc = MyProc;
   >>  	uint32		nextidx;
   >>  	uint32		wakeidx;
   > 
   > You might want to get rid of the local variable altogether and just refer to
   > ProcGlobal directly..
   
   Done.
   
   >> @@ -512,7 +512,7 @@ ProcessProcSignalBarrier(void)
   >>  {
   >>  	uint64		local_gen;
   >>  	uint64		shared_gen;
   >> -	volatile uint32 flags;
   >> +	uint32		flags;
   >>  	Assert(MyProcSignalSlot);
   > 
   > Are you sure about this one? 'flags' is used in the PG_TRY/CATCH block that
   > follows. It is modified in the PG_TRY(), here:
   > 
   >> 
   >> 				/*
   >> 				 * To avoid an infinite loop, we must always unset the bit in
   >> 				 * flags.
   >> 				 */
   >> 				BARRIER_CLEAR_BIT(flags, type);
   > 
   > and read later in the PG_CATCH() block.
   
   Whoops.  You are right.  I reverted this part.
   
   -- 
   nathan
   

4. Re: remove unnecessary volatile qualifiers

   Nathan Bossart <nathandbossart@gmail.com> — 2026-07-07T16:03:13Z

   Committed.
   
   -- 
   nathan