remove unnecessary volatile qualifiers
remove unnecessary volatile qualifiers
От:
Nathan Bossart <nathandbossart@gmail.com>
Дата:
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
Re: remove unnecessary volatile qualifiers
От:
Nathan Bossart <nathandbossart@gmail.com>
Дата:
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
Re: remove unnecessary volatile qualifiers
От:
Heikki Linnakangas <hlinnaka@iki.fi>
Дата:
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
