neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email )
Change subject: add osmo_stats_report_lock api
......................................................................
add osmo_stats_report_lock api
Allow multi-threaded access to reported stats:
- enable use of a stats mutex with osmo_stats_report_use_lock(true).
- lock/unlock externally with osmo_stats_report_lock(true/false).
Rationale:
In osmo-hnbgw, we would like to collect stats from nftables, and do so
in a separate thead. The most efficient way is to write the parsing
results from nft directly to the rate_ctr destination.
But what if the main thread reports rate counters at exactly that time?
- is writing to stats atomic on a data type level?
- do applications need stats to be "atomic" as a whole?
In osmo-hnbgw in particular, there are two counters, 'packets' and
'total_bytes'. These correspond, and it would skew statistics if we
reported them out of sync to each other.
The simplest way to ensure correctness in all cases is a mutex around
the stats reporting.
But this mutex isn't needed in most of our programs. To completely avoid
any overhead the mutex may bring, make use of it optional with a global
flag.
This use case is likely to also show up in other programs that would
like to collect and report stats from a separate thread.
Related: SYS#6773
Related: osmo-hnbgw I9dc54e6bc94c553f45adfa71ae8ad70be4afbc8f
Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d
---
M include/osmocom/core/stats.h
M src/core/libosmocore.map
M src/core/stats.c
M src/vty/stats_vty.c
4 files changed, 96 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/38/36538/1
diff --git a/include/osmocom/core/stats.h b/include/osmocom/core/stats.h
index a034a61..4ec448c 100644
--- a/include/osmocom/core/stats.h
+++ b/include/osmocom/core/stats.h
@@ -109,6 +109,8 @@
void osmo_stats_init(void *ctx);
int osmo_stats_report(void);
+void osmo_stats_report_use_lock(bool enable);
+void osmo_stats_report_lock(bool lock);
int osmo_stats_set_interval(int interval);
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index c5ab6e3..a3c1db6 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -488,6 +488,8 @@
osmo_stats_config;
osmo_stats_init;
osmo_stats_report;
+osmo_stats_report_use_lock;
+osmo_stats_report_lock;
osmo_stats_reporter_alloc;
osmo_stats_reporter_create_log;
osmo_stats_reporter_create_statsd;
diff --git a/src/core/stats.c b/src/core/stats.c
index 16e6f62..21c7ddd 100644
--- a/src/core/stats.c
+++ b/src/core/stats.c
@@ -71,6 +71,7 @@
#include <stdio.h>
#include <sys/types.h>
#include <inttypes.h>
+#include <pthread.h>
#ifdef HAVE_SYS_SOCKET_H
#include <sys/socket.h>
@@ -787,13 +788,59 @@
}
}
+static pthread_mutex_t *g_report_lock = NULL;
+
+/*! Enable osmo_stats_report() mutex locking with osmo_stats_report_lock().
+ * Calling osmo_stats_report_use_lock(true) */
+void osmo_stats_report_use_lock(bool enable)
+{
+ static pthread_mutex_t report_lock;
+ if (enable) {
+ /* enable locking */
+ if (!g_report_lock) {
+ pthread_mutex_init(&report_lock, NULL);
+ g_report_lock = &report_lock;
+ }
+ return;
+ }
+ /* disable locking */
+ if (g_report_lock) {
+ pthread_mutex_destroy(g_report_lock);
+ g_report_lock = NULL;
+ }
+}
+
+/*! After osmo_stats_report_use_lock(true), lock a mutex to avoid
osmo_stats_report() reading from stats.
+ * The caller must ensure to call osmo_stats_report_lock(false) as soon as
possible, or the application will lock up.
+ * This function has no effect with osmo_stats_report_use_lock(false).
+ * Useful in multi-threaded applications to allow writing to
stats/counters/rate_ctrs directly from various threads.
+ * Particularly useful to ensure that a given set of stats/counters updates
from another thread are reported in sync.
+ */
+void osmo_stats_report_lock(bool lock)
+{
+ if (!g_report_lock)
+ return;
+ if (lock)
+ pthread_mutex_lock(g_report_lock);
+ else
+ pthread_mutex_unlock(g_report_lock);
+}
+
int osmo_stats_report(void)
{
+ pthread_mutex_t *lock = g_report_lock;
+
/* per group actions */
TRACE(LIBOSMOCORE_STATS_START());
+ if (lock)
+ pthread_mutex_lock(lock);
+ /* { */
osmo_counters_for_each(handle_counter, NULL);
rate_ctr_for_each_group(rate_ctr_group_handler, NULL);
osmo_stat_item_for_each_group(osmo_stat_item_group_handler, NULL);
+ /* } */
+ if (lock)
+ pthread_mutex_unlock(lock);
/* global actions */
flush_all_reporters();
diff --git a/src/vty/stats_vty.c b/src/vty/stats_vty.c
index f940018..4a62dc4 100644
--- a/src/vty/stats_vty.c
+++ b/src/vty/stats_vty.c
@@ -426,7 +426,9 @@
if (argc > 0)
skip_zero = true;
+ osmo_stats_report_lock(true);
vty_out_statistics_full2(vty, "", skip_zero);
+ osmo_stats_report_lock(false);
return CMD_SUCCESS;
}
@@ -444,7 +446,9 @@
bool skip_zero = false;
if (argc > 1)
skip_zero = true;
+ osmo_stats_report_lock(true);
vty_out_statistics_partial2(vty, "", level, skip_zero);
+ osmo_stats_report_lock(false);
return CMD_SUCCESS;
}
@@ -632,7 +636,9 @@
struct rctr_vty_ctx rctx = { .vty = vty, .skip_zero = false };
if (argc > 0)
rctx.skip_zero = true;
+ osmo_stats_report_lock(true);
rate_ctr_for_each_group(rate_ctr_group_handler, &rctx);
+ osmo_stats_report_lock(false);
return CMD_SUCCESS;
}
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36538?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ib335bea7d2a440ca284e6c439066f96456bf2c2d
Gerrit-Change-Number: 36538
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-MessageType: newchange
