Make the read of the current seq->value atomic, i.e., not needing to acquire the global mutex when reading it. On 64-bit systems, this incurs no overhead, and it will avoid the mutex and potentially a system call.
For incrementing the value followed by waking up the threads, we are still taking the mutex, so the current behavior is not changing. The seq_read() behavior is already defined as, "Returns seq's current sequence number (which could change immediately)". So the change should not impact the current behavior. Signed-off-by: Eelco Chaudron <[email protected]> --- v2: Update acquire and release semantics for seq->value. lib/ovs-rcu.c | 2 +- lib/seq.c | 32 +++++++++++--------------------- lib/seq.h | 1 - 3 files changed, 12 insertions(+), 23 deletions(-) diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c index 946aa04d1..9e07d9bab 100644 --- a/lib/ovs-rcu.c +++ b/lib/ovs-rcu.c @@ -170,7 +170,7 @@ ovsrcu_try_quiesce(void) ovs_assert(!single_threaded()); perthread = ovsrcu_perthread_get(); if (!seq_try_lock()) { - perthread->seqno = seq_read_protected(global_seqno); + perthread->seqno = seq_read(global_seqno); if (perthread->cbset) { ovsrcu_flush_cbset__(perthread, true); } diff --git a/lib/seq.c b/lib/seq.c index 99e5bf8bd..7c2fa0c69 100644 --- a/lib/seq.c +++ b/lib/seq.c @@ -32,7 +32,7 @@ COVERAGE_DEFINE(seq_change); /* A sequence number object. */ struct seq { - uint64_t value OVS_GUARDED; + atomic_uint64_t value; struct hmap waiters OVS_GUARDED; /* Contains 'struct seq_waiter's. */ }; @@ -72,6 +72,7 @@ static void seq_wake_waiters(struct seq *) OVS_REQUIRES(seq_mutex); struct seq * OVS_EXCLUDED(seq_mutex) seq_create(void) { + uint64_t seq_value; struct seq *seq; seq_init(); @@ -81,7 +82,8 @@ seq_create(void) COVERAGE_INC(seq_change); ovs_mutex_lock(&seq_mutex); - seq->value = seq_next++; + seq_value = seq_next++; + atomic_store_relaxed(&seq->value, seq_value); hmap_init(&seq->waiters); ovs_mutex_unlock(&seq_mutex); @@ -126,9 +128,11 @@ void seq_change_protected(struct seq *seq) OVS_REQUIRES(seq_mutex) { + uint64_t seq_value = seq_next++; + COVERAGE_INC(seq_change); - seq->value = seq_next++; + atomic_store_explicit(&seq->value, seq_value, memory_order_release); seq_wake_waiters(seq); } @@ -143,18 +147,6 @@ seq_change(struct seq *seq) ovs_mutex_unlock(&seq_mutex); } -/* Returns 'seq''s current sequence number (which could change immediately). - * - * seq_read() and seq_wait() can be used together to yield a race-free wakeup - * when an object changes, even without an ability to lock the object. See - * Usage in seq.h for details. */ -uint64_t -seq_read_protected(const struct seq *seq) - OVS_REQUIRES(seq_mutex) -{ - return seq->value; -} - /* Returns 'seq''s current sequence number (which could change immediately). * * seq_read() and seq_wait() can be used together to yield a race-free wakeup @@ -162,14 +154,12 @@ seq_read_protected(const struct seq *seq) * Usage in seq.h for details. */ uint64_t seq_read(const struct seq *seq) - OVS_EXCLUDED(seq_mutex) { uint64_t value; - ovs_mutex_lock(&seq_mutex); - value = seq_read_protected(seq); - ovs_mutex_unlock(&seq_mutex); - + /* Note that the odd CONST_CAST() is here to keep sparse happy. */ + atomic_read_explicit(&CONST_CAST(struct seq *, seq)->value, &value, + memory_order_acquire); return value; } @@ -226,7 +216,7 @@ seq_wait_at(const struct seq *seq_, uint64_t value, const char *where) struct seq *seq = CONST_CAST(struct seq *, seq_); ovs_mutex_lock(&seq_mutex); - if (value == seq->value) { + if (value == seq_read(seq_)) { seq_wait__(seq, value, where); } else { poll_immediate_wake_at(where); diff --git a/lib/seq.h b/lib/seq.h index c88b9d1c8..fcfa01037 100644 --- a/lib/seq.h +++ b/lib/seq.h @@ -128,7 +128,6 @@ void seq_unlock(void); /* For observers. */ uint64_t seq_read(const struct seq *); -uint64_t seq_read_protected(const struct seq *); void seq_wait_at(const struct seq *, uint64_t value, const char *where); #define seq_wait(seq, value) seq_wait_at(seq, value, OVS_SOURCE_LOCATOR) _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
