Re: LogwrtResult contended spinlock - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: LogwrtResult contended spinlock
Date
Msg-id 202407021155.s3f4knnte6gv@alvherre.pgsql
Whole thread Raw
In response to Re: LogwrtResult contended spinlock  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
On 2024-Jul-01, Alvaro Herrera wrote:

> On 2024-Jul-01, Andres Freund wrote:
> 
> > On 2024-07-01 11:10:24 +0200, Alvaro Herrera wrote:
> > > In the meantime I noticed that pg_attribute_aligned() is not supported
> > > in every platform/compiler, so for safety sake I think it's better to go
> > > with what we do for PGAlignedBlock: use a union with a double member.
> > > That should be 8-byte aligned on x86 as well, unless I misunderstand.
> > 
> > If a platform wants to support 8 byte atomics, it better provides a way to
> > make variables 8 bytes aligned. We already rely on that, actually. See use of
> > pg_attribute_aligned in e.g. src/include/port/atomics/generic-msvc.h

I noticed that all definitions have the value struct member named "value", so I
gave this a quick try, and hit another issue: the "expected" argument of
pg_atomic_compare_exchange_u64_impl() is not volatile, which
pg_atomic_uint64.value is, so we get this warning:

In file included from /pgsql/source/master/src/include/storage/lwlock.h:21,
                 from /pgsql/source/master/src/include/storage/lock.h:23,
                 from /pgsql/source/master/src/include/access/twophase.h:20,
                 from /pgsql/source/master/src/backend/access/transam/xlog.c:57:
/pgsql/source/master/src/include/port/atomics.h: In function ‘pg_atomic_monotonic_advance_u64’:
/pgsql/source/master/src/include/port/atomics.h:600:62: warning: passing argument 2 of
‘pg_atomic_compare_exchange_u64_impl’discards ‘volatile’ qualifier from pointer target type [-Wdiscarded-qualifiers]
 
  600 |                 if (pg_atomic_compare_exchange_u64_impl(ptr, &currval.value, target_))
      |                                                              ^~~~~~~~~~~~~~
In file included from /pgsql/source/master/src/include/port/atomics.h:69:
/pgsql/source/master/src/include/port/atomics/arch-x86.h:206:81: note: expected ‘uint64 *’ {aka ‘long unsigned int *’}
butargument is of type ‘volatile uint64 *’ {aka ‘volatile long unsigned int *’}
 
  206 |                                                                         uint64 *expected, uint64 newval)
      |                                                                         ~~~~~~~~^~~~~~~~

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Most hackers will be perfectly comfortable conceptualizing users as entropy
 sources, so let's move on."                               (Nathaniel Smith)
      https://mail.gnu.org/archive/html/monotone-devel/2007-01/msg00080.html



pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: Re: plenty code is confused about function level static
Next
From: Alexander Lakhin
Date:
Subject: Re: Improving tracking/processing of buildfarm test failures