pgsql: Improve 64bit atomics support.

Andres Freund wrote:

Improve 64bit atomics support.

When adding atomics back in b64d92f1a, I added 64bit support as
optional; there wasn't yet a direct user in sight. That turned out to
be a bit short-sighted, it'd already have been useful a number of times.

Seems like this killed an arapaima:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=arapaima&dt=2017-04-07%2022%3A06%3A59

Program terminated with signal 6, Aborted.
#0 0x00c6a402 in __kernel_vsyscall ()
#0 0x00c6a402 in __kernel_vsyscall ()
#1 0x00284b10 in raise () from /lib/libc.so.6
#2 0x00286421 in abort () from /lib/libc.so.6
#3 0x084d967e in ExceptionalCondition (
conditionName=0xe19dac "(((uintptr_t) ((uintptr_t)(ptr)) + ((8) - 1)) & ~((uintptr_t) ((8) - 1))) != (uintptr_t)(ptr)",
errorType=0xe19831 "UnalignedPointer",
fileName=0xe19d88 "../../../src/include/port/atomics.h", lineNumber=428)
at assert.c:54
#4 0x00e189b0 in pg_atomic_init_u64 ()
at ../../../src/include/port/atomics.h:428
#5 test_atomic_uint64 () at regress.c:1007
#6 0x00e1905d in test_atomic_ops (fcinfo=0x9362584) at regress.c:1097
#7 0x08273ab2 in ExecInterpExpr (state=0x9362510, econtext=0x93622e0,
isnull=0xbfbc1a4b "") at execExprInterp.c:650
#8 0x082990a7 in ExecEvalExprSwitchContext (node=0x9362294)
at ../../../src/include/executor/executor.h:289
#9 ExecProject (node=0x9362294)
at ../../../src/include/executor/executor.h:323
#10 ExecResult (node=0x9362294) at nodeResult.c:132
#11 0x0827c6d5 in ExecProcNode (node=0x9362294) at execProcnode.c:416
#12 0x0827a08d in ExecutePlan (queryDesc=0x92daf38,
direction=ForwardScanDirection, count=0, execute_once=1 '\001')
at execMain.c:1651
#13 standard_ExecutorRun (queryDesc=0x92daf38,
direction=ForwardScanDirection, count=0, execute_once=1 '\001')
at execMain.c:360
#14 0x083cd8bb in PortalRunSelect (portal=0x92d78a8,
forward=<value optimized out>, count=0, dest=0x9337728) at pquery.c:933
#15 0x083cef1e in PortalRun (portal=0x92d78a8, count=2147483647,
isTopLevel=1 '\001', run_once=1 '\001', dest=0x9337728,
altdest=0x9337728, completionTag=0xbfbc1c6a "") at pquery.c:774
#16 0x083cb38f in exec_simple_query (
query_string=0x93360b0 "SELECT test_atomic_ops();") at postgres.c:1105
#17 0x083cc640 in PostgresMain (argc=1, argv=0x92e4160,
dbname=0x92e3fe0 "regression", username=0x92e3fc4 "postgres")
at postgres.c:4075
#18 0x08349eaf in BackendStartup () at postmaster.c:4317
#19 ServerLoop () at postmaster.c:1729
#20 0x0834db50 in PostmasterMain (argc=8, argv=0x92b9968) at postmaster.c:1337
#21 0x082c01e2 in main (argc=8, argv=Cannot access memory at address 0x5aa5
) at main.c:228

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Apr 8, 2017 at 12:50 AM, Andres Freund <andres@anarazel.de> wrote:

Additionally optimize reads/writes to 64bit on a number of platforms
where aligned writes of that size are atomic. This can now be tested
with PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY.

BTW, PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY is never used.

It's likely you meant something like this.

diff --git a/src/include/port/atomics/generic.h
b/src/include/port/atomics/generic.h
new file mode 100644
index c094248..c430bf5
*** a/src/include/port/atomics/generic.h
--- b/src/include/port/atomics/generic.h
*************** pg_atomic_exchange_u64_impl(volatile pg_
*** 271,277 ****
  }
  #endif

! #ifndef PG_HAVE_ATOMIC_READ_U64
  #define PG_HAVE_ATOMIC_READ_U64
  static inline uint64
  pg_atomic_read_u64_impl(volatile pg_atomic_uint64 *ptr)
--- 271,277 ----
  }
  #endif

