From: Ben Pfaff Date: Wed, 18 Jul 2012 17:59:52 +0000 (-0700) Subject: system-stats: Run as part of worker process. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=35a22d8c10ae5ab8d0720864f552a21168b08375;p=openvswitch 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 --- diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 41ad5928..a36705ba 100644 --- a/vswitchd/bridge.c +++ b/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); } +} + +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); - ovsdb_datum_from_smap(&datum, &stats); - ovsdb_idl_txn_write(&cfg->header_, &ovsrec_open_vswitch_col_statistics, - &datum); + 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 diff --git a/vswitchd/system-stats.c b/vswitchd/system-stats.c index 4dc2723c..e0937a39 100644 --- a/vswitchd/system-stats.c +++ b/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) +{ + 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) { - get_cpu_cores(stats); - get_load_average(stats); - get_memory_stats(stats); - get_process_stats(stats); - get_filesys_stats(stats); + 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); } diff --git a/vswitchd/system-stats.h b/vswitchd/system-stats.h index 9f965c63..83b4bcbc 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 -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 */