libtracecmd is a library, containing functions that can be used without the trace-cmd application. However, some of the functions declared as libtracecmd APIs in trace-cmd.h depend on trace-cmd context. That causes a problem when other application uses the library. The problem can be observed when running kerneshark and there is a python module, loaded by the python plugin - there is a bunch of warnings. To resolve the problem, implementations of all trace-cmd independent functions are moved into libtracecmd. All libtracecmd functions, that depend on trace-cmd context are removed from the library and from trace-cmd.h file. [ v2 changes: - Added a new patch: Move trace-cmd-local.h from the application to the library. - Remove trace-output.c dependency of version.h. Moved FILE_VERSION_STRING define from top Makefile to trace-cmd-local.h. ] Tzvetomir Stoyanov (VMware) (8): trace-cmd: Move trace-cmd-local.h from the application to the library trace-cmd: Move trace-output.c into the library code trace-cmd: Move trace-msg.c into the library. trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd trace-cmd: Move trace-cmd global variable "debug" to libtracecmd trace-cmd: Move plog() function to libtracecmd. trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd Makefile | 3 - include/trace-cmd/trace-cmd.h | 19 +- .../include => include/trace-cmd}/trace-msg.h | 3 - include/version.h | 5 - lib/trace-cmd/Makefile | 2 + .../trace-cmd}/include/trace-cmd-local.h | 7 +- {tracecmd => lib/trace-cmd}/trace-msg.c | 4 +- {tracecmd => lib/trace-cmd}/trace-output.c | 5 +- lib/trace-cmd/trace-util.c | 162 ++++++++++++++++++ tracecmd/Makefile | 2 - tracecmd/include/trace-local.h | 14 +- tracecmd/trace-cmd.c | 3 - tracecmd/trace-list.c | 2 +- tracecmd/trace-listen.c | 77 ++------- tracecmd/trace-read.c | 8 +- tracecmd/trace-record.c | 8 +- tracecmd/trace-stack.c | 56 +----- 17 files changed, 218 insertions(+), 162 deletions(-) rename {tracecmd/include => include/trace-cmd}/trace-msg.h (79%) rename {tracecmd => lib/trace-cmd}/include/trace-cmd-local.h (83%) rename {tracecmd => lib/trace-cmd}/trace-msg.c (99%) rename {tracecmd => lib/trace-cmd}/trace-output.c (99%) -- 2.21.0
The trace-cmd-local.h file is used by trace-input.c, trace-output.c and trace-msg.c files. The first one is part of the libtracecmd library. The others will be moved to the same library, so this header file should be part of the library too. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- {tracecmd => lib/trace-cmd}/include/trace-cmd-local.h | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {tracecmd => lib/trace-cmd}/include/trace-cmd-local.h (100%) diff --git a/tracecmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h similarity index 100% rename from tracecmd/include/trace-cmd-local.h rename to lib/trace-cmd/include/trace-cmd-local.h -- 2.21.0
Functions, implemented in trace-output.c file, do not depend on trace-cmd application context and can be used standalone. The file is moved from trace-cmd to libtracecmd. It also fixes a warning while loading python modules from kernelshark: ImportError: ctracecmd.so: undefined symbol: tracecmd_append_cpu_data Since in trace-output.c is the only code, that uses FILE_VERSION_STRING define (the version of trace.dat file format), this define is moved from version.h in trace-cmd-local.h. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- [ v2 changes: - Removed trace-output.c dependency of version.h. Moved FILE_VERSION_STRING define from top Makefile to trace-cmd-local.h. ] Makefile | 3 --- include/version.h | 5 ----- lib/trace-cmd/Makefile | 1 + lib/trace-cmd/include/trace-cmd-local.h | 7 +++++++ {tracecmd => lib/trace-cmd}/trace-output.c | 1 - tracecmd/Makefile | 1 - 6 files changed, 8 insertions(+), 10 deletions(-) rename {tracecmd => lib/trace-cmd}/trace-output.c (99%) diff --git a/Makefile b/Makefile index d34c615..a848394 100644 --- a/Makefile +++ b/Makefile @@ -10,9 +10,6 @@ export TC_PATCHLEVEL export TC_EXTRAVERSION export TRACECMD_VERSION -# file format version -FILE_VERSION = 6 - MAKEFLAGS += --no-print-directory # Makefiles suck: This macro sets a default value of $(2) for the diff --git a/include/version.h b/include/version.h index 68ce79e..fcf7ba0 100644 --- a/include/version.h +++ b/include/version.h @@ -9,9 +9,4 @@ #include "tc_version.h" #endif -#define _STR(x) #x -#define STR(x) _STR(x) - -#define FILE_VERSION_STRING STR(FILE_VERSION) - #endif /* _VERSION_H */ diff --git a/lib/trace-cmd/Makefile b/lib/trace-cmd/Makefile index 44c1332..0543b91 100644 --- a/lib/trace-cmd/Makefile +++ b/lib/trace-cmd/Makefile @@ -10,6 +10,7 @@ OBJS = OBJS += trace-hash.o OBJS += trace-hooks.o OBJS += trace-input.o +OBJS += trace-output.o OBJS += trace-recorder.o OBJS += trace-util.o OBJS += trace-filter-hash.o diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h index fa96d4f..bad325f 100644 --- a/lib/trace-cmd/include/trace-cmd-local.h +++ b/lib/trace-cmd/include/trace-cmd-local.h @@ -11,6 +11,13 @@ #include "trace-cmd.h" #include "event-utils.h" +/* trace.dat file format version */ +#define FILE_VERSION 6 + +#define _STR(x) #x +#define STR(x) _STR(x) +#define FILE_VERSION_STRING STR(FILE_VERSION) + extern int quiet; static ssize_t __do_write(int fd, const void *data, size_t size) diff --git a/tracecmd/trace-output.c b/lib/trace-cmd/trace-output.c similarity index 99% rename from tracecmd/trace-output.c rename to lib/trace-cmd/trace-output.c index 33d6ce3..1f94346 100644 --- a/tracecmd/trace-output.c +++ b/lib/trace-cmd/trace-output.c @@ -23,7 +23,6 @@ #include "trace-cmd-local.h" #include "list.h" #include "trace-msg.h" -#include "version.h" /* We can't depend on the host size for size_t, all must be 64 bit */ typedef unsigned long long tsize_t; diff --git a/tracecmd/Makefile b/tracecmd/Makefile index bcd437a..6968f83 100644 --- a/tracecmd/Makefile +++ b/tracecmd/Makefile @@ -29,7 +29,6 @@ TRACE_CMD_OBJS += trace-restore.o TRACE_CMD_OBJS += trace-check-events.o TRACE_CMD_OBJS += trace-show.o TRACE_CMD_OBJS += trace-list.o -TRACE_CMD_OBJS += trace-output.o TRACE_CMD_OBJS += trace-usage.o TRACE_CMD_OBJS += trace-msg.o -- 2.21.0
Functions, implemented in trace-msg.c file, do not depend on trace-cmd application context and can be used standalone. The file is moved from trace-cmd to libtracecmd. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- {tracecmd/include => include/trace-cmd}/trace-msg.h | 0 lib/trace-cmd/Makefile | 1 + {tracecmd => lib/trace-cmd}/trace-msg.c | 0 tracecmd/Makefile | 1 - 4 files changed, 1 insertion(+), 1 deletion(-) rename {tracecmd/include => include/trace-cmd}/trace-msg.h (100%) rename {tracecmd => lib/trace-cmd}/trace-msg.c (100%) diff --git a/tracecmd/include/trace-msg.h b/include/trace-cmd/trace-msg.h similarity index 100% rename from tracecmd/include/trace-msg.h rename to include/trace-cmd/trace-msg.h diff --git a/lib/trace-cmd/Makefile b/lib/trace-cmd/Makefile index 0543b91..78875e4 100644 --- a/lib/trace-cmd/Makefile +++ b/lib/trace-cmd/Makefile @@ -14,6 +14,7 @@ OBJS += trace-output.o OBJS += trace-recorder.o OBJS += trace-util.o OBJS += trace-filter-hash.o +OBJS += trace-msg.o # Additional util objects OBJS += trace-blk-hack.o diff --git a/tracecmd/trace-msg.c b/lib/trace-cmd/trace-msg.c similarity index 100% rename from tracecmd/trace-msg.c rename to lib/trace-cmd/trace-msg.c diff --git a/tracecmd/Makefile b/tracecmd/Makefile index 6968f83..d491aae 100644 --- a/tracecmd/Makefile +++ b/tracecmd/Makefile @@ -30,7 +30,6 @@ TRACE_CMD_OBJS += trace-check-events.o TRACE_CMD_OBJS += trace-show.o TRACE_CMD_OBJS += trace-list.o TRACE_CMD_OBJS += trace-usage.o -TRACE_CMD_OBJS += trace-msg.o ALL_OBJS := $(TRACE_CMD_OBJS:%.o=$(bdir)/%.o) -- 2.21.0
A trace-cmd global variable "quiet" is used from libtracecmd and should be defined there. A new library APIs are implemented to access it: void tracecmd_set_quiet(int quiet); int tracecmd_get_quiet(void); Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- include/trace-cmd/trace-cmd.h | 3 +++ lib/trace-cmd/include/trace-cmd-local.h | 2 -- lib/trace-cmd/trace-output.c | 4 ++-- lib/trace-cmd/trace-util.c | 21 +++++++++++++++++++++ tracecmd/include/trace-local.h | 1 - tracecmd/trace-cmd.c | 1 - tracecmd/trace-record.c | 6 +++--- 7 files changed, 29 insertions(+), 9 deletions(-) diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h index c4a437a..5ce8fb3 100644 --- a/include/trace-cmd/trace-cmd.h +++ b/include/trace-cmd/trace-cmd.h @@ -49,6 +49,9 @@ enum { void tracecmd_record_ref(struct tep_record *record); void free_record(struct tep_record *record); +void tracecmd_set_quiet(int quiet); +int tracecmd_get_quiet(void); + struct tracecmd_input; struct tracecmd_output; struct tracecmd_recorder; diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h index bad325f..09574db 100644 --- a/lib/trace-cmd/include/trace-cmd-local.h +++ b/lib/trace-cmd/include/trace-cmd-local.h @@ -18,8 +18,6 @@ #define STR(x) _STR(x) #define FILE_VERSION_STRING STR(FILE_VERSION) -extern int quiet; - static ssize_t __do_write(int fd, const void *data, size_t size) { ssize_t tot = 0; diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c index 1f94346..35252ef 100644 --- a/lib/trace-cmd/trace-output.c +++ b/lib/trace-cmd/trace-output.c @@ -1157,7 +1157,7 @@ int tracecmd_write_cpu_data(struct tracecmd_output *handle, goto out_free; for (i = 0; i < cpus; i++) { - if (!quiet) + if (!tracecmd_get_quiet()) fprintf(stderr, "CPU%d data recorded at offset=0x%llx\n", i, (unsigned long long) offsets[i]); offset = lseek64(handle->fd, offsets[i], SEEK_SET); @@ -1172,7 +1172,7 @@ int tracecmd_write_cpu_data(struct tracecmd_output *handle, check_size, sizes[i]); goto out_free; } - if (!quiet) + if (!tracecmd_get_quiet()) fprintf(stderr, " %llu bytes in size\n", (unsigned long long)check_size); } diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c index 7c74bae..26b9a18 100644 --- a/lib/trace-cmd/trace-util.c +++ b/lib/trace-cmd/trace-util.c @@ -28,6 +28,7 @@ int tracecmd_disable_sys_plugins; int tracecmd_disable_plugins; +static int tracecmd_quiet; static struct registered_plugin_options { struct registered_plugin_options *next; @@ -96,6 +97,26 @@ char **trace_util_list_plugin_options(void) return list; } +/** + * tracecmd_set_quiet - Set if to print output to the screen + * @quiet: If non zero, print no output to the screen + * + */ +void tracecmd_set_quiet(int quiet) +{ + tracecmd_quiet = quiet; +} + +/** + * tracecmd_get_quiet - Get if to print output to the screen + * Returns non zero, if no output to the screen should be printed + * + */ +int tracecmd_get_quiet(void) +{ + return tracecmd_quiet; +} + void trace_util_free_plugin_options_list(char **list) { tracecmd_free_list(list); diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h index 78c52dc..23a3a29 100644 --- a/tracecmd/include/trace-local.h +++ b/tracecmd/include/trace-local.h @@ -13,7 +13,6 @@ #include "event-utils.h" extern int debug; -extern int quiet; /* fix stupid glib guint64 typecasts and printf formats */ typedef unsigned long long u64; diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c index 797b303..5283ba7 100644 --- a/tracecmd/trace-cmd.c +++ b/tracecmd/trace-cmd.c @@ -17,7 +17,6 @@ int silence_warnings; int show_status; int debug; -int quiet; void warning(const char *fmt, ...) { diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index b2ed6bf..b25b659 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -3387,7 +3387,7 @@ static void print_stat(struct buffer_instance *instance) { int cpu; - if (quiet) + if (tracecmd_get_quiet()) return; if (!is_top_instance(instance)) @@ -4207,7 +4207,7 @@ static void check_plugin(const char *plugin) } die ("Plugin '%s' does not exist", plugin); out: - if (!quiet) + if (!tracecmd_get_quiet()) fprintf(stderr, " plugin '%s'\n", plugin); free(buf); } @@ -5154,7 +5154,7 @@ static void parse_record_options(int argc, break; case OPT_quiet: case 'q': - quiet = 1; + tracecmd_set_quiet(1); break; default: usage(argv); -- 2.21.0
A trace-cmd global variable "debug" is used from libtracecmd and should be defined there. A new library APIs are implemented to access it: void tracecmd_set_debug(bool debug); bool tracecmd_get_debug(void); Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- include/trace-cmd/trace-cmd.h | 3 +++ lib/trace-cmd/trace-msg.c | 4 ++-- lib/trace-cmd/trace-util.c | 21 +++++++++++++++++++++ tracecmd/include/trace-local.h | 2 -- tracecmd/trace-cmd.c | 2 -- tracecmd/trace-list.c | 2 +- tracecmd/trace-listen.c | 8 ++++---- tracecmd/trace-read.c | 8 ++++---- tracecmd/trace-record.c | 2 +- 9 files changed, 36 insertions(+), 16 deletions(-) diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h index 5ce8fb3..b96de04 100644 --- a/include/trace-cmd/trace-cmd.h +++ b/include/trace-cmd/trace-cmd.h @@ -51,6 +51,9 @@ void free_record(struct tep_record *record); void tracecmd_set_quiet(int quiet); int tracecmd_get_quiet(void); +void tracecmd_set_debug(bool debug); +bool tracecmd_get_debug(void); + struct tracecmd_input; struct tracecmd_output; diff --git a/lib/trace-cmd/trace-msg.c b/lib/trace-cmd/trace-msg.c index e2dd188..92562c7 100644 --- a/lib/trace-cmd/trace-msg.c +++ b/lib/trace-cmd/trace-msg.c @@ -32,7 +32,7 @@ static inline void dprint(const char *fmt, ...) { va_list ap; - if (!debug) + if (!tracecmd_get_debug()) return; va_start(ap, fmt); @@ -351,7 +351,7 @@ static int tracecmd_msg_recv_wait(int fd, struct tracecmd_msg *msg) pfd.fd = fd; pfd.events = POLLIN; - ret = poll(&pfd, 1, debug ? -1 : msg_wait_to); + ret = poll(&pfd, 1, tracecmd_get_debug() ? -1 : msg_wait_to); if (ret < 0) return -errno; else if (ret == 0) diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c index 26b9a18..b5ce84f 100644 --- a/lib/trace-cmd/trace-util.c +++ b/lib/trace-cmd/trace-util.c @@ -29,6 +29,7 @@ int tracecmd_disable_sys_plugins; int tracecmd_disable_plugins; static int tracecmd_quiet; +static bool tracecmd_debug; static struct registered_plugin_options { struct registered_plugin_options *next; @@ -117,6 +118,26 @@ int tracecmd_get_quiet(void) return tracecmd_quiet; } +/** + * tracecmd_set_quiet - Set if to print output to the screen + * @quiet: If non zero, print no output to the screen + * + */ +void tracecmd_set_debug(bool debug) +{ + tracecmd_debug = debug; +} + +/** + * tracecmd_get_quiet - Get if to print output to the screen + * Returns non zero, if no output to the screen should be printed + * + */ +bool tracecmd_get_debug(void) +{ + return tracecmd_debug; +} + void trace_util_free_plugin_options_list(char **list) { tracecmd_free_list(list); diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h index 23a3a29..e2d5506 100644 --- a/tracecmd/include/trace-local.h +++ b/tracecmd/include/trace-local.h @@ -12,8 +12,6 @@ #include "trace-cmd.h" #include "event-utils.h" -extern int debug; - /* fix stupid glib guint64 typecasts and printf formats */ typedef unsigned long long u64; diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c index 5283ba7..30691b6 100644 --- a/tracecmd/trace-cmd.c +++ b/tracecmd/trace-cmd.c @@ -16,8 +16,6 @@ int silence_warnings; int show_status; -int debug; - void warning(const char *fmt, ...) { va_list ap; diff --git a/tracecmd/trace-list.c b/tracecmd/trace-list.c index 00c6073..832540e 100644 --- a/tracecmd/trace-list.c +++ b/tracecmd/trace-list.c @@ -427,7 +427,7 @@ void trace_list(int argc, char **argv) break; case '-': if (strcmp(argv[i], "--debug") == 0) { - debug = true; + tracecmd_set_debug(true); break; } fprintf(stderr, "list: invalid option -- '%s'\n", diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c index 3106022..3cbee67 100644 --- a/tracecmd/trace-listen.c +++ b/tracecmd/trace-listen.c @@ -717,7 +717,7 @@ static int do_fork(int cfd) pid_t pid; /* in debug mode, we do not fork off children */ - if (debug) + if (tracecmd_get_debug()) return 0; pid = fork(); @@ -769,7 +769,7 @@ static int do_connection(int cfd, struct sockaddr_storage *peer_addr, tracecmd_msg_handle_close(msg_handle); - if (!debug) + if (!tracecmd_get_debug()) exit(0); return 0; @@ -910,7 +910,7 @@ static void do_listen(char *port) struct addrinfo *result, *rp; int sfd, s; - if (!debug) + if (!tracecmd_get_debug()) signal_setup(SIGCHLD, sigstub); make_pid_file(); @@ -1009,7 +1009,7 @@ void trace_listen(int argc, char **argv) daemon = 1; break; case OPT_debug: - debug = 1; + tracecmd_set_debug(true); break; default: usage(argv); diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c index d22c723..12b8b3d 100644 --- a/tracecmd/trace-read.c +++ b/tracecmd/trace-read.c @@ -782,10 +782,10 @@ void trace_show_data(struct tracecmd_input *handle, struct tep_record *record) cpu, record->missed_events); else if (record->missed_events < 0) trace_seq_printf(&s, "CPU:%d [EVENTS DROPPED]\n", cpu); - if (buffer_breaks || debug) { + if (buffer_breaks || tracecmd_get_debug()) { if (tracecmd_record_at_buffer_start(handle, record)) { trace_seq_printf(&s, "CPU:%d [SUBBUFFER START]", cpu); - if (debug) + if (tracecmd_get_debug()) trace_seq_printf(&s, " [%lld:0x%llx]", tracecmd_page_ts(handle, record), record->offset & ~(page_size - 1)); @@ -816,7 +816,7 @@ void trace_show_data(struct tracecmd_input *handle, struct tep_record *record) tep_print_event(pevent, &s, record, use_trace_clock); if (s.len && *(s.buffer + s.len - 1) == '\n') s.len--; - if (debug) { + if (tracecmd_get_debug()) { struct kbuffer *kbuf; struct kbuffer_raw_info info; void *page; @@ -1616,7 +1616,7 @@ void trace_report (int argc, char **argv) break; case OPT_debug: buffer_breaks = 1; - debug = 1; + tracecmd_set_debug(true); break; case OPT_profile: profile = 1; diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index b25b659..32793b3 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -5143,7 +5143,7 @@ static void parse_record_options(int argc, no_filter = true; break; case OPT_debug: - debug = 1; + tracecmd_set_debug(true); break; case OPT_module: if (ctx->instance->filter_mod) -- 2.21.0
plog() function writes logs into a log file. It is used in libtracecmd and its implementation should be there. The function is moved from trace-cmd into the library, and 2 additional APIs are implemented: int trace_set_log_file(char *logfile); - use it to set the log file. void plog_error(const char *fmt, ...); - use it to log an error message into the file. The plog() function is used also from pdie() in trace-cmd. pdie() depends on trace-cmd context and cannot be moved to the library. It is reimplemented as macros, in order to utilize the new plog() library function. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- include/trace-cmd/trace-cmd.h | 4 ++ include/trace-cmd/trace-msg.h | 3 -- lib/trace-cmd/trace-util.c | 71 +++++++++++++++++++++++++++++++++++ tracecmd/trace-listen.c | 69 ++++------------------------------ 4 files changed, 83 insertions(+), 64 deletions(-) diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h index b96de04..8db0686 100644 --- a/include/trace-cmd/trace-cmd.h +++ b/include/trace-cmd/trace-cmd.h @@ -398,6 +398,10 @@ struct hook_list { struct hook_list *tracecmd_create_event_hook(const char *arg); void tracecmd_free_hooks(struct hook_list *hooks); +void plog(const char *fmt, ...); +void plog_error(const char *fmt, ...); +int trace_set_log_file(char *logfile); + /* --- Hack! --- */ int tracecmd_blk_hack(struct tracecmd_input *handle); diff --git a/include/trace-cmd/trace-msg.h b/include/trace-cmd/trace-msg.h index b7fe10b..aab8a69 100644 --- a/include/trace-cmd/trace-msg.h +++ b/include/trace-cmd/trace-msg.h @@ -12,7 +12,4 @@ extern unsigned int page_size; -void plog(const char *fmt, ...); -void pdie(const char *fmt, ...); - #endif /* _TRACE_MSG_H_ */ diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c index b5ce84f..8c1a0a0 100644 --- a/lib/trace-cmd/trace-util.c +++ b/lib/trace-cmd/trace-util.c @@ -31,6 +31,8 @@ int tracecmd_disable_plugins; static int tracecmd_quiet; static bool tracecmd_debug; +static FILE *trace_logfp; + static struct registered_plugin_options { struct registered_plugin_options *next; struct tep_plugin_option *options; @@ -1716,3 +1718,72 @@ void __weak *malloc_or_die(unsigned int size) die("malloc"); return data; } + +#define LOG_BUF_SIZE 1024 +static void __plog(const char *prefix, const char *fmt, va_list ap, FILE *fp) +{ + static int newline = 1; + char buf[LOG_BUF_SIZE]; + int r; + + r = vsnprintf(buf, LOG_BUF_SIZE, fmt, ap); + + if (r > LOG_BUF_SIZE) + r = LOG_BUF_SIZE; + + if (trace_logfp) { + if (newline) + fprintf(trace_logfp, "[%d]%s%.*s", getpid(), prefix, r, buf); + else + fprintf(trace_logfp, "[%d]%s%.*s", getpid(), prefix, r, buf); + newline = buf[r - 1] == '\n'; + fflush(trace_logfp); + return; + } + + fprintf(fp, "%.*s", r, buf); +} + +void plog(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + __plog("", fmt, ap, stdout); + va_end(ap); + /* Make sure it gets to the screen, in case we crash afterward */ + fflush(stdout); +} + +void plog_error(const char *fmt, ...) +{ + va_list ap; + char *str = ""; + + va_start(ap, fmt); + __plog("Error: ", fmt, ap, stderr); + va_end(ap); + if (errno) + str = strerror(errno); + if (trace_logfp) + fprintf(trace_logfp, "\n%s\n", str); + else + fprintf(stderr, "\n%s\n", str); +} + +/** + * trace_set_log_file - Set file for logging + * @logfile: Name of the log file + * + * Returns 0 on successful completion or -1 in case of error + */ +int trace_set_log_file(char *logfile) +{ + if (trace_logfp) + fclose(trace_logfp); + trace_logfp = fopen(logfile, "w"); + if (!trace_logfp) + return -1; + return 0; +} + diff --git a/tracecmd/trace-listen.c b/tracecmd/trace-listen.c index 3cbee67..e0dcd71 100644 --- a/tracecmd/trace-listen.c +++ b/tracecmd/trace-listen.c @@ -34,8 +34,6 @@ static char *output_dir; static char *default_output_file = "trace"; static char *output_file; -static FILE *logfp; - static int backlog = 5; static int do_daemon; @@ -44,6 +42,13 @@ static int do_daemon; static struct tracecmd_msg_handle *stop_msg_handle; static bool done; +#define pdie(fmt, ...) \ + do { \ + plog_error(fmt, ##__VA_ARGS__); \ + remove_pid_file(); \ + exit(-1); \ + } while (0) + #define TEMP_FILE_STR "%s.%s:%s.cpu%d", output_file, host, port, cpu static char *get_temp_file(const char *host, const char *port, int cpu) { @@ -114,43 +119,6 @@ static void finish(int sig) done = true; } -#define LOG_BUF_SIZE 1024 -static void __plog(const char *prefix, const char *fmt, va_list ap, - FILE *fp) -{ - static int newline = 1; - char buf[LOG_BUF_SIZE]; - int r; - - r = vsnprintf(buf, LOG_BUF_SIZE, fmt, ap); - - if (r > LOG_BUF_SIZE) - r = LOG_BUF_SIZE; - - if (logfp) { - if (newline) - fprintf(logfp, "[%d]%s%.*s", getpid(), prefix, r, buf); - else - fprintf(logfp, "[%d]%s%.*s", getpid(), prefix, r, buf); - newline = buf[r - 1] == '\n'; - fflush(logfp); - return; - } - - fprintf(fp, "%.*s", r, buf); -} - -void plog(const char *fmt, ...) -{ - va_list ap; - - va_start(ap, fmt); - __plog("", fmt, ap, stdout); - va_end(ap); - /* Make sure it gets to the screen, in case we crash afterward */ - fflush(stdout); -} - static void make_pid_name(int mode, char *buf) { snprintf(buf, PATH_MAX, VAR_RUN_DIR "/trace-cmd-net.pid"); @@ -169,26 +137,6 @@ static void remove_pid_file(void) unlink(buf); } -void pdie(const char *fmt, ...) -{ - va_list ap; - char *str = ""; - - va_start(ap, fmt); - __plog("Error: ", fmt, ap, stderr); - va_end(ap); - if (errno) - str = strerror(errno); - if (logfp) - fprintf(logfp, "\n%s\n", str); - else - fprintf(stderr, "\n%s\n", str); - - remove_pid_file(); - - exit(-1); -} - static int process_udp_child(int sfd, const char *host, const char *port, int cpu, int page_size, int use_tcp) { @@ -1030,8 +978,7 @@ void trace_listen(int argc, char **argv) if (logfile) { /* set the writes to a logfile instead */ - logfp = fopen(logfile, "w"); - if (!logfp) + if (trace_set_log_file(logfile) < 0) die("creating log file %s", logfile); } -- 2.21.0
These APIs depend on trace-cmd context and cannot be used standalone: tracecmd_create_top_instance() tracecmd_remove_instances() tracecmd_filter_pid() tracecmd_add_event() tracecmd_enable_events() tracecmd_disable_all_tracing() tracecmd_disable_tracing() tracecmd_enable_tracing() tracecmd_stat_cpu() They are implemented in trace-cmd application, but declared as APIs in trace-cmd.h. The declarations are moved from trace-cmd.h to trace-local.h, local header file visible only to trace-cmd application. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- include/trace-cmd/trace-cmd.h | 9 --------- tracecmd/include/trace-local.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h index 8db0686..2c8d798 100644 --- a/include/trace-cmd/trace-cmd.h +++ b/include/trace-cmd/trace-cmd.h @@ -137,8 +137,6 @@ int tracecmd_buffer_instances(struct tracecmd_input *handle); const char *tracecmd_buffer_instance_name(struct tracecmd_input *handle, int indx); struct tracecmd_input *tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx); int tracecmd_is_buffer_instance(struct tracecmd_input *handle); -void tracecmd_create_top_instance(char *name); -void tracecmd_remove_instances(void); void tracecmd_set_ts_offset(struct tracecmd_input *handle, unsigned long long offset); void tracecmd_set_ts2secs(struct tracecmd_input *handle, unsigned long long hz); @@ -304,14 +302,7 @@ struct tracecmd_recorder *tracecmd_create_buffer_recorder_maxkb(const char *file int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long sleep); void tracecmd_stop_recording(struct tracecmd_recorder *recorder); -void tracecmd_stat_cpu(struct trace_seq *s, int cpu); long tracecmd_flush_recording(struct tracecmd_recorder *recorder); -void tracecmd_filter_pid(int pid, int exclude); -int tracecmd_add_event(const char *event_str, int stack); -void tracecmd_enable_events(void); -void tracecmd_disable_all_tracing(int disable_tracer); -void tracecmd_disable_tracing(void); -void tracecmd_enable_tracing(void); enum tracecmd_msg_flags { TRACECMD_MSG_FL_USE_TCP = 1 << 0, diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h index e2d5506..05760d8 100644 --- a/tracecmd/include/trace-local.h +++ b/tracecmd/include/trace-local.h @@ -216,6 +216,17 @@ void show_instance_file(struct buffer_instance *instance, const char *name); int count_cpus(void); +/* moved from trace-cmd.h */ +void tracecmd_create_top_instance(char *name); +void tracecmd_remove_instances(void); +void tracecmd_filter_pid(int pid, int exclude); +int tracecmd_add_event(const char *event_str, int stack); +void tracecmd_enable_events(void); +void tracecmd_disable_all_tracing(int disable_tracer); +void tracecmd_disable_tracing(void); +void tracecmd_enable_tracing(void); +void tracecmd_stat_cpu(struct trace_seq *s, int cpu); + /* No longer in event-utils.h */ void __noreturn die(const char *fmt, ...); /* Can be overriden */ void *malloc_or_die(unsigned int size); /* Can be overridden */ -- 2.21.0
tracecmd_stack_tracer_status() function reads the stack tracer status from the proc file system. It does not depend on trace-cmd context and can be used standalone. The function is moved from trace-cmd application into libtracecmd. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- lib/trace-cmd/trace-util.c | 49 +++++++++++++++++++++++++++++++++ tracecmd/trace-stack.c | 56 ++------------------------------------ 2 files changed, 51 insertions(+), 54 deletions(-) diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c index 8c1a0a0..d16a018 100644 --- a/lib/trace-cmd/trace-util.c +++ b/lib/trace-cmd/trace-util.c @@ -25,6 +25,7 @@ #define LOCAL_PLUGIN_DIR ".trace-cmd/plugins" #define TRACEFS_PATH "/sys/kernel/tracing" #define DEBUGFS_PATH "/sys/kernel/debug" +#define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled" int tracecmd_disable_sys_plugins; int tracecmd_disable_plugins; @@ -1787,3 +1788,51 @@ int trace_set_log_file(char *logfile) return 0; } +/** + * tracecmd_stack_tracer_status - Check stack trace status + * @status: Returned stack trace status: + * 0 - not configured, disabled + * non 0 - enabled + * + * Returns -1 in case of an error, 0 if file does not exist + * (stack tracer not enabled) or 1 on successful completion. + */ +int tracecmd_stack_tracer_status(int *status) +{ + struct stat stat_buf; + char buf[64]; + long num; + int fd; + int n; + + if (stat(PROC_STACK_FILE, &stat_buf) < 0) { + /* stack tracer not configured on running kernel */ + *status = 0; /* not configured means disabled */ + return 0; + } + + fd = open(PROC_STACK_FILE, O_RDONLY); + + if (fd < 0) + return -1; + + n = read(fd, buf, sizeof(buf)); + close(fd); + + if (n <= 0) + return -1; + + if (n >= sizeof(buf)) + return -1; + + buf[n] = 0; + errno = 0; + num = strtol(buf, NULL, 10); + + /* Check for various possible errors */ + if (num > INT_MAX || num < INT_MIN || (!num && errno)) + return -1; + + *status = num; + return 1; /* full success */ +} diff --git a/tracecmd/trace-stack.c b/tracecmd/trace-stack.c index 34b3b58..bb002c0 100644 --- a/tracecmd/trace-stack.c +++ b/tracecmd/trace-stack.c @@ -36,58 +36,6 @@ static void test_available(void) die("stack tracer not configured on running kernel"); } -/* - * Returns: - * -1 - Something went wrong - * 0 - File does not exist (stack tracer not enabled) - * 1 - Success - */ -static int read_proc(int *status) -{ - struct stat stat_buf; - char buf[64]; - long num; - int fd; - int n; - - if (stat(PROC_FILE, &stat_buf) < 0) { - /* stack tracer not configured on running kernel */ - *status = 0; /* not configured means disabled */ - return 0; - } - - fd = open(PROC_FILE, O_RDONLY); - - if (fd < 0) - return -1; - - n = read(fd, buf, sizeof(buf)); - close(fd); - - if (n <= 0) - return -1; - - if (n >= sizeof(buf)) - return -1; - - buf[n] = 0; - errno = 0; - num = strtol(buf, NULL, 10); - - /* Check for various possible errors */ - if (num > INT_MAX || num < INT_MIN || (!num && errno)) - return -1; - - *status = num; - return 1; /* full success */ -} - -/* Public wrapper of read_proc() */ -int tracecmd_stack_tracer_status(int *status) -{ - return read_proc(status); -} - /* NOTE: this implementation only accepts new_status in the range [0..9]. */ static void change_stack_tracer_status(unsigned new_status) { @@ -102,7 +50,7 @@ static void change_stack_tracer_status(unsigned new_status) return; } - ret = read_proc(&status); + ret = tracecmd_stack_tracer_status(&status); if (ret < 0) die("error reading %s", PROC_FILE); @@ -160,7 +108,7 @@ static void read_trace(void) size_t n; int r; - if (read_proc(&status) <= 0) + if (tracecmd_stack_tracer_status(&status) <= 0) die("Invalid stack tracer state"); if (status > 0) -- 2.21.0
New options to trace-cmd record are added: - "--proc-map" - Saves traced process(es) address map in trace.dat file. - "--user" - Executes the traced process as given user. As "--proc-map" option uses trace-cmd ptrace logic, part of this code is rewritten. There were some leftovers related to this logic, the code is upadted. [ v5 changes: - Added new patch: "Extend ptrace logic to work with multiple filtered pids" It resolves "filter_pid" leftover in ptrace related logic. - "--proc-map" does not depend on option -F, it works with any command, specified as trace-cmd argument or option -P. - Renamed "mmap" to "proc-map" - the option name and the names of the functions, variables and defines related to this feature. v4 changes: - Added check for strdup() failure. - Made input user string argument of change_user() and run_cmd() constant. - Added description of the new "--mmap" trace-cmd option in the program's help and the man page. (Suggested by Slavomir Kaslev) Problems, reported by Yordan Karadzhov: - Improved the parsing of /proc/<pid>/maps. Made it not so strict, as it failed on some machines due to different size of fields. - Implemented trace_pid_mmap_free() cleanup function to free mmap related resources at trace-cmd exit. - Fixed potential problem with non-terminated string, returned by readlink(). - Coding style fixes. v3 changes: - "--user" does not depend on option -F, it works with any command, specified as trace-cmd argument. - Changed tracecmd_search_task_mmap() API to return not only the library name, but also the start and end memory addresses. - Renamed *tracee* to *task* - Improved resources cleanup, in case of an error. - Removed (this) changelog from the commit message. v2 changes: - Check for errors in change_user(). If an error occurs while changing the user, the message is printed and the traced process is not executed. - Replaced usage of tracecmd_add_option_v() with tracecmd_add_option() API. - Added checks to prevent buffer overflow when sscanf (... "%s", buf) is used. - Return error in case memory allocation fails. - Return error if option string is not in the expected format. - Sort memory maps and use binary search to find matching library in the map. ] Tzvetomir Stoyanov (VMware) (3): trace-cmd: Extend ptrace logic to work with multiple filtered pids trace-cmd: Save the tracee address map into the trace.dat file. trace-cmd: Add option to execute traced process as given user Documentation/trace-cmd-record.1.txt | 6 + include/trace-cmd/trace-cmd.h | 10 + lib/trace-cmd/trace-input.c | 172 +++++++++++++++- tracecmd/include/trace-local.h | 10 + tracecmd/trace-record.c | 282 +++++++++++++++++++++++++-- tracecmd/trace-usage.c | 2 + 6 files changed, 464 insertions(+), 18 deletions(-) -- 2.21.0
In the trace-record.c file there is a global variable named "filter_pid". It is not set anywhere in the code, but there is a logic which relies on it. This variable is a leftover from the past implementation of trace-cmd record "-P" option, when it was designed to filter only a single PID. Now "-P" option works with a list of PIDs, stored in filter_pids global list. The code is modified to work with filter_pids instead of filter_pid. This logic is used only if there is no ftrace "options/event-fork" on the system and we have ptrace support. There is one significant change in the trace-cmd record behavior in this specific use case: - filtered pids are specified with the "-P" option. - there is no ftrace "options/event-fork" on the system. - there is ptrace support. The trace continues until Ctrl^C is hit or all filtered PIDs exit, whatever comes first. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- tracecmd/trace-record.c | 65 ++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index 5dc6f17..e0fa07d 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -86,7 +86,6 @@ static bool use_tcp; static int do_ptrace; static int filter_task; -static int filter_pid = -1; static bool no_filter = false; static int local_cpu_count; @@ -866,6 +865,13 @@ static void add_filter_pid(int pid, int exclude) struct filter_pids *p; char buf[100]; + for (p = filter_pids; p; p = p->next) { + if (p->pid == pid) { + p->exclude = exclude; + return; + } + } + p = malloc(sizeof(*p)); if (!p) die("Failed to allocate pid filter"); @@ -1223,17 +1229,34 @@ static void enable_ptrace(void) ptrace(PTRACE_TRACEME, 0, NULL, 0); } -static void ptrace_wait(enum trace_type type, int main_pid) +static void ptrace_wait(enum trace_type type) { + struct filter_pids *fpid; unsigned long send_sig; unsigned long child; siginfo_t sig; + int main_pids; int cstatus; int status; + int i = 0; + int *pids; int event; int pid; int ret; + pids = calloc(nr_filter_pids, sizeof(int)); + if (!pids) + return; + + for (fpid = filter_pids; fpid; fpid = fpid->next) { + if (fpid->exclude) + continue; + pids[i++] = fpid->pid; + if (i >= nr_filter_pids) + break; + } + main_pids = i; + do { ret = trace_waitpid(type, -1, &status, WSTOPPED | __WALL); if (ret < 0) @@ -1275,11 +1298,24 @@ static void ptrace_wait(enum trace_type type, int main_pid) PTRACE_O_TRACEEXIT); ptrace(PTRACE_CONT, pid, NULL, send_sig); } - } while (!finished && ret > 0 && - (!WIFEXITED(status) || pid != main_pid)); + if (WIFEXITED(status) || + (WIFSTOPPED(status) && event == PTRACE_EVENT_EXIT)) { + for (i = 0; i < nr_filter_pids; i++) { + if (pid == pids[i]) { + pids[i] = 0; + main_pids--; + if (!main_pids) + finished = 1; + break; + } + } + } + } while (!finished && ret > 0); + + free(pids); } #else -static inline void ptrace_wait(enum trace_type type, int main_pid) { } +static inline void ptrace_wait(enum trace_type type) { } static inline void enable_ptrace(void) { } static inline void ptrace_attach(int pid) { } @@ -1289,8 +1325,8 @@ static void trace_or_sleep(enum trace_type type) { struct timeval tv = { 1 , 0 }; - if (do_ptrace && filter_pid >= 0) - ptrace_wait(type, filter_pid); + if (do_ptrace && filter_pids) + ptrace_wait(type); else if (type & TRACE_TYPE_STREAM) trace_stream_read(pids, recorder_threads, &tv); else @@ -1327,7 +1363,7 @@ static void run_cmd(enum trace_type type, int argc, char **argv) } if (do_ptrace) { add_filter_pid(pid, 0); - ptrace_wait(type, pid); + ptrace_wait(type); } else trace_waitpid(type, pid, &status, 0); } @@ -2318,7 +2354,7 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol *event = *old_event; add_event(instance, event); - if (event->filter || filter_task || filter_pid) { + if (event->filter || filter_task || filter_pids) { event->filter_file = strdup(path); if (!event->filter_file) die("malloc filter file"); @@ -4924,9 +4960,9 @@ static void parse_record_options(int argc, add_func(&ctx->instance->filter_funcs, ctx->instance->filter_mod, "*"); - if (do_ptrace && !filter_task && (filter_pid < 0)) + if (do_ptrace && !filter_task && !nr_filter_pids) die(" -c can only be used with -F (or -P with event-fork support)"); - if (ctx->do_child && !filter_task &&! filter_pid) + if (ctx->do_child && !filter_task && !nr_filter_pids) die(" -c can only be used with -P or -F"); if ((argc - optind) >= 2) { @@ -4997,6 +5033,7 @@ static void record_trace(int argc, char **argv, { enum trace_type type = get_trace_cmd_type(ctx->curr_cmd); struct buffer_instance *instance; + struct filter_pids *pid; /* * If top_instance doesn't have any plugins or events, then @@ -5083,8 +5120,10 @@ static void record_trace(int argc, char **argv, update_task_filter(); tracecmd_enable_tracing(); /* We don't ptrace ourself */ - if (do_ptrace && filter_pid >= 0) - ptrace_attach(filter_pid); + if (do_ptrace && filter_pids) + for (pid = filter_pids; pid; pid = pid->next) + if (!pid->exclude) + ptrace_attach(pid->pid); /* sleep till we are woken with Ctrl^C */ printf("Hit Ctrl^C to stop recording\n"); while (!finished) -- 2.21.0
A new trace-cmd record option is added: "--proc-map". When it is set the address map of the traced applications is stored in the trace.dat file. The traced applications can be specified using the option -P, or as a given 'command'. A new API tracecmd_search_task_map() can be used to look up into stored address maps. The map is retrieved from /proc/<pid>/maps file. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- [ v5 changes: - Added new patch: "Extend ptrace logic to work with multiple filtered pids" It resolves "filter_pid" leftover in ptrace related logic. - "--proc-map" does not depend on option -F, it works with any command, specified as trace-cmd argument or option -P. - Renamed "mmap" to "proc-map" - the option name and the names of the functions, variables and defines related to this feature. v4 changes: - Added description of the new "--mmap" trace-cmd option in the program's help and the man page. (Suggested by Slavomir Kaslev) Problems, reported by Yordan Karadzhov: - Improved the parsing of /proc/<pid>/maps. Made it not so strict, as it failed on some machines due to different size of fields. - Implemented trace_pid_mmap_free() cleanup function to free mmap related resources at trace-cmd exit. - Fixed potential problem with non-terminated string, returned by readlink(). - Coding style fixes. v3 changes: - Changed tracecmd_search_task_mmap() API to return not only the library name, but also the start and end memory addresses. - Renamed *tracee* to *task* - Improved resources cleanup, in case of an error. - Removed (this) changelog from the commit message. v2 changes: - Replaced usage of tracecmd_add_option_v() with tracecmd_add_option() API. - Added checks to prevent buffer overflow when sscanf (... "%s", buf) is used. - Return error in case memory allocation fails. - Return error if option string is not in the expected format. - Sort memory maps and use binary search to find matching library in the map. ] Documentation/trace-cmd-record.1.txt | 3 + include/trace-cmd/trace-cmd.h | 10 ++ lib/trace-cmd/trace-input.c | 172 ++++++++++++++++++++++++++- tracecmd/include/trace-local.h | 10 ++ tracecmd/trace-record.c | 172 ++++++++++++++++++++++++++- tracecmd/trace-usage.c | 1 + 6 files changed, 364 insertions(+), 4 deletions(-) diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt index 26a8299..e697f03 100644 --- a/Documentation/trace-cmd-record.1.txt +++ b/Documentation/trace-cmd-record.1.txt @@ -288,6 +288,9 @@ OPTIONS '--module snd -n "*"' is equivalent to '-n :mod:snd' +*--proc-map*:: + Save the traced process address map into the trace.dat file. The traced + processes can be specified using the option *-P*, or as a given 'command'. *--profile*:: With the *--profile* option, "trace-cmd" will enable tracing that can diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h index 6f62ab9..c4a437a 100644 --- a/include/trace-cmd/trace-cmd.h +++ b/include/trace-cmd/trace-cmd.h @@ -82,6 +82,7 @@ enum { TRACECMD_OPTION_OFFSET, TRACECMD_OPTION_CPUCOUNT, TRACECMD_OPTION_VERSION, + TRACECMD_OPTION_PROCMAPS, }; enum { @@ -97,6 +98,12 @@ struct tracecmd_ftrace { int long_size; }; +struct tracecmd_proc_addr_map { + unsigned long long start; + unsigned long long end; + char *lib_name; +}; + typedef void (*tracecmd_show_data_func)(struct tracecmd_input *handle, struct tep_record *record); typedef void (*tracecmd_handle_init_func)(struct tracecmd_input *handle, @@ -208,6 +215,9 @@ unsigned long long tracecmd_page_ts(struct tracecmd_input *handle, unsigned int tracecmd_record_ts_delta(struct tracecmd_input *handle, struct tep_record *record); +struct tracecmd_proc_addr_map * +tracecmd_search_task_map(struct tracecmd_input *handle, + int pid, unsigned long long addr); #ifndef SWIG /* hack for function graph work around */ extern __thread struct tracecmd_input *tracecmd_curr_thread_handle; diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c index 654101f..a6fa7f5 100644 --- a/lib/trace-cmd/trace-input.c +++ b/lib/trace-cmd/trace-input.c @@ -101,6 +101,7 @@ struct tracecmd_input { struct tracecmd_ftrace finfo; struct hook_list *hooks; + struct pid_addr_maps *pid_maps; /* file information */ size_t header_files_start; size_t ftrace_files_start; @@ -2136,6 +2137,167 @@ void tracecmd_set_ts2secs(struct tracecmd_input *handle, handle->use_trace_clock = false; } +static int trace_pid_map_cmp(const void *a, const void *b) +{ + struct tracecmd_proc_addr_map *m_a = (struct tracecmd_proc_addr_map *)a; + struct tracecmd_proc_addr_map *m_b = (struct tracecmd_proc_addr_map *)b; + + if (m_a->start > m_b->start) + return 1; + if (m_a->start < m_b->start) + return -1; + return 0; +} + +static void procmap_free(struct pid_addr_maps *maps) +{ + int i; + + if (!maps) + return; + if (maps->lib_maps) { + for (i = 0; i < maps->nr_lib_maps; i++) + free(maps->lib_maps[i].lib_name); + free(maps->lib_maps); + } + free(maps->proc_name); + free(maps); +} + +#define STR_PROCMAP_LINE_MAX (PATH_MAX+22) +static int trace_pid_map_load(struct tracecmd_input *handle, char *buf) +{ + struct pid_addr_maps *maps = NULL; + char mapname[STR_PROCMAP_LINE_MAX]; + char *line; + int res; + int ret; + int i; + + maps = calloc(1, sizeof(*maps)); + if (!maps) + return -ENOMEM; + + ret = -EINVAL; + line = strchr(buf, '\n'); + if (!line) + goto out_fail; + + *line = '\0'; + if (strlen(buf) > STR_PROCMAP_LINE_MAX) + goto out_fail; + + res = sscanf(buf, "%x %x %s", &maps->pid, &maps->nr_lib_maps, mapname); + if (res != 3) + goto out_fail; + + ret = -ENOMEM; + maps->proc_name = strdup(mapname); + if (!maps->proc_name) + goto out_fail; + + maps->lib_maps = calloc(maps->nr_lib_maps, sizeof(struct tracecmd_proc_addr_map)); + if (!maps->lib_maps) + goto out_fail; + + buf = line + 1; + line = strchr(buf, '\n'); + for (i = 0; i < maps->nr_lib_maps; i++) { + if (!line) + break; + *line = '\0'; + if (strlen(buf) > STR_PROCMAP_LINE_MAX) + break; + res = sscanf(buf, "%llx %llx %s", &maps->lib_maps[i].start, + &maps->lib_maps[i].end, mapname); + if (res != 3) + break; + maps->lib_maps[i].lib_name = strdup(mapname); + if (!maps->lib_maps[i].lib_name) + goto out_fail; + buf = line + 1; + line = strchr(buf, '\n'); + } + + ret = -EINVAL; + if (i != maps->nr_lib_maps) + goto out_fail; + + qsort(maps->lib_maps, maps->nr_lib_maps, + sizeof(*maps->lib_maps), trace_pid_map_cmp); + + maps->next = handle->pid_maps; + handle->pid_maps = maps; + + return 0; + +out_fail: + procmap_free(maps); + return ret; +} + +static void trace_pid_map_free(struct pid_addr_maps *maps) +{ + struct pid_addr_maps *del; + + while (maps) { + del = maps; + maps = maps->next; + procmap_free(del); + } +} + +static int trace_pid_map_search(const void *a, const void *b) +{ + struct tracecmd_proc_addr_map *key = (struct tracecmd_proc_addr_map *)a; + struct tracecmd_proc_addr_map *map = (struct tracecmd_proc_addr_map *)b; + + if (key->start >= map->end) + return 1; + if (key->start < map->start) + return -1; + return 0; +} + +/** + * tracecmd_search_task_map - Search task memory address map + * @handle: input handle to the trace.dat file + * @pid: pid of the task + * @addr: address from the task memory space. + * + * Map of the task memory can be saved in the trace.dat file, using the option + * "--proc-map". If there is such information, this API can be used to look up + * into this memory map to find what library is loaded at the given @addr. + * + * A pointer to struct tracecmd_proc_addr_map is returned, containing the name + * of the library at given task @addr and the library start and end addresses. + */ +struct tracecmd_proc_addr_map * +tracecmd_search_task_map(struct tracecmd_input *handle, + int pid, unsigned long long addr) +{ + struct tracecmd_proc_addr_map *lib; + struct tracecmd_proc_addr_map key; + struct pid_addr_maps *maps; + + if (!handle || !handle->pid_maps) + return NULL; + + maps = handle->pid_maps; + while (maps) { + if (maps->pid == pid) + break; + maps = maps->next; + } + if (!maps || !maps->nr_lib_maps || !maps->lib_maps) + return NULL; + key.start = addr; + lib = bsearch(&key, maps->lib_maps, maps->nr_lib_maps, + sizeof(*maps->lib_maps), trace_pid_map_search); + + return lib; +} + static int handle_options(struct tracecmd_input *handle) { unsigned long long offset; @@ -2223,9 +2385,6 @@ static int handle_options(struct tracecmd_input *handle) case TRACECMD_OPTION_UNAME: handle->uname = strdup(buf); break; - case TRACECMD_OPTION_VERSION: - handle->version = strdup(buf); - break; case TRACECMD_OPTION_HOOK: hook = tracecmd_create_event_hook(buf); hook->next = handle->hooks; @@ -2235,6 +2394,10 @@ static int handle_options(struct tracecmd_input *handle) cpus = *(int *)buf; handle->cpus = tep_read_number(handle->pevent, &cpus, 4); break; + case TRACECMD_OPTION_PROCMAPS: + if (buf[size-1] == '\0') + trace_pid_map_load(handle, buf); + break; default: warning("unknown option %d", option); break; @@ -2848,6 +3011,9 @@ void tracecmd_close(struct tracecmd_input *handle) tracecmd_free_hooks(handle->hooks); handle->hooks = NULL; + trace_pid_map_free(handle->pid_maps); + handle->pid_maps = NULL; + if (handle->flags & TRACECMD_FL_BUFFER_INSTANCE) tracecmd_close(handle->parent); else { diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h index 1cad3cc..78c52dc 100644 --- a/tracecmd/include/trace-local.h +++ b/tracecmd/include/trace-local.h @@ -157,6 +157,14 @@ struct func_list { const char *mod; }; +struct pid_addr_maps { + struct pid_addr_maps *next; + struct tracecmd_proc_addr_map *lib_maps; + unsigned int nr_lib_maps; + char *proc_name; + int pid; +}; + struct buffer_instance { struct buffer_instance *next; const char *name; @@ -183,6 +191,8 @@ struct buffer_instance { struct tracecmd_msg_handle *msg_handle; struct tracecmd_output *network_handle; + struct pid_addr_maps *pid_maps; + char *max_graph_depth; int flags; diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index e0fa07d..17c676c 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -84,6 +84,8 @@ static int max_kb; static bool use_tcp; static int do_ptrace; +static int do_children; +static int get_procmap; static int filter_task; static bool no_filter = false; @@ -1068,6 +1070,121 @@ static char *make_pid_filter(char *curr_filter, const char *field) return filter; } +#define _STRINGIFY(x) #x +#define STRINGIFY(x) _STRINGIFY(x) + +static int get_pid_addr_maps(int pid) +{ + struct buffer_instance *instance = &top_instance; + struct pid_addr_maps *maps = instance->pid_maps; + struct tracecmd_proc_addr_map *map; + unsigned long long begin, end; + struct pid_addr_maps *m; + char mapname[PATH_MAX+1]; + char fname[PATH_MAX+1]; + char buf[PATH_MAX+100]; + FILE *f; + int ret; + int res; + int i; + + sprintf(fname, "/proc/%d/exe", pid); + ret = readlink(fname, mapname, PATH_MAX); + if (ret >= PATH_MAX || ret < 0) + return -ENOENT; + mapname[ret] = 0; + + sprintf(fname, "/proc/%d/maps", pid); + f = fopen(fname, "r"); + if (!f) + return -ENOENT; + + while (maps) { + if (pid == maps->pid) + break; + maps = maps->next; + } + + ret = -ENOMEM; + if (!maps) { + maps = calloc(1, sizeof(*maps)); + if (!maps) + goto out_fail; + maps->pid = pid; + maps->next = instance->pid_maps; + instance->pid_maps = maps; + } else { + for (i = 0; i < maps->nr_lib_maps; i++) + free(maps->lib_maps[i].lib_name); + free(maps->lib_maps); + maps->lib_maps = NULL; + maps->nr_lib_maps = 0; + free(maps->proc_name); + } + + maps->proc_name = strdup(mapname); + if (!maps->proc_name) + goto out; + + while (fgets(buf, sizeof(buf), f)) { + mapname[0] = '\0'; + res = sscanf(buf, "%llx-%llx %*s %*x %*s %*d %"STRINGIFY(PATH_MAX)"s", + &begin, &end, mapname); + if (res == 3 && mapname[0] != '\0') { + map = realloc(maps->lib_maps, + (maps->nr_lib_maps + 1) * sizeof(*map)); + if (!map) + goto out_fail; + map[maps->nr_lib_maps].end = end; + map[maps->nr_lib_maps].start = begin; + map[maps->nr_lib_maps].lib_name = strdup(mapname); + if (!map[maps->nr_lib_maps].lib_name) + goto out_fail; + maps->lib_maps = map; + maps->nr_lib_maps++; + } + } +out: + fclose(f); + return 0; + +out_fail: + fclose(f); + if (maps) { + for (i = 0; i < maps->nr_lib_maps; i++) + free(maps->lib_maps[i].lib_name); + if (instance->pid_maps != maps) { + m = instance->pid_maps; + while (m) { + if (m->next == maps) { + m->next = maps->next; + break; + } + m = m->next; + } + } else + instance->pid_maps = maps->next; + free(maps->lib_maps); + maps->lib_maps = NULL; + maps->nr_lib_maps = 0; + free(maps->proc_name); + maps->proc_name = NULL; + free(maps); + } + return ret; +} + +static void get_filter_pid_maps(void) +{ + struct filter_pids *p; + + for (p = filter_pids; p; p = p->next) { + if (p->exclude) + continue; + get_pid_addr_maps(p->pid); + } +} + static void update_task_filter(void) { struct buffer_instance *instance; @@ -1076,6 +1193,9 @@ static void update_task_filter(void) if (no_filter) return; + if (get_procmap && filter_pids) + get_filter_pid_maps(); + if (filter_task) add_filter_pid(pid, 0); @@ -1287,6 +1407,8 @@ static void ptrace_wait(enum trace_type type) break; case PTRACE_EVENT_EXIT: + if (get_procmap) + get_pid_addr_maps(pid); ptrace(PTRACE_GETEVENTMSG, pid, NULL, &cstatus); ptrace(PTRACE_DETACH, pid, NULL, NULL); break; @@ -1363,6 +1485,7 @@ static void run_cmd(enum trace_type type, int argc, char **argv) } if (do_ptrace) { add_filter_pid(pid, 0); + ptrace_attach(pid); ptrace_wait(type); } else trace_waitpid(type, pid, &status, 0); @@ -3130,6 +3253,36 @@ static void append_buffer(struct tracecmd_output *handle, } } + +static void +add_pid_maps(struct tracecmd_output *handle, struct buffer_instance *instance) +{ + struct pid_addr_maps *maps = instance->pid_maps; + struct trace_seq s; + int i; + + trace_seq_init(&s); + while (maps) { + if (!maps->nr_lib_maps) { + maps = maps->next; + continue; + } + trace_seq_reset(&s); + trace_seq_printf(&s, "%x %x %s\n", + maps->pid, maps->nr_lib_maps, maps->proc_name); + for (i = 0; i < maps->nr_lib_maps; i++) + trace_seq_printf(&s, "%llx %llx %s\n", + maps->lib_maps[i].start, + maps->lib_maps[i].end, + maps->lib_maps[i].lib_name); + trace_seq_terminate(&s); + tracecmd_add_option(handle, TRACECMD_OPTION_PROCMAPS, + s.len + 1, s.buffer); + maps = maps->next; + } + trace_seq_destroy(&s); +} + static void add_buffer_stat(struct tracecmd_output *handle, struct buffer_instance *instance) { @@ -3323,6 +3476,10 @@ static void record_data(struct common_record_context *ctx) if (!no_top_instance() && !top_instance.msg_handle) print_stat(&top_instance); + for_all_instances(instance) { + add_pid_maps(handle, instance); + } + tracecmd_append_cpu_data(handle, local_cpu_count, temp_files); for (i = 0; i < max_cpu_count; i++) @@ -4433,6 +4590,7 @@ void update_first_instance(struct buffer_instance *instance, int topt) } enum { + OPT_procmap = 244, OPT_quiet = 245, OPT_debug = 246, OPT_no_filter = 247, @@ -4663,6 +4821,7 @@ static void parse_record_options(int argc, {"debug", no_argument, NULL, OPT_debug}, {"quiet", no_argument, NULL, OPT_quiet}, {"help", no_argument, NULL, '?'}, + {"proc-map", no_argument, NULL, OPT_procmap}, {"module", required_argument, NULL, OPT_module}, {NULL, 0, NULL, 0} }; @@ -4752,6 +4911,7 @@ static void parse_record_options(int argc, die("-c invalid: ptrace not supported"); #endif do_ptrace = 1; + do_children = 1; } else { save_option("event-fork"); ctx->do_child = 1; @@ -4894,6 +5054,9 @@ static void parse_record_options(int argc, case 'i': ignore_event_not_found = 1; break; + case OPT_procmap: + get_procmap = 1; + break; case OPT_date: ctx->date = 1; if (ctx->data_flags & DATA_FL_OFFSET) @@ -4960,7 +5123,7 @@ static void parse_record_options(int argc, add_func(&ctx->instance->filter_funcs, ctx->instance->filter_mod, "*"); - if (do_ptrace && !filter_task && !nr_filter_pids) + if (do_children && !filter_task && !nr_filter_pids) die(" -c can only be used with -F (or -P with event-fork support)"); if (ctx->do_child && !filter_task && !nr_filter_pids) die(" -c can only be used with -P or -F"); @@ -4974,6 +5137,13 @@ static void parse_record_options(int argc, "Did you mean 'record'?"); ctx->run_command = 1; } + + if (get_procmap) { + if (!ctx->run_command && !nr_filter_pids) + warning("--proc-map is ignored, no command or filtered PIDs are specified."); + else + do_ptrace = 1; + } } static enum trace_type get_trace_cmd_type(enum trace_cmd cmd) diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c index 406384c..7a67784 100644 --- a/tracecmd/trace-usage.c +++ b/tracecmd/trace-usage.c @@ -57,6 +57,7 @@ static struct usage_help usage_help[] = { " (use with caution)\n" " --max-graph-depth limit function_graph depth\n" " --no-filter include trace-cmd threads in the trace\n" + " --proc-map save the traced processes address map into the trace.dat file\n" }, { "start", -- 2.21.0
A new trace-cmd record option is added: "--user". When it is set with combination of option -F, the traced process is executed in the context of the specified user. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- [ v4 changes: - Added check for strdup() failure. - Made input user string argument of change_user() and run_cmd() constant. v3 changes: "--user" does not depend on option -F, it works with any command, specified as trace-cmd argument. v2 changes: - Check for errors in change_user(). If an error occurs while changing the user, the message is printed and the traced process is not executed. ] Documentation/trace-cmd-record.1.txt | 3 ++ tracecmd/trace-record.c | 49 ++++++++++++++++++++++++++-- tracecmd/trace-usage.c | 1 + 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt index e697f03..bb18e88 100644 --- a/Documentation/trace-cmd-record.1.txt +++ b/Documentation/trace-cmd-record.1.txt @@ -119,6 +119,9 @@ OPTIONS Used with either *-F* (or *-P* if kernel supports it) to trace the process' children too. +*--user*:: + Execute the specified *command* as given user. + *-C* 'clock':: Set the trace clock to "clock". diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index 17c676c..088aa28 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -33,6 +33,8 @@ #include <errno.h> #include <limits.h> #include <libgen.h> +#include <pwd.h> +#include <grp.h> #include "version.h" #include "trace-local.h" @@ -208,6 +210,7 @@ struct common_record_context { struct buffer_instance *instance; const char *output; char *date2ts; + char *user; int data_flags; int record_all; @@ -1455,7 +1458,34 @@ static void trace_or_sleep(enum trace_type type) sleep(10); } -static void run_cmd(enum trace_type type, int argc, char **argv) +static int change_user(const char *user) +{ + struct passwd *pwd; + + if (!user) + return 0; + + pwd = getpwnam(user); + if (!pwd) + return -1; + if (initgroups(user, pwd->pw_gid) < 0) + return -1; + if (setgid(pwd->pw_gid) < 0) + return -1; + if (setuid(pwd->pw_uid) < 0) + return -1; + + if (setenv("HOME", pwd->pw_dir, 1) < 0) + return -1; + if (setenv("USER", pwd->pw_name, 1) < 0) + return -1; + if (setenv("LOGNAME", pwd->pw_name, 1) < 0) + return -1; + + return 0; +} + +static void run_cmd(enum trace_type type, const char *user, int argc, char **argv) { int status; int pid; @@ -1476,6 +1506,10 @@ static void run_cmd(enum trace_type type, int argc, char **argv) dup2(save_stdout, 1); close(save_stdout); } + + if (change_user(user) < 0) + die("Failed to change user to %s", user); + if (execvp(argv[0], argv)) { fprintf(stderr, "\n********************\n"); fprintf(stderr, " Unable to exec %s\n", argv[0]); @@ -4590,6 +4624,7 @@ void update_first_instance(struct buffer_instance *instance, int topt) } enum { + OPT_user = 243, OPT_procmap = 244, OPT_quiet = 245, OPT_debug = 246, @@ -4822,6 +4857,7 @@ static void parse_record_options(int argc, {"quiet", no_argument, NULL, OPT_quiet}, {"help", no_argument, NULL, '?'}, {"proc-map", no_argument, NULL, OPT_procmap}, + {"user", required_argument, NULL, OPT_user}, {"module", required_argument, NULL, OPT_module}, {NULL, 0, NULL, 0} }; @@ -5054,6 +5090,11 @@ static void parse_record_options(int argc, case 'i': ignore_event_not_found = 1; break; + case OPT_user: + ctx->user = strdup(optarg); + if (!ctx->user) + die("Failed to allocate user name"); + break; case OPT_procmap: get_procmap = 1; break; @@ -5137,7 +5178,9 @@ static void parse_record_options(int argc, "Did you mean 'record'?"); ctx->run_command = 1; } - + if (ctx->user && !ctx->run_command) + warning("--user %s is ignored, no command is specified", + ctx->user); if (get_procmap) { if (!ctx->run_command && !nr_filter_pids) warning("--proc-map is ignored, no command or filtered PIDs are specified."); @@ -5285,7 +5328,7 @@ static void record_trace(int argc, char **argv, } if (ctx->run_command) - run_cmd(type, (argc - optind) - 1, &argv[optind + 1]); + run_cmd(type, ctx->user, (argc - optind) - 1, &argv[optind + 1]); else { update_task_filter(); tracecmd_enable_tracing(); diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c index 7a67784..20cb66f 100644 --- a/tracecmd/trace-usage.c +++ b/tracecmd/trace-usage.c @@ -58,6 +58,7 @@ static struct usage_help usage_help[] = { " --max-graph-depth limit function_graph depth\n" " --no-filter include trace-cmd threads in the trace\n" " --proc-map save the traced processes address map into the trace.dat file\n" + " --user execute the specified [command ...] as given user\n" }, { "start", -- 2.21.0
On Wed, 14 Aug 2019 11:47:09 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > New options to trace-cmd record are added: > - "--proc-map" - Saves traced process(es) address map in trace.dat > file. > - "--user" - Executes the traced process as given user. > > As "--proc-map" option uses trace-cmd ptrace logic, part of this code is > rewritten. There were some leftovers related to this logic, the code is > upadted. Hi Tzvetomir, For some reason, this patch series ended up as part of the: Separate trace-cmd and libtracecmd code series. -- Steve > > [ > v5 changes: > - Added new patch: > "Extend ptrace logic to work with multiple filtered pids" > It resolves "filter_pid" leftover in ptrace related logic. > - "--proc-map" does not depend on option -F, it works with any command, > specified as trace-cmd argument or option -P. > - Renamed "mmap" to "proc-map" - the option name and the names of > the functions, variables and defines related to this feature. > v4 changes: > - Added check for strdup() failure. > - Made input user string argument of change_user() and run_cmd() > constant. > - Added description of the new "--mmap" trace-cmd option in the > program's help and the man page. (Suggested by Slavomir Kaslev) > > Problems, reported by Yordan Karadzhov: > - Improved the parsing of /proc/<pid>/maps. Made it not so strict, as it > failed on some machines due to different size of fields. > - Implemented trace_pid_mmap_free() cleanup function to free mmap > related resources at trace-cmd exit. > - Fixed potential problem with non-terminated string, returned by > readlink(). > - Coding style fixes. > v3 changes: > - "--user" does not depend on option -F, it works with any command, > specified as trace-cmd argument. > - Changed tracecmd_search_task_mmap() API to return not only the library > name, but also the start and end memory addresses. > - Renamed *tracee* to *task* > - Improved resources cleanup, in case of an error. > - Removed (this) changelog from the commit message. > v2 changes: > - Check for errors in change_user(). If an error occurs while > changing the user, the message is printed and the traced > process is not executed. > - Replaced usage of tracecmd_add_option_v() with tracecmd_add_option() API. > - Added checks to prevent buffer overflow when sscanf (... "%s", buf) is used. > - Return error in case memory allocation fails. > - Return error if option string is not in the expected format. > - Sort memory maps and use binary search to find matching library in the map. > ] > > Tzvetomir Stoyanov (VMware) (3): > trace-cmd: Extend ptrace logic to work with multiple filtered pids > trace-cmd: Save the tracee address map into the trace.dat file. > trace-cmd: Add option to execute traced process as given user > > Documentation/trace-cmd-record.1.txt | 6 + > include/trace-cmd/trace-cmd.h | 10 + > lib/trace-cmd/trace-input.c | 172 +++++++++++++++- > tracecmd/include/trace-local.h | 10 + > tracecmd/trace-record.c | 282 +++++++++++++++++++++++++-- > tracecmd/trace-usage.c | 2 + > 6 files changed, 464 insertions(+), 18 deletions(-) >
On Wed, 14 Aug 2019 11:47:10 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > In the trace-record.c file there is a global variable named "filter_pid". > It is not set anywhere in the code, but there is a logic which relies on it. > This variable is a leftover from the past implementation of trace-cmd > record "-P" option, when it was designed to filter only a single PID. > Now "-P" option works with a list of PIDs, stored in filter_pids global > list. The code is modified to work with filter_pids instead of filter_pid. > This logic is used only if there is no ftrace "options/event-fork" on the > system and we have ptrace support. There is one significant change in > the trace-cmd record behavior in this specific use case: > - filtered pids are specified with the "-P" option. > - there is no ftrace "options/event-fork" on the system. > - there is ptrace support. > The trace continues until Ctrl^C is hit or all filtered PIDs exit, > whatever comes first. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > tracecmd/trace-record.c | 65 ++++++++++++++++++++++++++++++++--------- > 1 file changed, 52 insertions(+), 13 deletions(-) > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index 5dc6f17..e0fa07d 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -86,7 +86,6 @@ static bool use_tcp; > static int do_ptrace; > > static int filter_task; > -static int filter_pid = -1; > static bool no_filter = false; > > static int local_cpu_count; > @@ -866,6 +865,13 @@ static void add_filter_pid(int pid, int exclude) > struct filter_pids *p; > char buf[100]; > > + for (p = filter_pids; p; p = p->next) { > + if (p->pid == pid) { > + p->exclude = exclude; > + return; > + } > + } > + > p = malloc(sizeof(*p)); > if (!p) > die("Failed to allocate pid filter"); > @@ -1223,17 +1229,34 @@ static void enable_ptrace(void) > ptrace(PTRACE_TRACEME, 0, NULL, 0); > } > > -static void ptrace_wait(enum trace_type type, int main_pid) > +static void ptrace_wait(enum trace_type type) > { > + struct filter_pids *fpid; > unsigned long send_sig; > unsigned long child; > siginfo_t sig; > + int main_pids; > int cstatus; > int status; > + int i = 0; > + int *pids; > int event; > int pid; > int ret; > > + pids = calloc(nr_filter_pids, sizeof(int)); > + if (!pids) Probably at the minimum, we should add a warning here that it didn't get allocated. > + return; > + > + for (fpid = filter_pids; fpid; fpid = fpid->next) { > + if (fpid->exclude) > + continue; > + pids[i++] = fpid->pid; > + if (i >= nr_filter_pids) > + break; > + } > + main_pids = i; > + > do { > ret = trace_waitpid(type, -1, &status, WSTOPPED | __WALL); > if (ret < 0) > @@ -1275,11 +1298,24 @@ static void ptrace_wait(enum trace_type type, int main_pid) > PTRACE_O_TRACEEXIT); > ptrace(PTRACE_CONT, pid, NULL, send_sig); > } > - } while (!finished && ret > 0 && > - (!WIFEXITED(status) || pid != main_pid)); > + if (WIFEXITED(status) || > + (WIFSTOPPED(status) && event == PTRACE_EVENT_EXIT)) { > + for (i = 0; i < nr_filter_pids; i++) { > + if (pid == pids[i]) { > + pids[i] = 0; > + main_pids--; > + if (!main_pids) > + finished = 1; > + break; > + } > + } > + } > + } while (!finished && ret > 0); > + > + free(pids); > } > #else > -static inline void ptrace_wait(enum trace_type type, int main_pid) { } > +static inline void ptrace_wait(enum trace_type type) { } > static inline void enable_ptrace(void) { } > static inline void ptrace_attach(int pid) { } > > @@ -1289,8 +1325,8 @@ static void trace_or_sleep(enum trace_type type) > { > struct timeval tv = { 1 , 0 }; > > - if (do_ptrace && filter_pid >= 0) > - ptrace_wait(type, filter_pid); > + if (do_ptrace && filter_pids) > + ptrace_wait(type); > else if (type & TRACE_TYPE_STREAM) > trace_stream_read(pids, recorder_threads, &tv); > else > @@ -1327,7 +1363,7 @@ static void run_cmd(enum trace_type type, int argc, char **argv) > } > if (do_ptrace) { > add_filter_pid(pid, 0); > - ptrace_wait(type, pid); > + ptrace_wait(type); > } else > trace_waitpid(type, pid, &status, 0); > } > @@ -2318,7 +2354,7 @@ create_event(struct buffer_instance *instance, char *path, struct event_list *ol > *event = *old_event; > add_event(instance, event); > > - if (event->filter || filter_task || filter_pid) { > + if (event->filter || filter_task || filter_pids) { > event->filter_file = strdup(path); > if (!event->filter_file) > die("malloc filter file"); > @@ -4924,9 +4960,9 @@ static void parse_record_options(int argc, > add_func(&ctx->instance->filter_funcs, > ctx->instance->filter_mod, "*"); > > - if (do_ptrace && !filter_task && (filter_pid < 0)) > + if (do_ptrace && !filter_task && !nr_filter_pids) > die(" -c can only be used with -F (or -P with event-fork support)"); > - if (ctx->do_child && !filter_task &&! filter_pid) > + if (ctx->do_child && !filter_task && !nr_filter_pids) > die(" -c can only be used with -P or -F"); > > if ((argc - optind) >= 2) { > @@ -4997,6 +5033,7 @@ static void record_trace(int argc, char **argv, > { > enum trace_type type = get_trace_cmd_type(ctx->curr_cmd); > struct buffer_instance *instance; > + struct filter_pids *pid; > > /* > * If top_instance doesn't have any plugins or events, then > @@ -5083,8 +5120,10 @@ static void record_trace(int argc, char **argv, > update_task_filter(); > tracecmd_enable_tracing(); > /* We don't ptrace ourself */ > - if (do_ptrace && filter_pid >= 0) > - ptrace_attach(filter_pid); > + if (do_ptrace && filter_pids) > + for (pid = filter_pids; pid; pid = pid->next) > + if (!pid->exclude) > + ptrace_attach(pid->pid); Just a nit, the above should have brackets. Leaving off brackets is fine for non complex blocks. That is, only the internal if should have no brackets: if (do_ptrace && filter_pids) { for (pid = filter_pids; pid; pid = pid->next) { if (!pid->exclude) ptrace_attach(pid->pid); } } Otherwise mistakes can easily be made. -- Steve > /* sleep till we are woken with Ctrl^C */ > printf("Hit Ctrl^C to stop recording\n"); > while (!finished)
On Wed, 14 Aug 2019 11:47:04 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> void tracecmd_set_quiet(int quiet);
> int tracecmd_get_quiet(void);
I rather make this a parameter to the descriptor:
tracecmd_set_quiet(struct tracecmd_output *handle, bool quiet);
tracecmd_get_quiet(struct tracecmd output *handle);
As we may have multiple handles and perhaps we don't want all of them
quiet.
-- Steve
On Wed, 14 Aug 2019 11:47:05 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> A trace-cmd global variable "debug" is used from libtracecmd and
> should be defined there. A new library APIs are implemented to
> access it:
> void tracecmd_set_debug(bool debug);
> bool tracecmd_get_debug(void);
>
I was thinking of doing a descriptor for this too, but screw it. Debug
is debug ;-)
-- Steve
On Wed, 14 Aug 2019 11:47:06 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > plog() function writes logs into a log file. It is used in > libtracecmd and its implementation should be there. > The function is moved from trace-cmd into the library, and 2 > additional APIs are implemented: > int trace_set_log_file(char *logfile); - use it to set > the log file. > void plog_error(const char *fmt, ...); - use it to log > an error message into the file. > > The plog() function is used also from pdie() in trace-cmd. > pdie() depends on trace-cmd context and cannot be moved to > the library. It is reimplemented as macros, in order to utilize > the new plog() library function. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > include/trace-cmd/trace-cmd.h | 4 ++ > include/trace-cmd/trace-msg.h | 3 -- > lib/trace-cmd/trace-util.c | 71 +++++++++++++++++++++++++++++++++++ > tracecmd/trace-listen.c | 69 ++++------------------------------ > 4 files changed, 83 insertions(+), 64 deletions(-) > > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h > index b96de04..8db0686 100644 > --- a/include/trace-cmd/trace-cmd.h > +++ b/include/trace-cmd/trace-cmd.h > @@ -398,6 +398,10 @@ struct hook_list { > struct hook_list *tracecmd_create_event_hook(const char *arg); > void tracecmd_free_hooks(struct hook_list *hooks); > > +void plog(const char *fmt, ...); > +void plog_error(const char *fmt, ...); If these are going to become visible in the library, then they need to have a prefix "tracecmd_" attached to them. > +int trace_set_log_file(char *logfile); > + > /* --- Hack! --- */ > int tracecmd_blk_hack(struct tracecmd_input *handle); > > diff --git a/include/trace-cmd/trace-msg.h b/include/trace-cmd/trace-msg.h > index b7fe10b..aab8a69 100644 > --- a/include/trace-cmd/trace-msg.h > +++ b/include/trace-cmd/trace-msg.h > @@ -12,7 +12,4 @@ > > extern unsigned int page_size; > > -void plog(const char *fmt, ...); > -void pdie(const char *fmt, ...); > - > #endif /* _TRACE_MSG_H_ */ > diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c > index b5ce84f..8c1a0a0 100644 > --- a/lib/trace-cmd/trace-util.c > +++ b/lib/trace-cmd/trace-util.c > @@ -31,6 +31,8 @@ int tracecmd_disable_plugins; > static int tracecmd_quiet; > static bool tracecmd_debug; > > +static FILE *trace_logfp; As this is a static variable, we only need to call it logfp. > + > static struct registered_plugin_options { > struct registered_plugin_options *next; > struct tep_plugin_option *options; > @@ -1716,3 +1718,72 @@ void __weak *malloc_or_die(unsigned int size) > die("malloc"); > return data; > } > + > +#define LOG_BUF_SIZE 1024 > +static void __plog(const char *prefix, const char *fmt, va_list ap, FILE *fp) > +{ > + static int newline = 1; > + char buf[LOG_BUF_SIZE]; > + int r; > + > + r = vsnprintf(buf, LOG_BUF_SIZE, fmt, ap); > + > + if (r > LOG_BUF_SIZE) > + r = LOG_BUF_SIZE; > + > + if (trace_logfp) { > + if (newline) > + fprintf(trace_logfp, "[%d]%s%.*s", getpid(), prefix, r, buf); > + else > + fprintf(trace_logfp, "[%d]%s%.*s", getpid(), prefix, r, buf); > + newline = buf[r - 1] == '\n'; > + fflush(trace_logfp); > + return; > + } > + > + fprintf(fp, "%.*s", r, buf); > +} > + > +void plog(const char *fmt, ...) > +{ > + va_list ap; > + > + va_start(ap, fmt); > + __plog("", fmt, ap, stdout); > + va_end(ap); > + /* Make sure it gets to the screen, in case we crash afterward */ > + fflush(stdout); > +} > + > +void plog_error(const char *fmt, ...) > +{ > + va_list ap; > + char *str = ""; > + > + va_start(ap, fmt); > + __plog("Error: ", fmt, ap, stderr); > + va_end(ap); > + if (errno) > + str = strerror(errno); > + if (trace_logfp) > + fprintf(trace_logfp, "\n%s\n", str); > + else > + fprintf(stderr, "\n%s\n", str); > +} > + > +/** > + * trace_set_log_file - Set file for logging > + * @logfile: Name of the log file > + * > + * Returns 0 on successful completion or -1 in case of error > + */ > +int trace_set_log_file(char *logfile) Should it be called tracecmd_set_logfile()? -- Steve > +{ > + if (trace_logfp) > + fclose(trace_logfp); > + trace_logfp = fopen(logfile, "w"); > + if (!trace_logfp) > + return -1; > + return 0; > +} > +
On Wed, 14 Aug 2019 11:47:07 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > These APIs depend on trace-cmd context and cannot be used standalone: > tracecmd_create_top_instance() > tracecmd_remove_instances() > tracecmd_filter_pid() > tracecmd_add_event() > tracecmd_enable_events() > tracecmd_disable_all_tracing() > tracecmd_disable_tracing() > tracecmd_enable_tracing() > tracecmd_stat_cpu() > They are implemented in trace-cmd application, but declared as APIs in > trace-cmd.h. The declarations are moved from trace-cmd.h to trace-local.h, > local header file visible only to trace-cmd application. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > include/trace-cmd/trace-cmd.h | 9 --------- > tracecmd/include/trace-local.h | 11 +++++++++++ > 2 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h > index 8db0686..2c8d798 100644 > --- a/include/trace-cmd/trace-cmd.h > +++ b/include/trace-cmd/trace-cmd.h > @@ -137,8 +137,6 @@ int tracecmd_buffer_instances(struct tracecmd_input *handle); > const char *tracecmd_buffer_instance_name(struct tracecmd_input *handle, int indx); > struct tracecmd_input *tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx); > int tracecmd_is_buffer_instance(struct tracecmd_input *handle); > -void tracecmd_create_top_instance(char *name); > -void tracecmd_remove_instances(void); > > void tracecmd_set_ts_offset(struct tracecmd_input *handle, unsigned long long offset); > void tracecmd_set_ts2secs(struct tracecmd_input *handle, unsigned long long hz); > @@ -304,14 +302,7 @@ struct tracecmd_recorder *tracecmd_create_buffer_recorder_maxkb(const char *file > > int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long sleep); > void tracecmd_stop_recording(struct tracecmd_recorder *recorder); > -void tracecmd_stat_cpu(struct trace_seq *s, int cpu); > long tracecmd_flush_recording(struct tracecmd_recorder *recorder); > -void tracecmd_filter_pid(int pid, int exclude); > -int tracecmd_add_event(const char *event_str, int stack); > -void tracecmd_enable_events(void); > -void tracecmd_disable_all_tracing(int disable_tracer); > -void tracecmd_disable_tracing(void); > -void tracecmd_enable_tracing(void); > > enum tracecmd_msg_flags { > TRACECMD_MSG_FL_USE_TCP = 1 << 0, > diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h > index e2d5506..05760d8 100644 > --- a/tracecmd/include/trace-local.h > +++ b/tracecmd/include/trace-local.h > @@ -216,6 +216,17 @@ void show_instance_file(struct buffer_instance *instance, const char *name); > > int count_cpus(void); > > +/* moved from trace-cmd.h */ > +void tracecmd_create_top_instance(char *name); > +void tracecmd_remove_instances(void); > +void tracecmd_filter_pid(int pid, int exclude); > +int tracecmd_add_event(const char *event_str, int stack); > +void tracecmd_enable_events(void); > +void tracecmd_disable_all_tracing(int disable_tracer); > +void tracecmd_disable_tracing(void); > +void tracecmd_enable_tracing(void); > +void tracecmd_stat_cpu(struct trace_seq *s, int cpu); > + I'll take this patch, but I wonder if we should change "tracecmd_" to "trace_" to let it be known that these are local functions. That can be a later patch. -- Steve > /* No longer in event-utils.h */ > void __noreturn die(const char *fmt, ...); /* Can be overriden */ > void *malloc_or_die(unsigned int size); /* Can be overridden */
On Wed, 14 Aug 2019 11:47:08 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > tracecmd_stack_tracer_status() function reads the stack tracer status > from the proc file system. It does not depend on trace-cmd context and > can be used standalone. The function is moved from trace-cmd application > into libtracecmd. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > lib/trace-cmd/trace-util.c | 49 +++++++++++++++++++++++++++++++++ > tracecmd/trace-stack.c | 56 ++------------------------------------ > 2 files changed, 51 insertions(+), 54 deletions(-) > > diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c > index 8c1a0a0..d16a018 100644 > --- a/lib/trace-cmd/trace-util.c > +++ b/lib/trace-cmd/trace-util.c > @@ -25,6 +25,7 @@ > #define LOCAL_PLUGIN_DIR ".trace-cmd/plugins" > #define TRACEFS_PATH "/sys/kernel/tracing" > #define DEBUGFS_PATH "/sys/kernel/debug" > +#define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled" > > int tracecmd_disable_sys_plugins; > int tracecmd_disable_plugins; > @@ -1787,3 +1788,51 @@ int trace_set_log_file(char *logfile) > return 0; > } > > +/** > + * tracecmd_stack_tracer_status - Check stack trace status > + * @status: Returned stack trace status: > + * 0 - not configured, disabled > + * non 0 - enabled > + * > + * Returns -1 in case of an error, 0 if file does not exist > + * (stack tracer not enabled) or 1 on successful completion. "(stack tracer not configured in kernel)" Saying "enabled" is ambiguous. > + */ > +int tracecmd_stack_tracer_status(int *status) If we are making this a library function, shouldn't it be declared in the trace-cmd.h file? -- Steve
On Wed, 14 Aug 2019 11:47:00 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> libtracecmd is a library, containing functions that can be
> used without the trace-cmd application. However, some of the
> functions declared as libtracecmd APIs in trace-cmd.h
> depend on trace-cmd context. That causes a problem when
> other application uses the library. The problem can be
> observed when running kerneshark and there is a python
> module, loaded by the python plugin - there is a bunch
> of warnings.
> To resolve the problem, implementations of all trace-cmd
> independent functions are moved into libtracecmd. All
> libtracecmd functions, that depend on trace-cmd context
> are removed from the library and from trace-cmd.h file.
>
I pulled in some but not all of these patches (the ones I had comments
on). Feel free to rebase on the git repo and you only need to send the
ones that have not been applied.
-- Steve
On Wed, Aug 28, 2019 at 10:59 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Wed, 14 Aug 2019 11:47:04 +0300 > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > > void tracecmd_set_quiet(int quiet); > > int tracecmd_get_quiet(void); > > I rather make this a parameter to the descriptor: > > tracecmd_set_quiet(struct tracecmd_output *handle, bool quiet); > tracecmd_get_quiet(struct tracecmd output *handle); > > As we may have multiple handles and perhaps we don't want all of them > quiet. > It makes sense to have "quiet" per tracecmd_output handler, but when I started to modify the code, I noticed one flow where tracecmd_get_quiet() is used and there is no tracecmd_output handler available - in check_plugin(). We have either to drop the usage of tracecmd_get_quiet() in check_plugin(), or leave the scope of "quiet" to be for the whole library. > -- Steve -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center
On Thu, 29 Aug 2019 14:39:43 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> On Wed, Aug 28, 2019 at 10:59 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Wed, 14 Aug 2019 11:47:04 +0300
> > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> >
> > > void tracecmd_set_quiet(int quiet);
> > > int tracecmd_get_quiet(void);
> >
> > I rather make this a parameter to the descriptor:
> >
> > tracecmd_set_quiet(struct tracecmd_output *handle, bool quiet);
> > tracecmd_get_quiet(struct tracecmd output *handle);
> >
> > As we may have multiple handles and perhaps we don't want all of them
> > quiet.
> >
> It makes sense to have "quiet" per tracecmd_output handler, but when I
> started to modify the code,
> I noticed one flow where tracecmd_get_quiet() is used and there is no
> tracecmd_output handler available -
> in check_plugin(). We have either to drop the usage of
> tracecmd_get_quiet() in check_plugin(), or leave the scope of
> "quiet" to be for the whole library.
We can still have a global variable in trace-record.c called quiet. And
that gets set by the parameter:
case OPT_quite:
case 'q':
quiet = 1;
break;
[..]
tracecmd_set_quiet(handle, quiet);
This is the proper way to handle it. The functions local to
trace-record.c, can just use its own quiet state variable, but when we
set quiet, we use the tracecmd_set_quiet() to notify the library that
it too should be quiet.
-- Steve
Hi Steven, On Wed, Aug 28, 2019 at 11:21 PM Steven Rostedt <rostedt@goodmis.org> wrote: > ... > > +/** > > + * tracecmd_stack_tracer_status - Check stack trace status > > + * @status: Returned stack trace status: > > + * 0 - not configured, disabled > > + * non 0 - enabled > > + * > > + * Returns -1 in case of an error, 0 if file does not exist > > + * (stack tracer not enabled) or 1 on successful completion. > > "(stack tracer not configured in kernel)" > > Saying "enabled" is ambiguous. > > > + */ > > +int tracecmd_stack_tracer_status(int *status) > > If we are making this a library function, shouldn't it be declared in > the trace-cmd.h file? > It is already part of trace-cmd.h, that was one of the problems - it was defined as library API, but implemented in the trace-cmd application. This patch moves the implementation in the library, the declaration in trace-cmd.h remains the same. > -- Steve -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center