! #if !defined(PG_HAVE_ATOMIC_READ_U64) &&
defined(PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY)
#define PG_HAVE_ATOMIC_READ_U64
static inline uint64
pg_atomic_read_u64_impl(volatile pg_atomic_uint64 *ptr)
*************** pg_atomic_read_u64_impl(volatile pg_atom
*** 280,286 ****
}
#endif

! #ifndef PG_HAVE_ATOMIC_WRITE_U64
  #define PG_HAVE_ATOMIC_WRITE_U64
  static inline void
  pg_atomic_write_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 val)
--- 280,286 ----
  }
  #endif

! #if !defined(PG_HAVE_ATOMIC_WRITE_U64) &&
defined(PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY)
#define PG_HAVE_ATOMIC_WRITE_U64
static inline void
pg_atomic_write_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 val)

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On 2017-04-07 19:55:21 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

Improve 64bit atomics support.

When adding atomics back in b64d92f1a, I added 64bit support as
optional; there wasn't yet a direct user in sight. That turned out to
be a bit short-sighted, it'd already have been useful a number of times.

Seems like this killed an arapaima:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=arapaima&amp;dt=2017-04-07%2022%3A06%3A59

Program terminated with signal 6, Aborted.
#0 0x00c6a402 in __kernel_vsyscall ()
#0 0x00c6a402 in __kernel_vsyscall ()
#1 0x00284b10 in raise () from /lib/libc.so.6
#2 0x00286421 in abort () from /lib/libc.so.6
#3 0x084d967e in ExceptionalCondition (
conditionName=0xe19dac "(((uintptr_t) ((uintptr_t)(ptr)) + ((8) - 1)) & ~((uintptr_t) ((8) - 1))) != (uintptr_t)(ptr)",
errorType=0xe19831 "UnalignedPointer",
fileName=0xe19d88 "../../../src/include/port/atomics.h", lineNumber=428)
at assert.c:54
#4 0x00e189b0 in pg_atomic_init_u64 ()
at ../../../src/include/port/atomics.h:428

Gah, that's fairly annoying :(. We can't trivially force alignment in
the generic fallback case, because not all compilers support that. We
don't really need it the fallback case, because things are protected by
a lock - but that means we'll have to make a bunch of locks conditional
:/

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-04-07 16:36:09 -0700, Andres Freund wrote:

On 2017-04-07 19:55:21 -0300, Alvaro Herrera wrote:

Andres Freund wrote:

Improve 64bit atomics support.

When adding atomics back in b64d92f1a, I added 64bit support as
optional; there wasn't yet a direct user in sight. That turned out to
be a bit short-sighted, it'd already have been useful a number of times.

Seems like this killed an arapaima:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=arapaima&amp;dt=2017-04-07%2022%3A06%3A59

Program terminated with signal 6, Aborted.
#0 0x00c6a402 in __kernel_vsyscall ()
#0 0x00c6a402 in __kernel_vsyscall ()
#1 0x00284b10 in raise () from /lib/libc.so.6
#2 0x00286421 in abort () from /lib/libc.so.6
#3 0x084d967e in ExceptionalCondition (
conditionName=0xe19dac "(((uintptr_t) ((uintptr_t)(ptr)) + ((8) - 1)) & ~((uintptr_t) ((8) - 1))) != (uintptr_t)(ptr)",
errorType=0xe19831 "UnalignedPointer",
fileName=0xe19d88 "../../../src/include/port/atomics.h", lineNumber=428)
at assert.c:54
#4 0x00e189b0 in pg_atomic_init_u64 ()
at ../../../src/include/port/atomics.h:428

Gah, that's fairly annoying :(. We can't trivially force alignment in
the generic fallback case, because not all compilers support that. We
don't really need it the fallback case, because things are protected by
a lock - but that means we'll have to make a bunch of locks conditional
:/

Pushed an attempt at fixing this along those lines, let's hope that
works.

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-04-08 02:16:34 +0300, Alexander Korotkov wrote:

On Sat, Apr 8, 2017 at 12:50 AM, Andres Freund <andres@anarazel.de> wrote:

Additionally optimize reads/writes to 64bit on a number of platforms
where aligned writes of that size are atomic. This can now be tested
with PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY.

BTW, PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY is never used.

It's likely you meant something like this.

Gah, indeed. Pushed a fix, but you're more than welcome to verify...

- Andres

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers