Commit 5882ca66 authored by Noah Misch's avatar Noah Misch

Call xlc __isync() after, not before, associated compare-and-swap.

Architecture reference material specifies this order, and s_lock.h
inline assembly agrees.  The former order failed to provide mutual
exclusion to lwlock.c and perhaps to other clients.  The two xlc
buildfarm members, hornet and mandrill, have failed sixteen times with
duplicate key errors involving pg_class_oid_index or pg_type_oid_index.
Back-patch to 9.5, where commit b64d92f1
introduced atomics.

Reviewed by Andres Freund and Tom Lane.
parent 481725c0
/exprparse.c /exprparse.c
/exprscan.c /exprscan.c
/pgbench /pgbench
/tmp_check/
...@@ -40,3 +40,9 @@ clean distclean: ...@@ -40,3 +40,9 @@ clean distclean:
maintainer-clean: distclean maintainer-clean: distclean
rm -f exprparse.c exprscan.c rm -f exprparse.c exprscan.c
check:
$(prove_check)
installcheck:
$(prove_installcheck)
use strict;
use warnings;
use PostgresNode;
use TestLib;
use Test::More tests => 3;
# Test concurrent insertion into table with UNIQUE oid column. DDL expects
# GetNewOidWithIndex() to successfully avoid violating uniqueness for indexes
# like pg_class_oid_index and pg_proc_oid_index. This indirectly exercises
# LWLock and spinlock concurrency. This test makes a 5-MiB table.
my $node = get_new_node('main');
$node->init;
$node->start;
$node->psql('postgres',
'CREATE UNLOGGED TABLE oid_tbl () WITH OIDS; '
. 'ALTER TABLE oid_tbl ADD UNIQUE (oid);');
my $script = $node->basedir . '/pgbench_script';
append_to_file($script,
'INSERT INTO oid_tbl SELECT FROM generate_series(1,1000);');
$node->command_like(
[ qw(pgbench --no-vacuum --client=5 --protocol=prepared
--transactions=25 --file), $script ],
qr{processed: 125/125},
'concurrent OID generation');
...@@ -40,19 +40,23 @@ static inline bool ...@@ -40,19 +40,23 @@ static inline bool
pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
uint32 *expected, uint32 newval) uint32 *expected, uint32 newval)
{ {
/*
* XXX: __compare_and_swap is defined to take signed parameters, but that
* shouldn't matter since we don't perform any arithmetic operations.
*/
bool ret = __compare_and_swap((volatile int*)&ptr->value,
(int *)expected, (int)newval);
/* /*
* xlc's documentation tells us: * xlc's documentation tells us:
* "If __compare_and_swap is used as a locking primitive, insert a call to * "If __compare_and_swap is used as a locking primitive, insert a call to
* the __isync built-in function at the start of any critical sections." * the __isync built-in function at the start of any critical sections."
*
* The critical section begins immediately after __compare_and_swap().
*/ */
__isync(); __isync();
/* return ret;
* XXX: __compare_and_swap is defined to take signed parameters, but that
* shouldn't matter since we don't perform any arithmetic operations.
*/
return __compare_and_swap((volatile int*)&ptr->value,
(int *)expected, (int)newval);
} }
#define PG_HAVE_ATOMIC_FETCH_ADD_U32 #define PG_HAVE_ATOMIC_FETCH_ADD_U32
...@@ -69,10 +73,12 @@ static inline bool ...@@ -69,10 +73,12 @@ static inline bool
pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr,
uint64 *expected, uint64 newval) uint64 *expected, uint64 newval)
{ {
bool ret = __compare_and_swaplp((volatile long*)&ptr->value,
(long *)expected, (long)newval);
__isync(); __isync();
return __compare_and_swaplp((volatile long*)&ptr->value, return ret;
(long *)expected, (long)newval);;
} }
#define PG_HAVE_ATOMIC_FETCH_ADD_U64 #define PG_HAVE_ATOMIC_FETCH_ADD_U64
......
...@@ -94,6 +94,7 @@ if exist src\bin\pg_basebackup\tmp_check rd /s /q src\bin\pg_basebackup\tmp_chec ...@@ -94,6 +94,7 @@ if exist src\bin\pg_basebackup\tmp_check rd /s /q src\bin\pg_basebackup\tmp_chec
if exist src\bin\pg_config\tmp_check rd /s /q src\bin\pg_config\tmp_check if exist src\bin\pg_config\tmp_check rd /s /q src\bin\pg_config\tmp_check
if exist src\bin\pg_ctl\tmp_check rd /s /q src\bin\pg_ctl\tmp_check if exist src\bin\pg_ctl\tmp_check rd /s /q src\bin\pg_ctl\tmp_check
if exist src\bin\pg_rewind\tmp_check rd /s /q src\bin\pg_rewind\tmp_check if exist src\bin\pg_rewind\tmp_check rd /s /q src\bin\pg_rewind\tmp_check
if exist src\bin\pgbench\tmp_check rd /s /q src\bin\pgbench\tmp_check
if exist src\bin\scripts\tmp_check rd /s /q src\bin\scripts\tmp_check if exist src\bin\scripts\tmp_check rd /s /q src\bin\scripts\tmp_check
REM Clean up datafiles built with contrib REM Clean up datafiles built with contrib
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment