Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit1f3bbe0

Browse files
committed
Fix and improve pg_atomic_flag fallback implementation.
The atomics fallback implementation for pg_atomic_flag was broken,returning the inverted value from pg_atomic_test_set_flag(). This wasunnoticed because a) atomic flags were unused until recently b) thetest code wasn't run when the fallback implementation was inuse (because it didn't allow to test for some edge cases).Fix the bug, and improve the fallback so it has the same behaviour asthe non-fallback implementation in the problematic edge cases. Thatbreaks ABI compatibility in the back branches when fallbacks are inuse, but given they were broken until now...Author: Andres FreundReported-by: Daniel GustafssonDiscussion:https://postgr.es/m/FB948276-7B32-4B77-83E6-D00167F8EEB4@yesql.sehttps://postgr.es/m/20180406233854.uni2h3mbnveczl32@alap3.anarazel.deBackpatch: 9.5-, where the atomics abstraction was introduced.
1 parente177450 commit1f3bbe0

File tree

3 files changed

+21
-27
lines changed

3 files changed

+21
-27
lines changed

‎src/backend/port/atomics.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,18 +69,35 @@ pg_atomic_init_flag_impl(volatile pg_atomic_flag *ptr)
6969
#else
7070
SpinLockInit((slock_t*)&ptr->sema);
7171
#endif
72+
73+
ptr->value= false;
7274
}
7375

7476
bool
7577
pg_atomic_test_set_flag_impl(volatilepg_atomic_flag*ptr)
7678
{
77-
returnTAS((slock_t*)&ptr->sema);
79+
uint32oldval;
80+
81+
SpinLockAcquire((slock_t*)&ptr->sema);
82+
oldval=ptr->value;
83+
ptr->value= true;
84+
SpinLockRelease((slock_t*)&ptr->sema);
85+
86+
returnoldval==0;
7887
}
7988

8089
void
8190
pg_atomic_clear_flag_impl(volatilepg_atomic_flag*ptr)
8291
{
83-
S_UNLOCK((slock_t*)&ptr->sema);
92+
SpinLockAcquire((slock_t*)&ptr->sema);
93+
ptr->value= false;
94+
SpinLockRelease((slock_t*)&ptr->sema);
95+
}
96+
97+
bool
98+
pg_atomic_unlocked_test_flag_impl(volatilepg_atomic_flag*ptr)
99+
{
100+
returnptr->value==0;
84101
}
85102

86103
#endif/* PG_HAVE_ATOMIC_FLAG_SIMULATION */

‎src/include/port/atomics/fallback.h

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ typedef struct pg_atomic_flag
8080
#else
8181
intsema;
8282
#endif
83+
volatileboolvalue;
8384
}pg_atomic_flag;
8485

8586
#endif/* PG_HAVE_ATOMIC_FLAG_SUPPORT */
@@ -114,17 +115,7 @@ extern bool pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr);
114115
externvoidpg_atomic_clear_flag_impl(volatilepg_atomic_flag*ptr);
115116

116117
#definePG_HAVE_ATOMIC_UNLOCKED_TEST_FLAG
117-
staticinlinebool
118-
pg_atomic_unlocked_test_flag_impl(volatilepg_atomic_flag*ptr)
119-
{
120-
/*
121-
* Can't do this efficiently in the semaphore based implementation - we'd
122-
* have to try to acquire the semaphore - so always return true. That's
123-
* correct, because this is only an unlocked test anyway. Do this in the
124-
* header so compilers can optimize the test away.
125-
*/
126-
return true;
127-
}
118+
externboolpg_atomic_unlocked_test_flag_impl(volatilepg_atomic_flag*ptr);
128119

129120
#endif/* PG_HAVE_ATOMIC_FLAG_SIMULATION */
130121

‎src/test/regress/regress.c

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,6 @@ wait_pid(PG_FUNCTION_ARGS)
898898
PG_RETURN_VOID();
899899
}
900900

901-
#ifndefPG_HAVE_ATOMIC_FLAG_SIMULATION
902901
staticvoid
903902
test_atomic_flag(void)
904903
{
@@ -928,7 +927,6 @@ test_atomic_flag(void)
928927

929928
pg_atomic_clear_flag(&flag);
930929
}
931-
#endif/* PG_HAVE_ATOMIC_FLAG_SIMULATION */
932930

933931
staticvoid
934932
test_atomic_uint32(void)
@@ -1113,19 +1111,7 @@ PG_FUNCTION_INFO_V1(test_atomic_ops);
11131111
Datum
11141112
test_atomic_ops(PG_FUNCTION_ARGS)
11151113
{
1116-
/* ---
1117-
* Can't run the test under the semaphore emulation, it doesn't handle
1118-
* checking two edge cases well:
1119-
* - pg_atomic_unlocked_test_flag() always returns true
1120-
* - locking a already locked flag blocks
1121-
* it seems better to not test the semaphore fallback here, than weaken
1122-
* the checks for the other cases. The semaphore code will be the same
1123-
* everywhere, whereas the efficient implementations wont.
1124-
* ---
1125-
*/
1126-
#ifndefPG_HAVE_ATOMIC_FLAG_SIMULATION
11271114
test_atomic_flag();
1128-
#endif
11291115

11301116
test_atomic_uint32();
11311117

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp