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 <b...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev