Re: LogwrtResult contended spinlock

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 *’} but argument 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