database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-01 01:04:50 +03:00
Code

Fix and improve pg_atomic_flag fallback implementation.

Browse Source 
The atomics fallback implementation for pg_atomic_flag was broken,
returning the inverted value from pg_atomic_test_set_flag().  This was
unnoticed because a) atomic flags were unused until recently b) the
test code wasn't run when the fallback implementation was in
use (because it didn't allow to test for some edge cases).

Fix the bug, and improve the fallback so it has the same behaviour as
the non-fallback implementation in the problematic edge cases. That
breaks ABI compatibility in the back branches when fallbacks are in
use, but given they were broken until now...

Author: Andres Freund
Reported-by: Daniel Gustafsson
Discussion:
    https://postgr.es/m/FB948276-7B32-4B77-83E6-D00167F8EEB4@yesql.se
    https://postgr.es/m/20180406233854.uni2h3mbnveczl32@alap3.anarazel.de
Backpatch: 9.5-, where the atomics abstraction was introduced.
This commit is contained in:
Andres Freund 2018-04-06 20:01:44 -07:00
parent 29ab1e24a6
commit 5b7fc7b037
3 changed files with 21 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/backend/port/atomics.c
View File

@ -68,18 +68,35 @@ pg_atomic_init_flag_impl(volatile pg_atomic_flag *ptr)
#else
SpinLockInit((slock_t *) &ptr->sema);
#endif
ptr->value = false;
}
bool
pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr)
{
return TAS((slock_t *) &ptr->sema);
uint32 oldval;
SpinLockAcquire((slock_t *) &ptr->sema);
oldval = ptr->value;
ptr->value = true;
SpinLockRelease((slock_t *) &ptr->sema);
return oldval == 0;
}
void
pg_atomic_clear_flag_impl(volatile pg_atomic_flag *ptr)
{
S_UNLOCK((slock_t *) &ptr->sema);
SpinLockAcquire((slock_t *) &ptr->sema);
ptr->value = false;
SpinLockRelease((slock_t *) &ptr->sema);
}
bool
pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr)
{
return ptr->value == 0;
}
#endif /* PG_HAVE_ATOMIC_FLAG_SIMULATION */

13
src/include/port/atomics/fallback.h
View File

@ -80,6 +80,7 @@ typedef struct pg_atomic_flag
#else
int sema;
#endif
volatile bool value;
} pg_atomic_flag;
#endif /* PG_HAVE_ATOMIC_FLAG_SUPPORT */
@ -132,17 +133,7 @@ extern bool pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr);
extern void pg_atomic_clear_flag_impl(volatile pg_atomic_flag *ptr);
#define PG_HAVE_ATOMIC_UNLOCKED_TEST_FLAG
static inline bool
pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr)
{
/*
* Can't do this efficiently in the semaphore based implementation - we'd
* have to try to acquire the semaphore - so always return true. That's
* correct, because this is only an unlocked test anyway. Do this in the
* header so compilers can optimize the test away.
*/
return true;
}
extern bool pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr);
#endif /* PG_HAVE_ATOMIC_FLAG_SIMULATION */

14
src/test/regress/regress.c
View File

@ -881,7 +881,6 @@ wait_pid(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
#ifndef PG_HAVE_ATOMIC_FLAG_SIMULATION
static void
test_atomic_flag(void)
{
@ -911,7 +910,6 @@ test_atomic_flag(void)
pg_atomic_clear_flag(&flag);
}
#endif /* PG_HAVE_ATOMIC_FLAG_SIMULATION */
static void
test_atomic_uint32(void)
@ -1094,19 +1092,7 @@ PG_FUNCTION_INFO_V1(test_atomic_ops);
Datum
test_atomic_ops(PG_FUNCTION_ARGS)
{
/* ---
* Can't run the test under the semaphore emulation, it doesn't handle
* checking two edge cases well:
* - pg_atomic_unlocked_test_flag() always returns true
* - locking a already locked flag blocks
* it seems better to not test the semaphore fallback here, than weaken
* the checks for the other cases. The semaphore code will be the same
* everywhere, whereas the efficient implementations wont.
* ---
*/
#ifndef PG_HAVE_ATOMIC_FLAG_SIMULATION
test_atomic_flag();
#endif
test_atomic_uint32();