From: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp> It is difficult to know which function needs a notrace directive. We are using notrace to avoid starting tracer recursively while tracing, but we can do the similar thing by using a thread local variable. With this patch, notrace is necessary only to the entry point (trace_function_enter) and the exit point (trace_function_exit). This patch removes notrace compiler.h so that we don't abuse the directive any more. This patch also allows us to use tracer with debug build. Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp> --- include/compiler.h | 1 - include/list.h | 1 - lib/logger.c | 40 ++++++++++++++++++++-------------------- lib/net.c | 4 ++-- lib/strbuf.c | 4 ++-- lib/util.c | 4 ++-- sheep/trace/graph.c | 6 +++--- sheep/trace/trace.c | 50 +++++++++++++++++++++++++++++++++----------------- 8 files changed, 62 insertions(+), 48 deletions(-) diff --git a/include/compiler.h b/include/compiler.h index b2ae002..df38dc8 100644 --- a/include/compiler.h +++ b/include/compiler.h @@ -12,7 +12,6 @@ #ifndef SD_COMPILER_H #define SD_COMPILER_H -#define notrace __attribute__((no_instrument_function)) #define __packed __attribute((packed)) #define __printf(a, b) __attribute__((format(printf, a, b))) diff --git a/include/list.h b/include/list.h index c8ee425..44db5a6 100644 --- a/include/list.h +++ b/include/list.h @@ -164,7 +164,6 @@ static inline int hlist_unhashed(const struct hlist_node *h) return !h->pprev; } -__attribute__((always_inline)) static inline int hlist_empty(const struct hlist_head *h) { return !h->first; diff --git a/lib/logger.c b/lib/logger.c index 2bae1bd..a3ea11e 100644 --- a/lib/logger.c +++ b/lib/logger.c @@ -135,7 +135,7 @@ init_log_formatter(void) exit(1); } -static notrace int logarea_init(int size) +static int logarea_init(int size) { int shmid; @@ -196,7 +196,7 @@ static notrace int logarea_init(int size) return 0; } -static void notrace free_logarea(void) +static void free_logarea(void) { if (log_fd >= 0) close(log_fd); @@ -205,7 +205,7 @@ static void notrace free_logarea(void) shmdt(la); } -static notrace int server_log_formatter(char *buff, size_t size, +static int server_log_formatter(char *buff, size_t size, const struct logmsg *msg) { char *p = buff; @@ -251,7 +251,7 @@ static notrace int server_log_formatter(char *buff, size_t size, } log_format_register("server", server_log_formatter); -static notrace int default_log_formatter(char *buff, size_t size, +static int default_log_formatter(char *buff, size_t size, const struct logmsg *msg) { int len; @@ -265,7 +265,7 @@ static notrace int default_log_formatter(char *buff, size_t size, } log_format_register("default", default_log_formatter); -static notrace int json_log_formatter(char *buff, size_t size, +static int json_log_formatter(char *buff, size_t size, const struct logmsg *msg) { int i, body_len; @@ -332,7 +332,7 @@ static notrace int json_log_formatter(char *buff, size_t size, log_format_register("json", json_log_formatter); /* this one can block under memory pressure */ -static notrace void log_syslog(const struct logmsg *msg) +static void log_syslog(const struct logmsg *msg) { char str[MAX_MSG_SIZE]; @@ -344,7 +344,7 @@ static notrace void log_syslog(const struct logmsg *msg) syslog(msg->prio, "%s", str); } -static notrace void init_logmsg(struct logmsg *msg, struct timeval *tv, +static void init_logmsg(struct logmsg *msg, struct timeval *tv, int prio, const char *func, int line) { msg->tv = *tv; @@ -356,7 +356,7 @@ static notrace void init_logmsg(struct logmsg *msg, struct timeval *tv, msg->worker_idx = worker_idx; } -static notrace void dolog(int prio, const char *func, int line, +static void dolog(int prio, const char *func, int line, const char *fmt, va_list ap) { char buf[sizeof(struct logmsg) + MAX_MSG_SIZE]; @@ -412,7 +412,7 @@ static notrace void dolog(int prio, const char *func, int line, } } -static notrace void rotate_log(void) +static void rotate_log(void) { int new_fd; @@ -441,7 +441,7 @@ static notrace void rotate_log(void) close(new_fd); } -notrace void log_write(int prio, const char *func, int line, const char *fmt, ...) +void log_write(int prio, const char *func, int line, const char *fmt, ...) { va_list ap; @@ -453,7 +453,7 @@ notrace void log_write(int prio, const char *func, int line, const char *fmt, .. va_end(ap); } -static notrace void log_flush(void) +static void log_flush(void) { struct sembuf ops; size_t size, done = 0; @@ -493,7 +493,7 @@ static bool is_sheep_dead(int signo) return signo == SIGHUP; } -static notrace void crash_handler(int signo) +static void crash_handler(int signo) { if (is_sheep_dead(signo)) sd_printf(SDOG_ERR, "sheep pid %d exited unexpectedly.", @@ -515,7 +515,7 @@ static notrace void crash_handler(int signo) reraise_crash_signal(signo, 1); } -static notrace void logger(char *log_dir, char *outfile) +static void logger(char *log_dir, char *outfile) { int fd; @@ -605,7 +605,7 @@ void early_log_init(const char *format_name, struct logger_user_info *user_info) exit(1); } -notrace int log_init(const char *program_name, bool to_stdout, int level, +int log_init(const char *program_name, bool to_stdout, int level, char *outfile) { char log_dir[PATH_MAX], tmp[PATH_MAX]; @@ -651,7 +651,7 @@ notrace int log_init(const char *program_name, bool to_stdout, int level, return 0; } -notrace void log_close(void) +void log_close(void) { if (la) { la->active = false; @@ -663,14 +663,14 @@ notrace void log_close(void) } } -notrace void set_thread_name(const char *name, bool show_idx) +void set_thread_name(const char *name, bool show_idx) { worker_name = name; if (show_idx) worker_idx = gettid(); } -notrace void get_thread_name(char *name) +void get_thread_name(char *name) { if (worker_name && worker_idx) snprintf(name, MAX_THREAD_NAME_LEN, "%s %d", @@ -713,7 +713,7 @@ static bool check_gdb(void) #define FRAME_POINTER ((unsigned long *)__builtin_frame_address(0) + 2) __attribute__ ((__noinline__)) -notrace int __sd_dump_variable(const char *var) +int __sd_dump_variable(const char *var) { char cmd[ARG_MAX], path[PATH_MAX], info[256]; FILE *f = NULL; @@ -759,7 +759,7 @@ notrace int __sd_dump_variable(const char *var) } __attribute__ ((__noinline__)) -static notrace int dump_stack_frames(void) +static int dump_stack_frames(void) { char path[PATH_MAX]; int i, stack_no = 0; @@ -826,7 +826,7 @@ static notrace int dump_stack_frames(void) } __attribute__ ((__noinline__)) -notrace void sd_backtrace(void) +void sd_backtrace(void) { void *addrs[SD_MAX_STACK_DEPTH]; int i, n = backtrace(addrs, ARRAY_SIZE(addrs)); diff --git a/lib/net.c b/lib/net.c index 30d0119..c673ad8 100644 --- a/lib/net.c +++ b/lib/net.c @@ -61,7 +61,7 @@ int conn_rx_on(struct connection *conn) return modify_event(conn->fd, conn->events); } -notrace bool is_conn_dead(const struct connection *conn) +bool is_conn_dead(const struct connection *conn) { if (conn->c_rx_state == C_IO_CLOSED || conn->c_tx_state == C_IO_CLOSED) return true; @@ -94,7 +94,7 @@ int rx(struct connection *conn, enum conn_state next_state) return ret; } -notrace int tx(struct connection *conn, enum conn_state next_state) +int tx(struct connection *conn, enum conn_state next_state) { int ret; diff --git a/lib/strbuf.c b/lib/strbuf.c index 28ed38c..4a41d4d 100644 --- a/lib/strbuf.c +++ b/lib/strbuf.c @@ -50,7 +50,7 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc) sb->buf[sb->len] = '\0'; } -notrace void strbuf_grow(struct strbuf *sb, size_t extra) +void strbuf_grow(struct strbuf *sb, size_t extra) { if (sb->len + extra + 1 <= sb->len) panic("you want to use way too much memory"); @@ -98,7 +98,7 @@ void strbuf_remove(struct strbuf *sb, size_t pos, size_t len) strbuf_splice(sb, pos, len, NULL, 0); } -notrace void strbuf_add(struct strbuf *sb, const void *data, size_t len) +void strbuf_add(struct strbuf *sb, const void *data, size_t len) { strbuf_grow(sb, len); memcpy(sb->buf + sb->len, data, len); diff --git a/lib/util.c b/lib/util.c index ff0eafb..560999c 100644 --- a/lib/util.c +++ b/lib/util.c @@ -64,7 +64,7 @@ void *xzalloc(size_t size) return xcalloc(1, size); } -notrace void *xrealloc(void *ptr, size_t size) +void *xrealloc(void *ptr, size_t size) { void *ret = realloc(ptr, size); if (!ret && !size) @@ -316,7 +316,7 @@ void eventfd_xwrite(int efd, int value) * @param buf_size size of destination buffer * @param str source string */ -void notrace pstrcpy(char *buf, int buf_size, const char *str) +void pstrcpy(char *buf, int buf_size, const char *str) { int c; char *q = buf; diff --git a/sheep/trace/graph.c b/sheep/trace/graph.c index 2dafd22..f3f0858 100644 --- a/sheep/trace/graph.c +++ b/sheep/trace/graph.c @@ -15,7 +15,7 @@ static __thread unsigned long long entry_time[SD_MAX_STACK_DEPTH]; -static notrace uint64_t clock_get_time(void) +static uint64_t clock_get_time(void) { struct timespec ts; @@ -23,7 +23,7 @@ static notrace uint64_t clock_get_time(void) return (uint64_t)ts.tv_sec * 1000000000LL + (uint64_t)ts.tv_nsec; } -static notrace void graph_tracer_exit(const struct caller *this_fn, int depth) +static void graph_tracer_exit(const struct caller *this_fn, int depth) { struct trace_graph_item trace = { .depth = depth, @@ -37,7 +37,7 @@ static notrace void graph_tracer_exit(const struct caller *this_fn, int depth) trace_buffer_push(sched_getcpu(), &trace); } -static notrace void graph_tracer_enter(const struct caller *this_fn, int depth) +static void graph_tracer_enter(const struct caller *this_fn, int depth) { struct trace_graph_item trace = { .type = TRACE_GRAPH_ENTRY, diff --git a/sheep/trace/trace.c b/sheep/trace/trace.c index 9f9420f..67fa95e 100644 --- a/sheep/trace/trace.c +++ b/sheep/trace/trace.c @@ -32,6 +32,8 @@ static struct strbuf *buffer; static pthread_mutex_t *buffer_lock; static int nr_cpu; +static __thread bool in_trace; + union instruction { unsigned char start[INSN_SIZE]; struct { @@ -40,12 +42,12 @@ union instruction { } __attribute__((packed)); }; -static notrace int caller_cmp(const struct caller *a, const struct caller *b) +static int caller_cmp(const struct caller *a, const struct caller *b) { return intcmp(a->mcount, b->mcount); } -static notrace unsigned char *get_new_call(unsigned long ip, unsigned long addr) +static unsigned char *get_new_call(unsigned long ip, unsigned long addr) { static union instruction code; @@ -55,7 +57,7 @@ static notrace unsigned char *get_new_call(unsigned long ip, unsigned long addr) return code.start; } -static notrace void replace_call(unsigned long ip, unsigned long func) +static void replace_call(unsigned long ip, unsigned long func) { unsigned char *new; @@ -63,14 +65,14 @@ static notrace void replace_call(unsigned long ip, unsigned long func) memcpy((void *)ip, new, INSN_SIZE); } -static notrace int make_text_writable(unsigned long ip) +static int make_text_writable(unsigned long ip) { unsigned long start = ip & ~(getpagesize() - 1); return mprotect((void *)start, getpagesize() + INSN_SIZE, PROT_READ | PROT_EXEC | PROT_WRITE); } -static notrace struct caller *trace_lookup_ip(unsigned long ip) +static struct caller *trace_lookup_ip(unsigned long ip) { const struct caller key = { .mcount = ip, @@ -79,29 +81,35 @@ static notrace struct caller *trace_lookup_ip(unsigned long ip) return xbsearch(&key, callers, nr_callers, caller_cmp); } -notrace void regist_tracer(struct tracer *tracer) +void regist_tracer(struct tracer *tracer) { list_add_tail(&tracer->list, &tracers); } -static notrace void patch_all_sites(unsigned long addr) +static void patch_all_sites(unsigned long addr) { for (int i = 0; i < nr_callers; i++) replace_call(callers[i].mcount, addr); } -static notrace void nop_all_sites(void) +static void nop_all_sites(void) { for (int i = 0; i < nr_callers; i++) memcpy((void *)callers[i].mcount, NOP5, INSN_SIZE); } /* the entry point of the function */ -notrace void trace_function_enter(unsigned long ip, unsigned long *ret_addr) +__attribute__((no_instrument_function)) +void trace_function_enter(unsigned long ip, unsigned long *ret_addr) { struct tracer *tracer; const struct caller *caller; + if (in_trace) + /* don't trace while tracing */ + return; + in_trace = true; + assert(ret_stack_index < ARRAY_SIZE(trace_ret_stack)); caller = trace_lookup_ip(ip); @@ -115,13 +123,19 @@ notrace void trace_function_enter(unsigned long ip, unsigned long *ret_addr) trace_ret_stack[ret_stack_index].ret = *ret_addr; ret_stack_index++; *ret_addr = (unsigned long)trace_return_caller; + + in_trace = false; } /* the exit point of the function */ -notrace unsigned long trace_function_exit(void) +__attribute__((no_instrument_function)) +unsigned long trace_function_exit(void) { struct tracer *tracer; + assert(!in_trace); + in_trace = true; + ret_stack_index--; list_for_each_entry(tracer, &tracers, list) { @@ -130,10 +144,12 @@ notrace unsigned long trace_function_exit(void) ret_stack_index); } + in_trace = false; + return trace_ret_stack[ret_stack_index].ret; } -static notrace size_t count_enabled_tracers(void) +static size_t count_enabled_tracers(void) { size_t nr = 0; struct tracer *t; @@ -146,7 +162,7 @@ static notrace size_t count_enabled_tracers(void) return nr; } -static notrace struct tracer *find_tracer(const char *name) +static struct tracer *find_tracer(const char *name) { struct tracer *t; @@ -158,7 +174,7 @@ static notrace struct tracer *find_tracer(const char *name) return NULL; } -notrace int trace_enable(const char *name) +int trace_enable(const char *name) { struct tracer *tracer = find_tracer(name); @@ -182,7 +198,7 @@ notrace int trace_enable(const char *name) return SD_RES_SUCCESS; } -notrace int trace_disable(const char *name) +int trace_disable(const char *name) { struct tracer *tracer = find_tracer(name); @@ -205,7 +221,7 @@ notrace int trace_disable(const char *name) return SD_RES_SUCCESS; } -notrace int trace_buffer_pop(void *buf, uint32_t len) +int trace_buffer_pop(void *buf, uint32_t len) { int readin, count = 0, requested = len; char *buff = (char *)buf; @@ -228,7 +244,7 @@ notrace int trace_buffer_pop(void *buf, uint32_t len) return count; } -notrace void trace_buffer_push(int cpuid, struct trace_graph_item *item) +void trace_buffer_push(int cpuid, struct trace_graph_item *item) { pthread_mutex_lock(&buffer_lock[cpuid]); strbuf_add(&buffer[cpuid], item, sizeof(*item)); @@ -347,7 +363,7 @@ static int init_callers(void) * Try to NOP all the mcount call sites that are supposed to be traced. Later * we can enable it by asking these sites to point to trace_caller. */ -notrace int trace_init(void) +int trace_init(void) { int i; -- 1.7.9.5 |