[ovs-dev] [PATCH v2 3/3] lib/ovs-thread: Avoid atomic read in ovsthread_once_start().
Jarno Rajahalme
jrajahalme at nicira.com
Fri Aug 29 21:10:44 UTC 2014
We can use a normal bool and rely on the mutex_lock/unlock and an
atomic_thread_fence for synchronization.
Also flip the return value of ovsthread_once_start__() to match the
one of ovsthread_once_start().
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
v3: Simplified.
lib/ovs-thread.c | 14 ++++++++++----
lib/ovs-thread.h | 22 ++++++++--------------
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 955c1c1..e2c3971 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -367,17 +367,23 @@ bool
ovsthread_once_start__(struct ovsthread_once *once)
{
ovs_mutex_lock(&once->mutex);
- if (!ovsthread_once_is_done__(once)) {
- return false;
+ /* Mutex synchronizes memory, so we get the current value of 'done'. */
+ if (!once->done) {
+ return true;
}
ovs_mutex_unlock(&once->mutex);
- return true;
+ return false;
}
void
ovsthread_once_done(struct ovsthread_once *once)
{
- atomic_store(&once->done, true);
+ /* We need release semantics here, so that the following store may not
+ * be moved ahead of any of the preceding initialization operations.
+ * A release atomic_thread_fence provides that prior memory accesses
+ * will not be reordered to take place after the following store. */
+ atomic_thread_fence(memory_order_release);
+ once->done = true;
ovs_mutex_unlock(&once->mutex);
}
diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index 962e867..42808b9 100644
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -533,13 +533,13 @@ void *ovsthread_getspecific(ovsthread_key_t);
*/
struct ovsthread_once {
- atomic_bool done;
+ bool done; /* Non-atomic, false negatives possible. */
struct ovs_mutex mutex;
};
#define OVSTHREAD_ONCE_INITIALIZER \
{ \
- ATOMIC_VAR_INIT(false), \
+ false, \
OVS_MUTEX_INITIALIZER, \
}
@@ -549,16 +549,7 @@ void ovsthread_once_done(struct ovsthread_once *once)
OVS_RELEASES(once->mutex);
bool ovsthread_once_start__(struct ovsthread_once *once)
- OVS_TRY_LOCK(false, once->mutex);
-
-static inline bool
-ovsthread_once_is_done__(struct ovsthread_once *once)
-{
- bool done;
-
- atomic_read_relaxed(&once->done, &done);
- return done;
-}
+ OVS_TRY_LOCK(true, once->mutex);
/* Returns true if this is the first call to ovsthread_once_start() for
* 'once'. In this case, the caller should perform whatever initialization
@@ -570,8 +561,11 @@ ovsthread_once_is_done__(struct ovsthread_once *once)
static inline bool
ovsthread_once_start(struct ovsthread_once *once)
{
- return OVS_UNLIKELY(!ovsthread_once_is_done__(once)
- && !ovsthread_once_start__(once));
+ /* We may be reading 'done' at the same time as the first thread
+ * is writing on it, or we can be using a stale copy of it. The
+ * worst that can happen is that we call ovsthread_once_start__()
+ * once when strictly not necessary. */
+ return OVS_UNLIKELY(!once->done && ovsthread_once_start__(once));
}
/* Thread ID.
--
1.7.10.4
More information about the dev
mailing list