Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/4] Few small trace-cmd fixes
@ 2020-04-30 12:22 Tzvetomir Stoyanov (VMware)
  2020-04-30 12:22 ` [PATCH 1/4] trace-cmd: Fix trace-cmd report crash while displaying trace.dat in specific use case Tzvetomir Stoyanov (VMware)
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-30 12:22 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

A couple of trace-cmd bugfixes, good to have in the next trace-cmd
release.

Tzvetomir Stoyanov (VMware) (4):
  trace-cmd: Fix trace-cmd report crash while displaying trace.dat in
    specific use case
  trace-cmd: Add "main" in the output of trace-cmd stat when displaying
    main instance
  trace-cmd: Create ftrace instances before using them.
  trace-cmd: Do not try to update parent's memory from a fork()-ed child

 lib/trace-cmd/trace-input.c |  2 ++
 tracecmd/trace-record.c     | 38 ++++++++++++++++++++++++++++++-------
 tracecmd/trace-stat.c       |  4 +++-
 3 files changed, 36 insertions(+), 8 deletions(-)

-- 
2.25.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/4] trace-cmd: Fix trace-cmd report crash while displaying trace.dat in specific use case
  2020-04-30 12:22 [PATCH 0/4] Few small trace-cmd fixes Tzvetomir Stoyanov (VMware)
@ 2020-04-30 12:22 ` Tzvetomir Stoyanov (VMware)
  2020-04-30 12:22 ` [PATCH 2/4] trace-cmd: Add "main" in the output of trace-cmd stat when displaying main instance Tzvetomir Stoyanov (VMware)
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-30 12:22 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

The trace-cmd report command crashes while displaying a file recorded with "--proc-map" and "-B" options:
#trace-cmd record --proc-map  -B test -e sched -F sleep 1
The "--proc-map" options saves the address map of "sleep" into the trace.dat file. This
information is used by KernelShark. The "-B" options traces the specified events into a
ftrace instance "test".
When such file is opened using libtracecmd APIs, the proc-map is parsed and saved into
a tracecmd_input handler, as linked list "pid_maps". Later, when the ftrace instance
"test" is parsed, a copy of this handler is used to fill it with the instance's trace data.
Both tracecmd_input handlers share the same "pid_maps" list, thus leads to a double
free of the list, when  handlers are destroyed.
As this "pid_maps" is not used in ftrace buffers, the "pid_maps" list of the copy can be
initialized to NULL.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-input.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 55c3d80a..7583d5cb 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3712,6 +3712,8 @@ tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx)
 
 	new_handle->flags |= TRACECMD_FL_BUFFER_INSTANCE;
 
+	new_handle->pid_maps = NULL;
+
 	/* Save where we currently are */
 	offset = lseek64(handle->fd, 0, SEEK_CUR);
 
-- 
2.25.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/4] trace-cmd: Add "main" in the output of trace-cmd stat when displaying main instance
  2020-04-30 12:22 [PATCH 0/4] Few small trace-cmd fixes Tzvetomir Stoyanov (VMware)
  2020-04-30 12:22 ` [PATCH 1/4] trace-cmd: Fix trace-cmd report crash while displaying trace.dat in specific use case Tzvetomir Stoyanov (VMware)
@ 2020-04-30 12:22 ` Tzvetomir Stoyanov (VMware)
  2020-04-30 12:22 ` [PATCH 3/4] trace-cmd: Create ftrace instances before using them Tzvetomir Stoyanov (VMware)
  2020-04-30 12:22 ` [PATCH 4/4] trace-cmd: Do not try to update parent's memory from a fork()-ed child Tzvetomir Stoyanov (VMware)
  3 siblings, 0 replies; 5+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-30 12:22 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

By default, "trace-cmd stat" command prints status of the main ftrace instance. When more
instances are configured, the user could be confused which status is displayed.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tracecmd/trace-stat.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tracecmd/trace-stat.c b/tracecmd/trace-stat.c
index c5057978..dcf2789c 100644
--- a/tracecmd/trace-stat.c
+++ b/tracecmd/trace-stat.c
@@ -920,8 +920,10 @@ static void stat_instance(struct buffer_instance *instance)
 			printf("---------------\n");
 		printf("Instance: %s\n",
 			tracefs_instance_get_name(instance->tracefs));
-	} else
+	} else {
 		report_instances();
+		printf("\nInstance: main\n\n");
+	}
 
 	report_plugin(instance);
 	report_events(instance);
-- 
2.25.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/4] trace-cmd: Create ftrace instances before using them.
  2020-04-30 12:22 [PATCH 0/4] Few small trace-cmd fixes Tzvetomir Stoyanov (VMware)
  2020-04-30 12:22 ` [PATCH 1/4] trace-cmd: Fix trace-cmd report crash while displaying trace.dat in specific use case Tzvetomir Stoyanov (VMware)
  2020-04-30 12:22 ` [PATCH 2/4] trace-cmd: Add "main" in the output of trace-cmd stat when displaying main instance Tzvetomir Stoyanov (VMware)
@ 2020-04-30 12:22 ` Tzvetomir Stoyanov (VMware)
  2020-04-30 12:22 ` [PATCH 4/4] trace-cmd: Do not try to update parent's memory from a fork()-ed child Tzvetomir Stoyanov (VMware)
  3 siblings, 0 replies; 5+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-30 12:22 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Ftrace instances should be created physically in the tracefs, before
trying to access its files. In record_trace() function, the call to
make_instances() should be before the logic which reads the instance's
tracing file.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tracecmd/trace-record.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index d8c24ebf..1e4d38fa 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -6222,6 +6222,8 @@ static void record_trace(int argc, char **argv,
 	if (!ctx->output)
 		ctx->output = DEFAULT_INPUT_FILE;
 
+	make_instances();
+
 	/* Save the state of tracing_on before starting */
 	for_all_instances(instance) {
 		instance->output_file = strdup(ctx->output);
@@ -6236,8 +6238,6 @@ static void record_trace(int argc, char **argv,
 			instance->tracing_on_init_val = 1;
 	}
 
-	make_instances();
-
 	if (ctx->events)
 		expand_event_list();
 
-- 
2.25.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 4/4] trace-cmd: Do not try to update parent's memory from a fork()-ed child
  2020-04-30 12:22 [PATCH 0/4] Few small trace-cmd fixes Tzvetomir Stoyanov (VMware)
                   ` (2 preceding siblings ...)
  2020-04-30 12:22 ` [PATCH 3/4] trace-cmd: Create ftrace instances before using them Tzvetomir Stoyanov (VMware)
@ 2020-04-30 12:22 ` Tzvetomir Stoyanov (VMware)
  3 siblings, 0 replies; 5+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-30 12:22 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

