[ovs-dev] [worker v2 14/14] system-stats: Run as part of worker process.

[Ethan Jackson]

> +    /* We can end up in these states is */

This sentence doesn't make a lot of sense to me.

Otherwise looks good, thanks.

Ethan


> +    S_REQUEST_SENT,             /* Sent a request to worker. */
> +    S_REPLY_RECEIVED            /* Received a reply from worker. */
> +} state;
> +
> +/* In S_WAITING state: the next time to wake up.
> + * In other states: not meaningful. */
> +static long long int next_refresh;
> +
> +/* In S_REPLY_RECEIVED: the stats that have just been received.
> + * In other states: not meaningful. */
> +static struct smap *received_stats;
> +
> +static worker_request_func system_stats_request_cb;
> +static worker_reply_func system_stats_reply_cb;
> +
> +/* Enables or disables system stats collection, according to 'new_enable'.
> + *
> + * Even if system stats are disabled, the caller should still periodically call
> + * system_stats_run(). */
> +void
> +system_stats_enable(bool new_enable)
> +{
> +    if (new_enable != enabled) {
> +        if (new_enable) {
> +            if (state == S_DISABLED) {
> +                state = S_WAITING;
> +                next_refresh = time_msec();
> +            }
> +        } else {
> +            if (state == S_WAITING) {
> +                state = S_DISABLED;
> +            }
> +        }
> +        enabled = new_enable;
> +    }
> +}
> +
> +/* Tries to obtain a new snapshot of system stats every SYSTEM_STATS_INTERVAL
> + * milliseconds.
> + *
> + * When a new snapshot is available (which only occurs if system stats are
> + * enabled), returns it as an smap owned by the caller.  The caller must use
> + * both smap_destroy() and free() to complete free the returned data.
> + *
> + * When no new snapshot is available, returns NULL. */
> +struct smap *
> +system_stats_run(void)
> +{
> +    switch (state) {
> +    case S_DISABLED:
> +        break;
> +
> +    case S_WAITING:
> +        if (time_msec() >= next_refresh) {
> +            worker_request(NULL, 0, NULL, 0, system_stats_request_cb,
> +                           system_stats_reply_cb, NULL);
> +            state = S_REQUEST_SENT;
> +        }
> +        break;
> +
> +    case S_REQUEST_SENT:
> +        break;
> +
> +    case S_REPLY_RECEIVED:
> +        if (enabled) {
> +            state = S_WAITING;
> +            next_refresh = time_msec() + SYSTEM_STATS_INTERVAL;
> +            return received_stats;
> +        } else {
> +            smap_destroy(received_stats);
> +            free(received_stats);
> +            state = S_DISABLED;
> +        }
> +        break;
> +    }
> +
> +    return NULL;
> +}
> +
> +/* Causes poll_block() to wake up when system_stats_run() needs to be
> + * called. */
>  void
> -get_system_stats(struct smap *stats)
> +system_stats_wait(void)
> +{
> +    switch (state) {
> +    case S_DISABLED:
> +        break;
> +
> +    case S_WAITING:
> +        poll_timer_wait_until(next_refresh);
> +        break;
> +
> +    case S_REQUEST_SENT:
> +        /* Someone else should be calling worker_wait() to wake up when the
> +         * reply arrives, otherwise there's a bug. */
> +        break;
> +
> +    case S_REPLY_RECEIVED:
> +        poll_immediate_wake();
> +        break;
> +    }
> +}
> +
> +static void
> +system_stats_request_cb(struct ofpbuf *request OVS_UNUSED,
> +                        const int fds[] OVS_UNUSED, size_t n_fds OVS_UNUSED)
>  {
> -    get_cpu_cores(stats);
> -    get_load_average(stats);
> -    get_memory_stats(stats);
> -    get_process_stats(stats);
> -    get_filesys_stats(stats);
> +    struct smap stats;
> +    struct json *json;
> +    char *s;
> +
> +    smap_init(&stats);
> +    get_cpu_cores(&stats);
> +    get_load_average(&stats);
> +    get_memory_stats(&stats);
> +    get_process_stats(&stats);
> +    get_filesys_stats(&stats);
> +
> +    json = smap_to_json(&stats);
> +    s = json_to_string(json, 0);
> +    worker_reply(s, strlen(s) + 1, NULL, 0);
> +
> +    free(s);
> +    json_destroy(json);
> +    smap_destroy(&stats);
> +}
> +
> +static void
> +system_stats_reply_cb(struct ofpbuf *reply,
> +                      const int fds[] OVS_UNUSED, size_t n_fds OVS_UNUSED,
> +                      void *aux OVS_UNUSED)
> +{
> +    struct json *json = json_from_string(reply->data);
> +
> +    received_stats = xmalloc(sizeof *received_stats);
> +    smap_init(received_stats);
> +    smap_from_json(received_stats, json);
> +
> +    assert(state == S_REQUEST_SENT);
> +    state = S_REPLY_RECEIVED;
> +
> +    json_destroy(json);
>  }
> diff --git a/vswitchd/system-stats.h b/vswitchd/system-stats.h
> index 9f965c6..83b4bcb 100644
> --- a/vswitchd/system-stats.h
> +++ b/vswitchd/system-stats.h
> @@ -16,8 +16,10 @@
>  #ifndef VSWITCHD_SYSTEM_STATS
>  #define VSWITCHD_SYSTEM_STATS 1
>
> -struct smap;
> +#include <stdbool.h>
>
> -void get_system_stats(struct smap *);
> +void system_stats_enable(bool enable);
> +struct smap *system_stats_run(void);
> +void system_stats_wait(void);
>
>  #endif /* vswitchd/system-stats.h */
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev