* [PATCH v2 1/8] trace-cmd: Move trace-cmd-local.h from the application to the library
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-14 8:47 ` [PATCH v2 2/8] trace-cmd: Move trace-output.c into the library code Tzvetomir Stoyanov (VMware)
` (11 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 2/8] trace-cmd: Move trace-output.c into the library code
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
2019-08-14 8:47 ` [PATCH v2 1/8] trace-cmd: Move trace-cmd-local.h from the application to the library Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-14 8:47 ` [PATCH v2 3/8] trace-cmd: Move trace-msg.c into the library Tzvetomir Stoyanov (VMware)
` (10 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 3/8] trace-cmd: Move trace-msg.c into the library.
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
2019-08-14 8:47 ` [PATCH v2 1/8] trace-cmd: Move trace-cmd-local.h from the application to the library Tzvetomir Stoyanov (VMware)
2019-08-14 8:47 ` [PATCH v2 2/8] trace-cmd: Move trace-output.c into the library code Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-14 8:47 ` [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd Tzvetomir Stoyanov (VMware)
` (9 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (2 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v2 3/8] trace-cmd: Move trace-msg.c into the library Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-28 19:59 ` Steven Rostedt
2019-08-14 8:47 ` [PATCH v2 5/8] trace-cmd: Move trace-cmd global variable "debug" " Tzvetomir Stoyanov (VMware)
` (8 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd
2019-08-14 8:47 ` [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd Tzvetomir Stoyanov (VMware)
@ 2019-08-28 19:59 ` Steven Rostedt
2019-08-29 11:39 ` Tzvetomir Stoyanov
0 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2019-08-28 19:59 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd
2019-08-28 19:59 ` Steven Rostedt
@ 2019-08-29 11:39 ` Tzvetomir Stoyanov
2019-08-29 16:38 ` Steven Rostedt
0 siblings, 1 reply; 24+ messages in thread
From: Tzvetomir Stoyanov @ 2019-08-29 11:39 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd
2019-08-29 11:39 ` Tzvetomir Stoyanov
@ 2019-08-29 16:38 ` Steven Rostedt
0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-08-29 16:38 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 5/8] trace-cmd: Move trace-cmd global variable "debug" to libtracecmd
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (3 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v2 4/8] trace-cmd: Move trace-cmd global variable "quiet" to libtracecmd Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-28 20:01 ` Steven Rostedt
2019-08-14 8:47 ` [PATCH v2 6/8] trace-cmd: Move plog() function " Tzvetomir Stoyanov (VMware)
` (7 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 5/8] trace-cmd: Move trace-cmd global variable "debug" to libtracecmd
2019-08-14 8:47 ` [PATCH v2 5/8] trace-cmd: Move trace-cmd global variable "debug" " Tzvetomir Stoyanov (VMware)
@ 2019-08-28 20:01 ` Steven Rostedt
0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-08-28 20:01 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 6/8] trace-cmd: Move plog() function to libtracecmd.
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (4 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v2 5/8] trace-cmd: Move trace-cmd global variable "debug" " Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-28 20:15 ` Steven Rostedt
2019-08-14 8:47 ` [PATCH v2 7/8] trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h Tzvetomir Stoyanov (VMware)
` (6 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/8] trace-cmd: Move plog() function to libtracecmd.
2019-08-14 8:47 ` [PATCH v2 6/8] trace-cmd: Move plog() function " Tzvetomir Stoyanov (VMware)
@ 2019-08-28 20:15 ` Steven Rostedt
0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-08-28 20:15 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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;
> +}
> +
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 7/8] trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (5 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v2 6/8] trace-cmd: Move plog() function " Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-28 20:17 ` Steven Rostedt
2019-08-14 8:47 ` [PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd Tzvetomir Stoyanov (VMware)
` (5 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 7/8] trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h
2019-08-14 8:47 ` [PATCH v2 7/8] trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h Tzvetomir Stoyanov (VMware)
@ 2019-08-28 20:17 ` Steven Rostedt
0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-08-28 20:17 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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 */
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (6 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v2 7/8] trace-cmd: Move trace-cmd APIs from trace-cmd.h to trace-local.h Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-28 20:21 ` Steven Rostedt
2019-08-14 8:47 ` [PATCH v5 0/3] Add new trace-cmd record options: "--proc-map" Tzvetomir Stoyanov (VMware)
` (4 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd
2019-08-14 8:47 ` [PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd Tzvetomir Stoyanov (VMware)
@ 2019-08-28 20:21 ` Steven Rostedt
2019-09-03 12:24 ` Tzvetomir Stoyanov
0 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2019-08-28 20:21 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd
2019-08-28 20:21 ` Steven Rostedt
@ 2019-09-03 12:24 ` Tzvetomir Stoyanov
0 siblings, 0 replies; 24+ messages in thread
From: Tzvetomir Stoyanov @ 2019-09-03 12:24 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 0/3] Add new trace-cmd record options: "--proc-map"
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (7 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v2 8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-27 23:28 ` Steven Rostedt
2019-08-14 8:47 ` [PATCH v5 1/3] trace-cmd: Extend ptrace logic to work with multiple filtered pids Tzvetomir Stoyanov (VMware)
` (3 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 0/3] Add new trace-cmd record options: "--proc-map"
2019-08-14 8:47 ` [PATCH v5 0/3] Add new trace-cmd record options: "--proc-map" Tzvetomir Stoyanov (VMware)
@ 2019-08-27 23:28 ` Steven Rostedt
0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-08-27 23:28 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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(-)
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 1/3] trace-cmd: Extend ptrace logic to work with multiple filtered pids
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (8 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v5 0/3] Add new trace-cmd record options: "--proc-map" Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-27 23:31 ` Steven Rostedt
2019-08-14 8:47 ` [PATCH v5 2/3] trace-cmd: Save the tracee address map into the trace.dat file Tzvetomir Stoyanov (VMware)
` (2 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/3] trace-cmd: Extend ptrace logic to work with multiple filtered pids
2019-08-14 8:47 ` [PATCH v5 1/3] trace-cmd: Extend ptrace logic to work with multiple filtered pids Tzvetomir Stoyanov (VMware)
@ 2019-08-27 23:31 ` Steven Rostedt
0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-08-27 23:31 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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)
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 2/3] trace-cmd: Save the tracee address map into the trace.dat file.
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (9 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v5 1/3] trace-cmd: Extend ptrace logic to work with multiple filtered pids Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-14 8:47 ` [PATCH v5 3/3] trace-cmd: Add option to execute traced process as given user Tzvetomir Stoyanov (VMware)
2019-08-28 20:25 ` [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Steven Rostedt
12 siblings, 0 replies; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 3/3] trace-cmd: Add option to execute traced process as given user
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (10 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v5 2/3] trace-cmd: Save the tracee address map into the trace.dat file Tzvetomir Stoyanov (VMware)
@ 2019-08-14 8:47 ` Tzvetomir Stoyanov (VMware)
2019-08-28 20:25 ` [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Steven Rostedt
12 siblings, 0 replies; 24+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2019-08-14 8:47 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/8] Separate trace-cmd and libtracecmd code
2019-08-14 8:47 [PATCH v2 0/8] Separate trace-cmd and libtracecmd code Tzvetomir Stoyanov (VMware)
` (11 preceding siblings ...)
2019-08-14 8:47 ` [PATCH v5 3/3] trace-cmd: Add option to execute traced process as given user Tzvetomir Stoyanov (VMware)
@ 2019-08-28 20:25 ` Steven Rostedt
12 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2019-08-28 20:25 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread