[ovs-dev] [PATCH] coverage: Add coverage_try_clear() for performance-critical threads.

[Alex Wang]

For performance-critical threads like pmd threads, we currently make them
never call coverage_clear() to avoid contention over the global mutex
'coverage_mutex'.  So, even though pmd thread still keeps updating their
thread-local coverage count, the count is never attributed to the global
total.  But it is useful to have them available.

This commit makes this happen by implementing a non-contending version
of the clear function, coverage_try_clear().  The function will use
the ovs_mutex_trylock() and return immediately if the mutex cannot
be acquired.  Since threads like pmd thread are always busy-looping,
the lock will eventually be acquired.

Requested-by: Ilya Maximets <i.maxim...@samsung.com>
Signed-off-by: Alex Wang <al...@nicira.com>
---
 lib/coverage.c    |   33 +++++++++++++++++++++++++++++----
 lib/coverage.h    |    1 +
 lib/dpif-netdev.c |    2 ++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/lib/coverage.c b/lib/coverage.c
index 6121956..9584cac 100644
--- a/lib/coverage.c
+++ b/lib/coverage.c
@@ -245,9 +245,14 @@ coverage_read(struct svec *lines)
 
 /* Runs approximately every COVERAGE_CLEAR_INTERVAL amount of time to
  * synchronize per-thread counters with global counters. Every thread maintains
- * a separate timer to ensure all counters are periodically aggregated. */
-void
-coverage_clear(void)
+ * a separate timer to ensure all counters are periodically aggregated.
+ *
+ * Uses 'ovs_mutex_trylock()' if 'trylock' is true.  This is to prevent
+ * multiple performance-critical threads contending over the 'coverage_mutex'.
+ *
+ * */
+static void
+coverage_clear__(bool trylock)
 {
     long long int now, *thread_time;
 
@@ -262,7 +267,15 @@ coverage_clear(void)
     if (now >= *thread_time) {
         size_t i;
 
-        ovs_mutex_lock(&coverage_mutex);
+        if (trylock) {
+            /* Returns if cannot acquire lock. */
+            if (ovs_mutex_trylock(&coverage_mutex)) {
+                return;
+            }
+        } else {
+            ovs_mutex_lock(&coverage_mutex);
+        }
+
         for (i = 0; i < n_coverage_counters; i++) {
             struct coverage_counter *c = coverage_counters[i];
             c->total += c->count();
@@ -272,6 +285,18 @@ coverage_clear(void)
     }
 }
 
+void
+coverage_clear(void)
+{
+    coverage_clear__(false);
+}
+
+void
+coverage_try_clear(void)
+{
+    coverage_clear__(true);
+}
+
 /* Runs approximately every COVERAGE_RUN_INTERVAL amount of time to update the
  * coverage counters' 'min' and 'hr' array.  'min' array is for cumulating
  * per second counts into per minute count.  'hr' array is for cumulating per
diff --git a/lib/coverage.h b/lib/coverage.h
index 832c433..34a04aa 100644
--- a/lib/coverage.h
+++ b/lib/coverage.h
@@ -88,6 +88,7 @@ void coverage_counter_register(struct coverage_counter*);
 void coverage_init(void);
 void coverage_log(void);
 void coverage_clear(void);
+void coverage_try_clear(void);
 void coverage_run(void);
 
 #endif /* coverage.h */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c144352..f4ff8cb 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -42,6 +42,7 @@
 #include "fat-rwlock.h"
 #include "flow.h"
 #include "cmap.h"
+#include "coverage.h"
 #include "latch.h"
 #include "list.h"
 #include "match.h"
@@ -2696,6 +2697,7 @@ reload:
             lc = 0;
 
             emc_cache_slow_sweep(&pmd->flow_cache);
+            coverage_try_clear();
             ovsrcu_quiesce();
 
             atomic_read_relaxed(&pmd->change_seq, &seq);
-- 
1.7.9.5

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev