Message-ID: <ca45cde2-1247-4146-ba64-363349214dbf@iki.fi>
Date: Mon, 6 Jul 2026 14:58:12 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: remove unnecessary volatile qualifiers
To: Nathan Bossart <nathandbossart@gmail.com>, pgsql-hackers@postgresql.org
References: <akQ5eJR1tCCXme8e@nathan>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <akQ5eJR1tCCXme8e@nathan>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/ca45cde2-1247-4146-ba64-363349214dbf%40iki.fi>
Precedence: bulk

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