When trace-cmd runs a command, specified with the "-F" flag, it forks a
child process and executes the command in its context. This child process
receives a full copy of the parents memory at the moment of fork(). When
it modifies this copy, the parent memory is not affected. Calling the
function update_task_filter() in the child context will operate on a valid
data, but will not update anything in the parent's databases.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tracecmd/trace-record.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 1e4d38fa..2b5cd42a 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -11,6 +11,7 @@
 #include <stdarg.h>
 #include <getopt.h>
 #include <time.h>
+#include <semaphore.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/time.h>
@@ -1487,18 +1488,35 @@ static int change_user(const char *user)
 	return 0;
 }
 
+#define TRACE_RUN_SEM	"trace_cmd_semaphore_run"
+#define TRACE_INIT_SEM	"trace_cmd_semaphore_init"
 static void run_cmd(enum trace_type type, const char *user, int argc, char **argv)
 {
+	sem_t *sem_init;
+	sem_t *sem_run;
 	int status;
 	int pid;
 
+	sem_init = sem_open(TRACE_INIT_SEM, O_CREAT, S_IRUSR|S_IWUSR, 0);
+	if (sem_init == SEM_FAILED)
+		die("failed to init trace_cmd init semaphore");
+
+	sem_run = sem_open(TRACE_RUN_SEM, O_CREAT, S_IRUSR|S_IWUSR, 0);
+	if (sem_run == SEM_FAILED) {
+		sem_close(sem_init);
+		sem_unlink(TRACE_INIT_SEM);
+		die("failed to init trace_cmd run semaphore");
+	}
+
 	if ((pid = fork()) < 0)
 		die("failed to fork");
 	if (!pid) {
 		/* child */
-		update_task_filter();
+		sem_wait(sem_init);
 		tracecmd_enable_tracing();
 		enable_ptrace();
+		sem_post(sem_run);
+
 		/*
 		 * If we are using stderr for stdout, switch
 		 * it back to the saved stdout for the code we run.
@@ -1519,11 +1537,17 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 			die("Failed to exec %s", argv[0]);
 		}
 	}
-	if (do_ptrace) {
-		add_filter_pid(pid, 0);
-		ptrace_attach(pid);
+	update_task_filter();
+	sem_post(sem_init);
+	sem_wait(sem_run);
+	sem_close(sem_init);
+	sem_unlink(TRACE_INIT_SEM);
+	sem_close(sem_run);
+	sem_unlink(TRACE_RUN_SEM);
+
+	if (do_ptrace)
 		ptrace_wait(type);
-	} else
+	else
 		trace_waitpid(type, pid, &status, 0);
 }
 
-- 
2.25.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 12:22 [PATCH 0/4] Few small trace-cmd fixes Tzvetomir Stoyanov (VMware)
2020-04-30 12:22 ` [PATCH 1/4] trace-cmd: Fix trace-cmd report crash while displaying trace.dat in specific use case Tzvetomir Stoyanov (VMware)
2020-04-30 12:22 ` [PATCH 2/4] trace-cmd: Add "main" in the output of trace-cmd stat when displaying main instance Tzvetomir Stoyanov (VMware)
2020-04-30 12:22 ` [PATCH 3/4] trace-cmd: Create ftrace instances before using them Tzvetomir Stoyanov (VMware)
2020-04-30 12:22 ` [PATCH 4/4] trace-cmd: Do not try to update parent's memory from a fork()-ed child Tzvetomir Stoyanov (VMware)

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git