On Fri, Jul 12, 2013 at 2:54 PM, Ben Pfaff <b...@nicira.com> wrote:
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> vswitchd/system-stats.c | 105
> +++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 84 insertions(+), 21 deletions(-)
>
> diff --git a/vswitchd/system-stats.c b/vswitchd/system-stats.c
> index e7c1d73..ed63899 100644
> --- a/vswitchd/system-stats.c
> +++ b/vswitchd/system-stats.c
> @@ -35,7 +35,9 @@
> #include "dirs.h"
> #include "dynamic-string.h"
> #include "json.h"
> +#include "latch.h"
> #include "ofpbuf.h"
> +#include "ovs-thread.h"
> #include "poll-loop.h"
> #include "shash.h"
> #include "smap.h"
> @@ -504,17 +506,32 @@ get_filesys_stats(struct smap *stats OVS_UNUSED)
>
> #define SYSTEM_STATS_INTERVAL (5 * 1000) /* In milliseconds. */
>
> -/* The next time to wake up, or LLONG_MAX if stats are disabled. */
> -static long long int next_refresh = LLONG_MAX;
> +static pthread_mutex_t mutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER;
> +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
> +static struct latch latch;
> +static bool enabled;
> +static bool started;
> +static struct smap *system_stats;
> +
> +static void *system_stats_thread_func(void *);
> +static void discard_stats(void);
>
> /* Enables or disables system stats collection, according to 'enable'. */
> void
> system_stats_enable(bool enable)
> {
> - if (!enable) {
> - next_refresh = LLONG_MAX;
> - } else if (next_refresh == LLONG_MAX) {
> - next_refresh = time_msec();
> + if (enabled != enable) {
> + xpthread_mutex_lock(&mutex);
> + if (enable) {
> + if (!started) {
I do not see "started" getting changed anywhere else.
>
+ xpthread_create(NULL, NULL, system_stats_thread_func,
> NULL);
> + latch_init(&latch);
> + }
> + discard_stats();
> + xpthread_cond_signal(&cond);
> + }
> + enabled = enable;
> + xpthread_mutex_unlock(&mutex);
> }
> }
>
> @@ -529,23 +546,22 @@ system_stats_enable(bool enable)
> struct smap *
> system_stats_run(void)
> {
> - if (time_msec() >= next_refresh) {
> - struct smap *stats;
> + struct smap *stats = NULL;
>
> - stats = xmalloc(sizeof *stats);
> - smap_init(stats);
> - get_cpu_cores(stats);
> - get_load_average(stats);
> - get_memory_stats(stats);
> - get_process_stats(stats);
> - get_filesys_stats(stats);
> -
> - next_refresh = time_msec() + SYSTEM_STATS_INTERVAL;
> + xpthread_mutex_lock(&mutex);
> + if (system_stats) {
> + latch_poll(&latch);
>
> - return stats;
> + if (enabled) {
> + stats = system_stats;
> + system_stats = NULL;
> + } else {
> + discard_stats();
> + }
> }
> + xpthread_mutex_unlock(&mutex);
>
> - return NULL;
> + return stats;
> }
>
> /* Causes poll_block() to wake up when system_stats_run() needs to be
> @@ -553,7 +569,54 @@ system_stats_run(void)
> void
> system_stats_wait(void)
> {
> - if (next_refresh != LLONG_MAX) {
> - poll_timer_wait_until(next_refresh);
> + if (enabled) {
> + latch_wait(&latch);
> + }
> +}
> +
> +static void
> +discard_stats(void)
> +{
> + if (system_stats) {
> + smap_destroy(system_stats);
> + free(system_stats);
> + system_stats = NULL;
> + }
> +}
> +
> +static void * NO_RETURN
> +system_stats_thread_func(void *arg OVS_UNUSED)
> +{
> + pthread_detach(pthread_self());
> +
> + for (;;) {
> + long long int next_refresh;
> + struct smap *stats;
> +
> + xpthread_mutex_lock(&mutex);
> + while (!enabled) {
> + xpthread_cond_wait(&cond, &mutex);
> + }
> + xpthread_mutex_unlock(&mutex);
> +
> + stats = xmalloc(sizeof *stats);
> + smap_init(stats);
> + get_cpu_cores(stats);
> + get_load_average(stats);
> + get_memory_stats(stats);
> + get_process_stats(stats);
> + get_filesys_stats(stats);
> +
> + xpthread_mutex_lock(&mutex);
> + discard_stats();
> + system_stats = stats;
> + latch_set(&latch);
> + xpthread_mutex_unlock(&mutex);
> +
> + next_refresh = time_msec() + SYSTEM_STATS_INTERVAL;
> + do {
> + poll_timer_wait_until(next_refresh);
>
The poll_timer_wait_until function sets the value of "loop->timeout_when".
The way I see it, it is not protected by any locks and can be overwritten
by multiple threads at the same time.
> + poll_block();
> + } while (time_msec() < next_refresh);
> }
> }
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
