system-stats: Run as part of worker process.

The stats gathering can be time-consuming in some cases, so it's better to
do it in the worker process.

Signed-off-by: Ben Pfaff <blp@nicira.com>

vswitchd/bridge.c

@@ -148,10 +148,10 @@ static struct ovsdb_idl *idl;
 /* Most recently processed IDL sequence number. */
 static unsigned int idl_seqno;
 
-/* Each time this timer expires, the bridge fetches systems and interface
+/* Each time this timer expires, the bridge fetches interface and mirror
  * statistics and pushes them into the database. */
-#define STATS_INTERVAL (5 * 1000) /* In milliseconds. */
-static long long int stats_timer = LLONG_MIN;
+#define IFACE_STATS_INTERVAL (5 * 1000) /* In milliseconds. */
+static long long int iface_stats_timer = LLONG_MIN;
 
 /* Stores the time after which rate limited statistics may be written to the
  * database.  Only updated when changes to the database require rate limiting.
@@ -220,6 +220,9 @@ static void port_configure_bond(struct port *, struct bond_settings *,
                                 uint32_t *bond_stable_ids);
 static bool port_is_synthetic(const struct port *);
 
+static void reconfigure_system_stats(const struct ovsrec_open_vswitch *);
+static void run_system_stats(void);
+
 static void bridge_configure_mirrors(struct bridge *);
 static struct mirror *mirror_create(struct bridge *,
                                     const struct ovsrec_mirror *);
@@ -456,6 +459,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
             iface_clear_db_record(if_cfg->cfg);
         }
     }
+
+    reconfigure_system_stats(ovs_cfg);
 }
 
 static bool
@@ -1872,19 +1877,36 @@ enable_system_stats(const struct ovsrec_open_vswitch *cfg)
 }
 
 static void
-refresh_system_stats(const struct ovsrec_open_vswitch *cfg)
+reconfigure_system_stats(const struct ovsrec_open_vswitch *cfg)
 {
-    struct ovsdb_datum datum;
-    struct smap stats;
+    bool enable = enable_system_stats(cfg);
 
-    smap_init(&stats);
-    if (enable_system_stats(cfg)) {
-        get_system_stats(&stats);
+    system_stats_enable(enable);
+    if (!enable) {
+        ovsrec_open_vswitch_set_statistics(cfg, NULL);
     }
+}
 
-    ovsdb_datum_from_smap(&datum, &stats);
-    ovsdb_idl_txn_write(&cfg->header_, &ovsrec_open_vswitch_col_statistics,
-                        &datum);
+static void
+run_system_stats(void)
+{
+    const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(idl);
+    struct smap *stats;
+
+    stats = system_stats_run();
+    if (stats && cfg) {
+        struct ovsdb_idl_txn *txn;
+        struct ovsdb_datum datum;
+
+        txn = ovsdb_idl_txn_create(idl);
+        ovsdb_datum_from_smap(&datum, stats);
+        ovsdb_idl_txn_write(&cfg->header_, &ovsrec_open_vswitch_col_statistics,
+                            &datum);
+        ovsdb_idl_txn_commit(txn);
+        ovsdb_idl_txn_destroy(txn);
+
+        free(stats);
+    }
 }
 
 static inline const char *
@@ -2080,8 +2102,8 @@ bridge_run(void)
         reconf_txn = NULL;
     }
 
-    /* Refresh system and interface stats if necessary. */
-    if (time_msec() >= stats_timer) {
+    /* Refresh interface and mirror stats if necessary. */
+    if (time_msec() >= iface_stats_timer) {
         if (cfg) {
             struct ovsdb_idl_txn *txn;
 
@@ -2104,15 +2126,16 @@ bridge_run(void)
                 }
 
             }
-            refresh_system_stats(cfg);
             refresh_controller_status();
             ovsdb_idl_txn_commit(txn);
             ovsdb_idl_txn_destroy(txn); /* XXX */
         }
 
-        stats_timer = time_msec() + STATS_INTERVAL;
+        iface_stats_timer = time_msec() + IFACE_STATS_INTERVAL;
     }
 
+    run_system_stats();
+
     if (time_msec() >= db_limiter) {
         struct ovsdb_idl_txn *txn;
 
@@ -2177,12 +2200,14 @@ bridge_wait(void)
         HMAP_FOR_EACH (br, node, &all_bridges) {
             ofproto_wait(br->ofproto);
         }
-        poll_timer_wait_until(stats_timer);
+        poll_timer_wait_until(iface_stats_timer);
 
         if (db_limiter > time_msec()) {
             poll_timer_wait_until(db_limiter);
         }
     }
+
+    system_stats_wait();
 }
 
 /* Adds some memory usage statistics for bridges into 'usage', for use with

vswitchd/system-stats.c

@@ -35,10 +35,14 @@
 #include "daemon.h"
 #include "dirs.h"
 #include "dynamic-string.h"
+#include "json.h"
+#include "ofpbuf.h"
+#include "poll-loop.h"
 #include "shash.h"
 #include "smap.h"
 #include "timeval.h"
 #include "vlog.h"
+#include "worker.h"
 
 VLOG_DEFINE_THIS_MODULE(system_stats);
 
@@ -490,13 +494,155 @@ get_filesys_stats(struct smap *stats OVS_UNUSED)
     ds_destroy(&s);
 #endif  /* HAVE_SETMNTENT && HAVE_STATVFS */
 }
+
+#define SYSTEM_STATS_INTERVAL (5 * 1000) /* In milliseconds. */
 
+/* Whether the client wants us to report system stats. */
+static bool enabled;
+
+static enum {
+    S_DISABLED,                 /* Not enabled, nothing going on. */
+    S_WAITING,                  /* Sleeping for SYSTEM_STATS_INTERVAL ms. */
+    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
-get_system_stats(struct smap *stats)
+system_stats_enable(bool new_enable)
 {
-    get_cpu_cores(stats);
-    get_load_average(stats);
-    get_memory_stats(stats);
-    get_process_stats(stats);
-    get_filesys_stats(stats);
+    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
+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)
+{
+    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);
 }

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 */