From 36ed4c24810223c6030532a9506c299899136b94 Mon Sep 17 00:00:00 2001 From: kib Date: Tue, 8 Sep 2015 08:41:07 +0000 Subject: In the pthread_once(), if the initializer has already run, then the calling thread is supposed to see accesses issued by the initializer. This means that the read of the once_control->state variable should have an acquire semantic. Use atomic_thread_fence_acq() when the value read is ONCE_DONE, instead of straightforward atomic_load_acq(), to only put a barrier when needed (*). On the other hand, the updates of the once_control->state with the intermediate progress state do not need to synchronize with other state accesses, remove _acq suffix. Reviewed by: alc (previous version) Suggested by: alc (*) Sponsored by: The FreeBSD Foundation MFC after: 1 week --- lib/libthr/thread/thr_once.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'lib/libthr/thread/thr_once.c') diff --git a/lib/libthr/thread/thr_once.c b/lib/libthr/thread/thr_once.c index 4f70374..4a6b4d7 100644 --- a/lib/libthr/thread/thr_once.c +++ b/lib/libthr/thread/thr_once.c @@ -68,13 +68,15 @@ _pthread_once(pthread_once_t *once_control, void (*init_routine) (void)) for (;;) { state = once_control->state; - if (state == ONCE_DONE) + if (state == ONCE_DONE) { + atomic_thread_fence_acq(); return (0); + } if (state == ONCE_NEVER_DONE) { - if (atomic_cmpset_acq_int(&once_control->state, state, ONCE_IN_PROGRESS)) + if (atomic_cmpset_int(&once_control->state, state, ONCE_IN_PROGRESS)) break; } else if (state == ONCE_IN_PROGRESS) { - if (atomic_cmpset_acq_int(&once_control->state, state, ONCE_WAIT)) + if (atomic_cmpset_int(&once_control->state, state, ONCE_WAIT)) _thr_umtx_wait_uint(&once_control->state, ONCE_WAIT, NULL, 0); } else if (state == ONCE_WAIT) { _thr_umtx_wait_uint(&once_control->state, state, NULL, 0); -- cgit v1.1 From b522c482ce6b0cc8601a1552392c4fec6a4169e5 Mon Sep 17 00:00:00 2001 From: kib Date: Tue, 8 Sep 2015 08:48:53 +0000 Subject: Style. Use ANSI definition, wrap long lines, no initialization in declaration for locals. Sponsored by: The FreeBSD Foundation MFC after: 1 week --- lib/libthr/thread/thr_once.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) (limited to 'lib/libthr/thread/thr_once.c') diff --git a/lib/libthr/thread/thr_once.c b/lib/libthr/thread/thr_once.c index 4a6b4d7..208b703 100644 --- a/lib/libthr/thread/thr_once.c +++ b/lib/libthr/thread/thr_once.c @@ -50,9 +50,11 @@ __weak_reference(_pthread_once, pthread_once); static void once_cancel_handler(void *arg) { - pthread_once_t *once_control = arg; + pthread_once_t *once_control; - if (atomic_cmpset_rel_int(&once_control->state, ONCE_IN_PROGRESS, ONCE_NEVER_DONE)) + once_control = arg; + if (atomic_cmpset_rel_int(&once_control->state, ONCE_IN_PROGRESS, + ONCE_NEVER_DONE)) return; atomic_store_rel_int(&once_control->state, ONCE_NEVER_DONE); _thr_umtx_wake(&once_control->state, INT_MAX, 0); @@ -73,13 +75,17 @@ _pthread_once(pthread_once_t *once_control, void (*init_routine) (void)) return (0); } if (state == ONCE_NEVER_DONE) { - if (atomic_cmpset_int(&once_control->state, state, ONCE_IN_PROGRESS)) + if (atomic_cmpset_int(&once_control->state, state, + ONCE_IN_PROGRESS)) break; } else if (state == ONCE_IN_PROGRESS) { - if (atomic_cmpset_int(&once_control->state, state, ONCE_WAIT)) - _thr_umtx_wait_uint(&once_control->state, ONCE_WAIT, NULL, 0); + if (atomic_cmpset_int(&once_control->state, state, + ONCE_WAIT)) + _thr_umtx_wait_uint(&once_control->state, + ONCE_WAIT, NULL, 0); } else if (state == ONCE_WAIT) { - _thr_umtx_wait_uint(&once_control->state, state, NULL, 0); + _thr_umtx_wait_uint(&once_control->state, state, + NULL, 0); } else return (EINVAL); } @@ -88,7 +94,8 @@ _pthread_once(pthread_once_t *once_control, void (*init_routine) (void)) THR_CLEANUP_PUSH(curthread, once_cancel_handler, once_control); init_routine(); THR_CLEANUP_POP(curthread, 0); - if (atomic_cmpset_rel_int(&once_control->state, ONCE_IN_PROGRESS, ONCE_DONE)) + if (atomic_cmpset_rel_int(&once_control->state, ONCE_IN_PROGRESS, + ONCE_DONE)) return (0); atomic_store_rel_int(&once_control->state, ONCE_DONE); _thr_umtx_wake(&once_control->state, INT_MAX, 0); @@ -96,6 +103,6 @@ _pthread_once(pthread_once_t *once_control, void (*init_routine) (void)) } void -_thr_once_init() +_thr_once_init(void) { } -- cgit v1.1