[ovs-dev] [threads 08/28] timeval: Remove backtrace feature.
Ben Pfaff
blp at nicira.com
Wed Jul 10 23:03:50 UTC 2013
The backtrace feature of timeval is useful because it provides a "poor
man's profile" view of Open vSwitch. But it is not likely to be useful in
a multithreaded process, because signal delivery doesn't necessarily follow
the profile when there is more than one thread. (A signal in a
multithreaded process are delivered to an arbitrary thread.)
Another problem with the backtrace feature is that it is difficult for
format_backtraces() to synchronize properly with the signal handler in a
multithreaded process. In a single-threaded process, it can just block
the signal handler, but in a multithreaded process this does not prevent
signal delivery to threads other than the one running format_backtrace().
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
lib/poll-loop.c | 6 --
lib/timeval.c | 174 -------------------------------------------------------
lib/timeval.h | 1 -
3 files changed, 0 insertions(+), 181 deletions(-)
diff --git a/lib/poll-loop.c b/lib/poll-loop.c
index ea00d26..5f4b16c 100644
--- a/lib/poll-loop.c
+++ b/lib/poll-loop.c
@@ -156,7 +156,6 @@ poll_immediate_wake(const char *where)
static void
log_wakeup(const char *where, const struct pollfd *pollfd, int timeout)
{
- static struct vlog_rate_limit trace_rl = VLOG_RATE_LIMIT_INIT(1, 1);
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
enum vlog_level level;
int cpu_usage;
@@ -200,11 +199,6 @@ log_wakeup(const char *where, const struct pollfd *pollfd, int timeout)
}
if (cpu_usage >= 0) {
ds_put_format(&s, " (%d%% CPU usage)", cpu_usage);
-
- if (!vlog_should_drop(THIS_MODULE, level, &trace_rl)) {
- ds_put_char(&s, '\n');
- format_backtraces(&s, 2);
- }
}
VLOG(level, "%s", ds_cstr(&s));
ds_destroy(&s);
diff --git a/lib/timeval.c b/lib/timeval.c
index b589cb9..a86af4f 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -37,15 +37,6 @@
#include "util.h"
#include "vlog.h"
-/* backtrace() from <execinfo.h> is really useful, but it is not signal safe
- * everywhere, such as on x86-64. */
-#if HAVE_BACKTRACE && !defined __x86_64__
-# define USE_BACKTRACE 1
-# include <execinfo.h>
-#else
-# define USE_BACKTRACE 0
-#endif
-
VLOG_DEFINE_THIS_MODULE(timeval);
struct clock {
@@ -72,44 +63,14 @@ static long long int boot_time;
* LLONG_MAX). */
static long long int deadline = LLONG_MAX;
-struct trace {
- void *backtrace[32]; /* Populated by backtrace(). */
- size_t n_frames; /* Number of frames in 'backtrace'. */
-
- /* format_backtraces() helper data. */
- struct hmap_node node;
- size_t count;
-};
-
-#define MAX_TRACES 50
-static struct trace traces[MAX_TRACES];
-static size_t trace_head = 0;
-
static void set_up_timer(void);
static void set_up_signal(int flags);
static void sigalrm_handler(int);
-static void block_sigalrm(sigset_t *);
-static void unblock_sigalrm(const sigset_t *);
static void log_poll_interval(long long int last_wakeup);
static struct rusage *get_recent_rusage(void);
static void refresh_rusage(void);
static void timespec_add(struct timespec *sum,
const struct timespec *a, const struct timespec *b);
-static unixctl_cb_func backtrace_cb;
-
-#if !USE_BACKTRACE
-static int
-backtrace(void **buffer OVS_UNUSED, int size OVS_UNUSED)
-{
- NOT_REACHED();
-}
-
-static char **
-backtrace_symbols(void *const *buffer OVS_UNUSED, int size OVS_UNUSED)
-{
- NOT_REACHED();
-}
-#endif /* !USE_BACKTRACE */
static void
init_clock(struct clock *c, clockid_t id)
@@ -125,22 +86,6 @@ do_init_time(void)
{
struct timespec ts;
- /* The implementation of backtrace() in glibc does some one time
- * initialization which is not signal safe. This can cause deadlocks if
- * run from the signal handler. As a workaround, force the initialization
- * to happen here. */
- if (USE_BACKTRACE) {
- void *bt[1];
-
- backtrace(bt, ARRAY_SIZE(bt));
- }
-
- memset(traces, 0, sizeof traces);
-
- if (USE_BACKTRACE && CACHE_TIME) {
- unixctl_command_register("backtrace", "", 0, 0, backtrace_cb, NULL);
- }
-
coverage_init();
init_clock(&monotonic_clock, (!clock_gettime(CLOCK_MONOTONIC, &ts)
@@ -415,29 +360,6 @@ static void
sigalrm_handler(int sig_nr OVS_UNUSED)
{
monotonic_clock.tick = wall_clock.tick = true;
-
- if (USE_BACKTRACE && CACHE_TIME) {
- struct trace *trace = &traces[trace_head];
-
- trace->n_frames = backtrace(trace->backtrace,
- ARRAY_SIZE(trace->backtrace));
- trace_head = (trace_head + 1) % MAX_TRACES;
- }
-}
-
-static void
-block_sigalrm(sigset_t *oldsigs)
-{
- sigset_t sigalrm;
- sigemptyset(&sigalrm);
- sigaddset(&sigalrm, SIGALRM);
- xpthread_sigmask(SIG_BLOCK, &sigalrm, oldsigs);
-}
-
-static void
-unblock_sigalrm(const sigset_t *oldsigs)
-{
- xpthread_sigmask(SIG_SETMASK, oldsigs, NULL);
}
long long int
@@ -594,89 +516,6 @@ get_cpu_usage(void)
{
return cpu_usage;
}
-
-static uint32_t
-hash_trace(struct trace *trace)
-{
- return hash_bytes(trace->backtrace,
- trace->n_frames * sizeof *trace->backtrace, 0);
-}
-
-static struct trace *
-trace_map_lookup(struct hmap *trace_map, struct trace *key)
-{
- struct trace *value;
-
- HMAP_FOR_EACH_WITH_HASH (value, node, hash_trace(key), trace_map) {
- if (key->n_frames == value->n_frames
- && !memcmp(key->backtrace, value->backtrace,
- key->n_frames * sizeof *key->backtrace)) {
- return value;
- }
- }
- return NULL;
-}
-
-/* Appends a string to 'ds' representing backtraces recorded at regular
- * intervals in the recent past. This information can be used to get a sense
- * of what the process has been spending the majority of time doing. Will
- * ommit any backtraces which have not occurred at least 'min_count' times. */
-void
-format_backtraces(struct ds *ds, size_t min_count)
-{
- time_init();
-
- if (USE_BACKTRACE && CACHE_TIME) {
- struct hmap trace_map = HMAP_INITIALIZER(&trace_map);
- struct trace *trace, *next;
- sigset_t oldsigs;
- size_t i;
-
- block_sigalrm(&oldsigs);
-
- for (i = 0; i < MAX_TRACES; i++) {
- struct trace *trace = &traces[i];
- struct trace *map_trace;
-
- if (!trace->n_frames) {
- continue;
- }
-
- map_trace = trace_map_lookup(&trace_map, trace);
- if (map_trace) {
- map_trace->count++;
- } else {
- hmap_insert(&trace_map, &trace->node, hash_trace(trace));
- trace->count = 1;
- }
- }
-
- HMAP_FOR_EACH_SAFE (trace, next, node, &trace_map) {
- char **frame_strs;
- size_t j;
-
- hmap_remove(&trace_map, &trace->node);
-
- if (trace->count < min_count) {
- continue;
- }
-
- frame_strs = backtrace_symbols(trace->backtrace, trace->n_frames);
-
- ds_put_format(ds, "Count %zu\n", trace->count);
- for (j = 0; j < trace->n_frames; j++) {
- ds_put_format(ds, "%s\n", frame_strs[j]);
- }
- ds_put_cstr(ds, "\n");
-
- free(frame_strs);
- }
- hmap_destroy(&trace_map);
-
- ds_chomp(ds, '\n');
- unblock_sigalrm(&oldsigs);
- }
-}
/* Unixctl interface. */
@@ -722,19 +561,6 @@ timeval_warp_cb(struct unixctl_conn *conn,
unixctl_command_reply(conn, "warped");
}
-static void
-backtrace_cb(struct unixctl_conn *conn,
- int argc OVS_UNUSED, const char *argv[] OVS_UNUSED,
- void *aux OVS_UNUSED)
-{
- struct ds ds = DS_EMPTY_INITIALIZER;
-
- ovs_assert(USE_BACKTRACE && CACHE_TIME);
- format_backtraces(&ds, 0);
- unixctl_command_reply(conn, ds_cstr(&ds));
- ds_destroy(&ds);
-}
-
void
timeval_dummy_register(void)
{
diff --git a/lib/timeval.h b/lib/timeval.h
index 7bf8d1f..8dd2e2b 100644
--- a/lib/timeval.h
+++ b/lib/timeval.h
@@ -76,7 +76,6 @@ void xgettimeofday(struct timeval *);
void xclock_gettime(clock_t, struct timespec *);
int get_cpu_usage(void);
-void format_backtraces(struct ds *, size_t min_count);
long long int time_boot_msec(void);
--
1.7.2.5
More information about the dev
mailing list