[ovs-dev] [PATCH RFC] coverage: fix coverage accounting for pmd threads
Ilya Maximets
i.maximets at samsung.com
Mon Aug 10 14:45:18 UTC 2015
Currently coverage_counter_register() is called only from
constructor functions at program initialization time by
main thread. Coverage counter values are static and
thread local.
This means, that all COVERAGE_INC() calls from pmd threads
leads to increment of thread local variables of that threads
that can't be accessed by anyone else. And they are not used
in final coverage accounting.
Fix that by adding ability to add/remove coverage counters
in runtime for newly created threads and counting coverage
using counters from all threads.
Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
---
lib/coverage.c | 83 +++++++++++++++++++++++++++++++++++++++++++++----------
lib/coverage.h | 27 ++++++++++++------
lib/dpif-netdev.c | 4 ++-
3 files changed, 91 insertions(+), 23 deletions(-)
diff --git a/lib/coverage.c b/lib/coverage.c
index 6121956..2ae9e7a 100644
--- a/lib/coverage.c
+++ b/lib/coverage.c
@@ -30,14 +30,22 @@ VLOG_DEFINE_THIS_MODULE(coverage);
/* The coverage counters. */
static struct coverage_counter **coverage_counters = NULL;
-static size_t n_coverage_counters = 0;
static size_t allocated_coverage_counters = 0;
+static void (**coverage_initializers)(void) = NULL;
+static size_t n_coverage_initializers = 0;
+static size_t allocated_coverage_initializers = 0;
+
static struct ovs_mutex coverage_mutex = OVS_MUTEX_INITIALIZER;
DEFINE_STATIC_PER_THREAD_DATA(long long int, coverage_clear_time, LLONG_MIN);
static long long int coverage_run_time = LLONG_MIN;
+volatile unsigned int *coverage_val[2048] = {NULL};
+volatile size_t n_coverage_counters = 0;
+
+DEFINE_STATIC_PER_THREAD_DATA(unsigned int, coverage_thread_start, 0);
+
/* Index counter used to compute the moving average array's index. */
static unsigned int idx_count = 0;
@@ -54,7 +62,44 @@ coverage_counter_register(struct coverage_counter* counter)
&allocated_coverage_counters,
sizeof(struct coverage_counter*));
}
- coverage_counters[n_coverage_counters++] = counter;
+ coverage_counters[n_coverage_counters] = counter;
+ coverage_val[n_coverage_counters++] = NULL;
+}
+
+void
+coverage_initializer_register(void (*f)(void))
+{
+ if (n_coverage_initializers >= allocated_coverage_initializers) {
+ coverage_initializers = x2nrealloc(coverage_initializers,
+ &allocated_coverage_initializers,
+ sizeof *coverage_initializers);
+ }
+ coverage_initializers[n_coverage_initializers++] = f;
+}
+
+void
+coverage_register_new_thread(void)
+{
+ int i;
+ ovs_mutex_lock(&coverage_mutex);
+ *coverage_thread_start_get() = n_coverage_counters;
+ for (i = 0; i < n_coverage_initializers; i++)
+ coverage_initializers[i]();
+ ovs_mutex_unlock(&coverage_mutex);
+}
+
+void
+coverage_unregister_thread(void)
+{
+ int i;
+ ovs_mutex_lock(&coverage_mutex);
+ for (i = *coverage_thread_start_get() + n_coverage_initializers;
+ i < n_coverage_counters; i++) {
+ coverage_counters[i - n_coverage_initializers] = coverage_counters[i];
+ coverage_val[i - n_coverage_initializers] = coverage_val[i];
+ }
+ n_coverage_counters -= n_coverage_initializers;
+ ovs_mutex_unlock(&coverage_mutex);
}
static void
@@ -102,13 +147,12 @@ coverage_hash(void)
uint32_t hash = 0;
int n_groups, i;
+ ovs_mutex_lock(&coverage_mutex);
/* Sort coverage counters into groups with equal totals. */
c = xmalloc(n_coverage_counters * sizeof *c);
- ovs_mutex_lock(&coverage_mutex);
for (i = 0; i < n_coverage_counters; i++) {
c[i] = coverage_counters[i];
}
- ovs_mutex_unlock(&coverage_mutex);
qsort(c, n_coverage_counters, sizeof *c, compare_coverage_counters);
/* Hash the names in each group along with the rank. */
@@ -130,6 +174,7 @@ coverage_hash(void)
i = j;
}
+ ovs_mutex_unlock(&coverage_mutex);
free(c);
return hash_int(n_groups, hash);
@@ -200,9 +245,11 @@ coverage_read(struct svec *lines)
{
struct coverage_counter **c = coverage_counters;
unsigned long long int *totals;
+ unsigned long long int *total_sec, *total_min, *total_hr;
size_t n_never_hit;
uint32_t hash;
size_t i;
+ int initial_n = n_coverage_initializers;
hash = coverage_hash();
@@ -213,14 +260,20 @@ coverage_read(struct svec *lines)
"hash=%08"PRIx32":",
COVERAGE_RUN_INTERVAL/1000, hash));
- totals = xmalloc(n_coverage_counters * sizeof *totals);
ovs_mutex_lock(&coverage_mutex);
+ totals = xcalloc(n_coverage_counters, sizeof *totals);
+ total_sec = xcalloc(n_coverage_counters, sizeof *total_sec);
+ total_min = xcalloc(n_coverage_counters, sizeof *total_min);
+ total_hr = xcalloc(n_coverage_counters, sizeof *total_hr);
+
for (i = 0; i < n_coverage_counters; i++) {
- totals[i] = c[i]->total;
+ totals[i % initial_n] += c[i]->total;
+ total_sec[i % initial_n] += c[i]->min[(idx_count - 1) % MIN_AVG_LEN];
+ total_min[i % initial_n] += coverage_array_sum(c[i]->min, MIN_AVG_LEN);
+ total_hr[i % initial_n] += coverage_array_sum(c[i]->hr, HR_AVG_LEN);
}
- ovs_mutex_unlock(&coverage_mutex);
- for (i = 0; i < n_coverage_counters; i++) {
+ for (i = 0; i < initial_n; i++) {
if (totals[i]) {
/* Shows the averaged per-second rates for the last
* COVERAGE_RUN_INTERVAL interval, the last minute and
@@ -229,18 +282,22 @@ coverage_read(struct svec *lines)
xasprintf("%-24s %5.1f/sec %9.3f/sec "
"%13.4f/sec total: %llu",
c[i]->name,
- (c[i]->min[(idx_count - 1) % MIN_AVG_LEN]
+ (total_sec[i]
* 1000.0 / COVERAGE_RUN_INTERVAL),
- coverage_array_sum(c[i]->min, MIN_AVG_LEN) / 60.0,
- coverage_array_sum(c[i]->hr, HR_AVG_LEN) / 3600.0,
+ total_min[i] / 60.0,
+ total_hr[i] / 3600.0,
totals[i]));
} else {
n_never_hit++;
}
}
+ ovs_mutex_unlock(&coverage_mutex);
svec_add_nocopy(lines, xasprintf("%"PRIuSIZE" events never hit", n_never_hit));
free(totals);
+ free(total_sec);
+ free(total_min);
+ free(total_hr);
}
/* Runs approximately every COVERAGE_CLEAR_INTERVAL amount of time to
@@ -265,7 +322,7 @@ coverage_clear(void)
ovs_mutex_lock(&coverage_mutex);
for (i = 0; i < n_coverage_counters; i++) {
struct coverage_counter *c = coverage_counters[i];
- c->total += c->count();
+ c->total += c->count(i);
}
ovs_mutex_unlock(&coverage_mutex);
*thread_time = now + COVERAGE_CLEAR_INTERVAL;
@@ -340,10 +397,8 @@ coverage_array_sum(const unsigned int *arr, const unsigned int len)
unsigned int sum = 0;
size_t i;
- ovs_mutex_lock(&coverage_mutex);
for (i = 0; i < len; i++) {
sum += arr[i];
}
- ovs_mutex_unlock(&coverage_mutex);
return sum;
}
diff --git a/lib/coverage.h b/lib/coverage.h
index 832c433..1f5ae17 100644
--- a/lib/coverage.h
+++ b/lib/coverage.h
@@ -45,9 +45,9 @@ BUILD_ASSERT_DECL(COVERAGE_RUN_INTERVAL % COVERAGE_CLEAR_INTERVAL == 0);
/* A coverage counter. */
struct coverage_counter {
- const char *const name; /* Textual name. */
- unsigned int (*const count)(void); /* Gets, zeros this thread's count. */
- unsigned long long int total; /* Total count. */
+ const char *const name; /* Textual name. */
+ unsigned int (*const count)(unsigned int); /* Gets, zeros this thread's count. */
+ unsigned long long int total; /* Total count. */
unsigned long long int last_total;
/* The moving average arrays. */
unsigned int min[MIN_AVG_LEN];
@@ -55,15 +55,20 @@ struct coverage_counter {
};
void coverage_counter_register(struct coverage_counter*);
+void coverage_initializer_register(void (*f)(void));
+void coverage_register_new_thread(void);
+void coverage_unregister_thread(void);
+extern volatile unsigned int *coverage_val[2048];
+extern volatile size_t n_coverage_counters;
/* Defines COUNTER. There must be exactly one such definition at file scope
* within a program. */
#define COVERAGE_DEFINE(COUNTER) \
DEFINE_STATIC_PER_THREAD_DATA(unsigned int, \
counter_##COUNTER, 0); \
- static unsigned int COUNTER##_count(void) \
+ static unsigned int COUNTER##_count(unsigned int i) \
{ \
- unsigned int *countp = counter_##COUNTER##_get(); \
+ volatile unsigned int *countp = coverage_val[i]; \
unsigned int count = *countp; \
*countp = 0; \
return count; \
@@ -72,11 +77,17 @@ void coverage_counter_register(struct coverage_counter*);
{ \
*counter_##COUNTER##_get() += n; \
} \
- extern struct coverage_counter counter_##COUNTER; \
- struct coverage_counter counter_##COUNTER \
+ extern thread_local struct coverage_counter counter_##COUNTER; \
+ thread_local struct coverage_counter counter_##COUNTER \
= { #COUNTER, COUNTER##_count, 0, 0, {0}, {0} }; \
- OVS_CONSTRUCTOR(COUNTER##_init) { \
+ static void COUNTER##_initializer(void) { \
coverage_counter_register(&counter_##COUNTER); \
+ coverage_val[n_coverage_counters - 1] = \
+ counter_##COUNTER##_get(); \
+ } \
+ OVS_CONSTRUCTOR(COUNTER##_init) { \
+ coverage_initializer_register(&COUNTER##_initializer); \
+ COUNTER##_initializer(); \
}
/* Adds 1 to COUNTER. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c144352..a92dd6a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -34,6 +34,7 @@
#include "bitmap.h"
#include "cmap.h"
#include "csum.h"
+#include "coverage.h"
#include "dp-packet.h"
#include "dpif.h"
#include "dpif-provider.h"
@@ -2666,7 +2667,7 @@ pmd_thread_main(void *f_)
poll_cnt = 0;
poll_list = NULL;
-
+ coverage_register_new_thread();
/* Stores the pmd thread's 'pmd' to 'per_pmd_key'. */
ovsthread_setspecific(pmd->dp->per_pmd_key, pmd);
pmd_thread_setaffinity_cpu(pmd->core_id);
@@ -2717,6 +2718,7 @@ reload:
}
dp_netdev_pmd_reload_done(pmd);
+ coverage_unregister_thread();
free(poll_list);
return NULL;
--
2.1.4
More information about the dev
mailing list