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

Commit8c3debb

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 parenteb2a0e0 commit8c3debb

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
@@ -68,18 +68,35 @@ pg_atomic_init_flag_impl(volatile pg_atomic_flag *ptr)
6868
#else
6969
SpinLockInit((slock_t*)&ptr->sema);
7070
#endif
71+
72+
ptr->value= false;
7173
}
7274

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

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

85102
#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 */
@@ -132,17 +133,7 @@ extern bool pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr);
132133
externvoidpg_atomic_clear_flag_impl(volatilepg_atomic_flag*ptr);
133134

134135
#definePG_HAVE_ATOMIC_UNLOCKED_TEST_FLAG
135-
staticinlinebool
136-
pg_atomic_unlocked_test_flag_impl(volatilepg_atomic_flag*ptr)
137-
{
138-
/*
139-
* Can't do this efficiently in the semaphore based implementation - we'd
140-
* have to try to acquire the semaphore - so always return true. That's
141-
* correct, because this is only an unlocked test anyway. Do this in the
142-
* header so compilers can optimize the test away.
143-
*/
144-
return true;
145-
}
136+
externboolpg_atomic_unlocked_test_flag_impl(volatilepg_atomic_flag*ptr);
146137

147138
#endif/* PG_HAVE_ATOMIC_FLAG_SIMULATION */
148139

‎src/test/regress/regress.c

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,6 @@ wait_pid(PG_FUNCTION_ARGS)
633633
PG_RETURN_VOID();
634634
}
635635

636-
#ifndefPG_HAVE_ATOMIC_FLAG_SIMULATION
637636
staticvoid
638637
test_atomic_flag(void)
639638
{
@@ -663,7 +662,6 @@ test_atomic_flag(void)
663662

664663
pg_atomic_clear_flag(&flag);
665664
}
666-
#endif/* PG_HAVE_ATOMIC_FLAG_SIMULATION */
667665

668666
staticvoid
669667
test_atomic_uint32(void)
@@ -846,19 +844,7 @@ PG_FUNCTION_INFO_V1(test_atomic_ops);
846844
Datum
847845
test_atomic_ops(PG_FUNCTION_ARGS)
848846
{
849-
/* ---
850-
* Can't run the test under the semaphore emulation, it doesn't handle
851-
* checking two edge cases well:
852-
* - pg_atomic_unlocked_test_flag() always returns true
853-
* - locking a already locked flag blocks
854-
* it seems better to not test the semaphore fallback here, than weaken
855-
* the checks for the other cases. The semaphore code will be the same
856-
* everywhere, whereas the efficient implementations wont.
857-
* ---
858-
*/
859-
#ifndefPG_HAVE_ATOMIC_FLAG_SIMULATION
860847
test_atomic_flag();
861-
#endif
862848

863849
test_atomic_uint32();
864850

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp