public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alvaro Herrera <[email protected]>
To: Andres Freund <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Jeff Davis <[email protected]>
Cc: [email protected]
Cc: Jaime Casanova <[email protected]>
Subject: Re: LogwrtResult contended spinlock
Date: Tue, 2 Jul 2024 13:55:49 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
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
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: LogwrtResult contended spinlock
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox