linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] trace-cmd: Fix trace-cmd stream
@ 2023-01-06 18:39 Steven Rostedt
  2023-01-06 18:39 ` [PATCH 01/10] trace-cmd library: Set recorder to nonblock when finished Steven Rostedt
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Looking at https://bugzilla.kernel.org/show_bug.cgi?id=216577 and analyzing
the fix, I found a bunch of other issues with trace-cmd stream.

This series is to address them.

Steven Rostedt (Google) (10):
  trace-cmd library: Set recorder to nonblock when finished
  trace-cmd stream: Do not block when stopping threads
  trace-cmd stream: Add a flush signal to kick the output
  trace-cmd stream: Do one last flush when finished
  trace-cmd library: Fix read_data() with error from tracefs_cpu_read()
  trace-cmd: Have trace_stream_read() use poll()
  trace-cmd stream: Set default sleep time to half a second
  trace-cmd library: Return the result of tracefs_cpu_stop()
  trace-cmd record: Set sleep_time to zero at end of recording
  trace-cmd record: Keep stopping the recording when finished

 .../include/private/trace-cmd-private.h       |  4 +-
 lib/trace-cmd/trace-recorder.c                | 33 ++++++---
 tracecmd/include/trace-local.h                |  2 +-
 tracecmd/trace-record.c                       | 72 +++++++++++++++----
 tracecmd/trace-stream.c                       | 34 ++++-----
 5 files changed, 99 insertions(+), 46 deletions(-)

-- 
2.35.1


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

* [PATCH 01/10] trace-cmd library: Set recorder to nonblock when finished
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 02/10] trace-cmd stream: Do not block when stopping threads Steven Rostedt
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When the recorder is signaled to stop, make sure it only reads in
non-blocking mode.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-recorder.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index f387091f5177..bb02f7b8ee11 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -317,11 +317,12 @@ static inline void update_fd(struct tracecmd_recorder *recorder, int size)
  */
 static long read_data(struct tracecmd_recorder *recorder)
 {
+	bool nonblock = recorder->stop;
 	char buf[recorder->subbuf_size];
 	long left;
 	long r, w;
 
-	r = tracefs_cpu_read(recorder->tcpu, buf, false);
+	r = tracefs_cpu_read(recorder->tcpu, buf, nonblock);
 
 	left = r;
 	do {
@@ -344,11 +345,13 @@ static long read_data(struct tracecmd_recorder *recorder)
  */
 static long direct_splice_data(struct tracecmd_recorder *recorder)
 {
-	return tracefs_cpu_pipe(recorder->tcpu, recorder->fd, false);
+	bool nonblock = recorder->stop;
+	return tracefs_cpu_pipe(recorder->tcpu, recorder->fd, nonblock);
 }
 
 static long move_data(struct tracecmd_recorder *recorder)
 {
+	bool nonblock = recorder->stop;
 	long ret;
 
 	if (recorder->flags & TRACECMD_RECORD_NOSPLICE)
@@ -357,7 +360,7 @@ static long move_data(struct tracecmd_recorder *recorder)
 	if (recorder->flags & TRACECMD_RECORD_NOBRASS)
 		return direct_splice_data(recorder);
 
-	ret = tracefs_cpu_write(recorder->tcpu, recorder->fd, false);
+	ret = tracefs_cpu_write(recorder->tcpu, recorder->fd, nonblock);
 	if (ret > 0)
 		update_fd(recorder, ret);
 	return ret;
-- 
2.35.1


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

* [PATCH 02/10] trace-cmd stream: Do not block when stopping threads
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
  2023-01-06 18:39 ` [PATCH 01/10] trace-cmd library: Set recorder to nonblock when finished Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 03/10] trace-cmd stream: Add a flush signal to kick the output Steven Rostedt
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When the trace-cmd stream is exiting, it flushes the data. But it does so in
a blocking way, where it can block when there's nothing to read. Set the
timeout of trace_stream_read() to zero in order to return immediately.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 tracecmd/trace-record.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 361524b58772..cc6f27bf22e8 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -798,7 +798,8 @@ static void stop_threads(enum trace_type type)
 	/* Flush out the pipes */
 	if (type & TRACE_TYPE_STREAM) {
 		do {
-			ret = trace_stream_read(pids, recorder_threads, NULL);
+			struct timeval tv = { 0, 0 };
+			ret = trace_stream_read(pids, recorder_threads, &tv);
 		} while (ret > 0);
 	}
 }
-- 
2.35.1


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

* [PATCH 03/10] trace-cmd stream: Add a flush signal to kick the output
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
  2023-01-06 18:39 ` [PATCH 01/10] trace-cmd library: Set recorder to nonblock when finished Steven Rostedt
  2023-01-06 18:39 ` [PATCH 02/10] trace-cmd stream: Do not block when stopping threads Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 04/10] trace-cmd stream: Do one last flush when finished Steven Rostedt
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Have trace-cmd stream periodically signal the recorder tasks to flush some
output, otherwise it will look empty for a long time (especially with the
default setting of a 50 percent buffering, where the ring buffers will not
return read until they are half full).

Add a "finish" flag to tracecmd_flush_recording() that will keep the old
behavior when set, but when not set, it will only do a single
tracefs_cpu_flush() that will output some data, but not all of it.

Use the SIGUSR2 to signal the recorder tasks to do the flush.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 .../include/private/trace-cmd-private.h       |  2 +-
 lib/trace-cmd/trace-recorder.c                | 10 ++++-
 tracecmd/trace-record.c                       | 41 +++++++++++++++----
 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index 2cb6f35cb250..e8b0989ac67e 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -378,7 +378,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);
-long tracecmd_flush_recording(struct tracecmd_recorder *recorder);
+long tracecmd_flush_recording(struct tracecmd_recorder *recorder, bool finish);
 
 enum tracecmd_msg_flags {
 	TRACECMD_MSG_FL_USE_TCP		= 1 << 0,
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index bb02f7b8ee11..79b95ce1193d 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -366,13 +366,19 @@ static long move_data(struct tracecmd_recorder *recorder)
 	return ret;
 }
 
-long tracecmd_flush_recording(struct tracecmd_recorder *recorder)
+long tracecmd_flush_recording(struct tracecmd_recorder *recorder, bool finish)
 {
 	char buf[recorder->subbuf_size];
 	long total = 0;
 	long wrote = 0;
 	long ret;
 
+	if (!recorder)
+		return 0;
+
+	if (!finish)
+		return tracefs_cpu_flush_write(recorder->tcpu, recorder->fd);
+
 	set_nonblock(recorder);
 
 	do {
@@ -421,7 +427,7 @@ int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long s
 	} while (!recorder->stop);
 
 	/* Flush out the rest */
-	ret = tracecmd_flush_recording(recorder);
+	ret = tracecmd_flush_recording(recorder, true);
 
 	if (ret < 0)
 		return ret;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index cc6f27bf22e8..a03d334a65d5 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -1641,12 +1641,21 @@ static inline void ptrace_attach(struct buffer_instance *instance, int pid) { }
 static void trace_or_sleep(enum trace_type type, bool pwait)
 {
 	struct timeval tv = { 1 , 0 };
+	int i;
 
 	if (pwait)
 		ptrace_wait(type);
-	else if (type & TRACE_TYPE_STREAM)
-		trace_stream_read(pids, recorder_threads, &tv);
-	else
+	else if (type & TRACE_TYPE_STREAM) {
+		/* Returns zero if it did not read anything (and did a sleep) */
+		if (trace_stream_read(pids, recorder_threads, &tv) > 0)
+			return;
+		/* Force a flush if nothing was read (including on errors) */
+		for (i = 0; i < recorder_threads; i++) {
+			if (pids[i].pid > 0) {
+				kill(pids[i].pid, SIGUSR2);
+			}
+		}
+	} else
 		sleep(10);
 }
 
@@ -3156,7 +3165,7 @@ static void expand_event_list(void)
 		expand_event_instance(instance);
 }
 
-static void finish(int sig)
+static void finish(void)
 {
 	/* all done */
 	if (recorder)
@@ -3164,6 +3173,23 @@ static void finish(int sig)
 	finished = 1;
 }
 
+static void flush(void)
+{
+	if (recorder)
+		tracecmd_flush_recording(recorder, false);
+}
+
+static void do_sig(int sig)
+{
+	switch (sig) {
+	case SIGUSR1:
+	case SIGINT:
+		return finish();
+	case SIGUSR2:
+		return flush();
+	}
+}
+
 static struct addrinfo *do_getaddrinfo(const char *host, unsigned int port,
 				       enum port_type type)
 {
@@ -3405,7 +3431,8 @@ static int create_recorder(struct buffer_instance *instance, int cpu,
 			return pid;
 
 		signal(SIGINT, SIG_IGN);
-		signal(SIGUSR1, finish);
+		signal(SIGUSR1, do_sig);
+		signal(SIGUSR2, do_sig);
 
 		if (rt_prio)
 			set_prio(rt_prio);
@@ -3451,7 +3478,7 @@ static int create_recorder(struct buffer_instance *instance, int cpu,
 		die ("can't create recorder");
 
 	if (type == TRACE_TYPE_EXTRACT) {
-		ret = tracecmd_flush_recording(recorder);
+		ret = tracecmd_flush_recording(recorder, true);
 		tracecmd_free_recorder(recorder);
 		recorder = NULL;
 		return ret;
@@ -6945,7 +6972,7 @@ static void record_trace(int argc, char **argv,
 	allocate_seq();
 
 	if (type & (TRACE_TYPE_RECORD | TRACE_TYPE_STREAM)) {
-		signal(SIGINT, finish);
+		signal(SIGINT, do_sig);
 		if (!latency)
 			start_threads(type, ctx);
 	}
-- 
2.35.1


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

* [PATCH 04/10] trace-cmd stream: Do one last flush when finished
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
                   ` (2 preceding siblings ...)
  2023-01-06 18:39 ` [PATCH 03/10] trace-cmd stream: Add a flush signal to kick the output Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 05/10] trace-cmd library: Fix read_data() with error from tracefs_cpu_read() Steven Rostedt
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When trace-cmd stream is finished, the trace_or_sleep() will exit before
waiting for the pipes to flush. Call trace_stream_read() one more time, and
with indefinite blocking, as the pipes should finish everything and then
close (which will stop the polling).

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 tracecmd/trace-record.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index a03d334a65d5..83ae8e60d7cb 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -7019,6 +7019,9 @@ static void record_trace(int argc, char **argv,
 		}
 		while (!finished && wait_indefinitely)
 			trace_or_sleep(type, pwait);
+		/* Streams need to be flushed one more time */
+		if (type & TRACE_TYPE_STREAM)
+			trace_stream_read(pids, recorder_threads, NULL);
 	}
 
 	tell_guests_to_stop(ctx);
-- 
2.35.1


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

* [PATCH 05/10] trace-cmd library: Fix read_data() with error from tracefs_cpu_read()
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
                   ` (3 preceding siblings ...)
  2023-01-06 18:39 ` [PATCH 04/10] trace-cmd stream: Do one last flush when finished Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 06/10] trace-cmd: Have trace_stream_read() use poll() Steven Rostedt
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

If tracefs_cpu_read() has an error, it will return a negative value. But the
value is then used as an index. An error check is required to make sure that
if an error is returned from tracefs_cpu_read() the function exits with that
error and does not continue further.

Fixes: 2610bdcde ("trace-cmd library: Use tracefs_cpu for recorder helpers")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-recorder.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index 79b95ce1193d..db159e5e5d14 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -323,6 +323,8 @@ static long read_data(struct tracecmd_recorder *recorder)
 	long r, w;
 
 	r = tracefs_cpu_read(recorder->tcpu, buf, nonblock);
+	if (r < 0)
+		return r;
 
 	left = r;
 	do {
-- 
2.35.1


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

* [PATCH 06/10] trace-cmd: Have trace_stream_read() use poll()
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
                   ` (4 preceding siblings ...)
  2023-01-06 18:39 ` [PATCH 05/10] trace-cmd library: Fix read_data() with error from tracefs_cpu_read() Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 07/10] trace-cmd stream: Set default sleep time to half a second Steven Rostedt
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Replace the select() that is used in trace_stream_read() with poll() as that
is the more robust function, as select() is limited to fds that are less than
1024, and today's kernels can have fds bigger than that.

As a side-effect, since poll() uses milliseconds for its timeout, the
granularity of the timeout is no longer microseconds, even though that is
what is passed in on the command line. We may need to changes this.

If 1us is passed in, it will be rounded up to 1ms. But zero and negative
numbers shall remain the same.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 tracecmd/include/trace-local.h |  2 +-
 tracecmd/trace-record.c        | 13 +++++--------
 tracecmd/trace-stream.c        | 34 +++++++++++++++-------------------
 3 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 023afb3baad1..ae208fb6b593 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -137,7 +137,7 @@ struct tracecmd_input *
 trace_stream_init(struct buffer_instance *instance, int cpu, int fd, int cpus,
 		  struct hook_list *hooks,
 		  tracecmd_handle_init_func handle_init, int global);
-int trace_stream_read(struct pid_record_data *pids, int nr_pids, struct timeval *tv);
+int trace_stream_read(struct pid_record_data *pids, int nr_pids, long sleep_us);
 
 void trace_show_data(struct tracecmd_input *handle, struct tep_record *record);
 
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 83ae8e60d7cb..03f990e3b2d1 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -73,7 +73,7 @@ static int rt_prio;
 static int keep;
 
 static int latency;
-static int sleep_time = 1000;
+static long sleep_time = 1000;
 static int recorder_threads;
 static struct pid_record_data *pids;
 static int buffers;
@@ -798,8 +798,7 @@ static void stop_threads(enum trace_type type)
 	/* Flush out the pipes */
 	if (type & TRACE_TYPE_STREAM) {
 		do {
-			struct timeval tv = { 0, 0 };
-			ret = trace_stream_read(pids, recorder_threads, &tv);
+			ret = trace_stream_read(pids, recorder_threads, 0);
 		} while (ret > 0);
 	}
 }
@@ -1307,7 +1306,6 @@ static void update_task_filter(void)
 
 static pid_t trace_waitpid(enum trace_type type, pid_t pid, int *status, int options)
 {
-	struct timeval tv = { 1, 0 };
 	int ret;
 
 	if (type & TRACE_TYPE_STREAM)
@@ -1319,7 +1317,7 @@ static pid_t trace_waitpid(enum trace_type type, pid_t pid, int *status, int opt
 			return ret;
 
 		if (type & TRACE_TYPE_STREAM)
-			trace_stream_read(pids, recorder_threads, &tv);
+			trace_stream_read(pids, recorder_threads, sleep_time);
 	} while (1);
 }
 
@@ -1640,14 +1638,13 @@ static inline void ptrace_attach(struct buffer_instance *instance, int pid) { }
 
 static void trace_or_sleep(enum trace_type type, bool pwait)
 {
-	struct timeval tv = { 1 , 0 };
 	int i;
 
 	if (pwait)
 		ptrace_wait(type);
 	else if (type & TRACE_TYPE_STREAM) {
 		/* Returns zero if it did not read anything (and did a sleep) */
-		if (trace_stream_read(pids, recorder_threads, &tv) > 0)
+		if (trace_stream_read(pids, recorder_threads, sleep_time) > 0)
 			return;
 		/* Force a flush if nothing was read (including on errors) */
 		for (i = 0; i < recorder_threads; i++) {
@@ -7021,7 +7018,7 @@ static void record_trace(int argc, char **argv,
 			trace_or_sleep(type, pwait);
 		/* Streams need to be flushed one more time */
 		if (type & TRACE_TYPE_STREAM)
-			trace_stream_read(pids, recorder_threads, NULL);
+			trace_stream_read(pids, recorder_threads, -1);
 	}
 
 	tell_guests_to_stop(ctx);
diff --git a/tracecmd/trace-stream.c b/tracecmd/trace-stream.c
index d83513f8aa89..ec6a10f7a6fa 100644
--- a/tracecmd/trace-stream.c
+++ b/tracecmd/trace-stream.c
@@ -4,6 +4,7 @@
  *
  */
 #include <stdio.h>
+#include <poll.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <errno.h>
@@ -83,17 +84,19 @@ trace_stream_init(struct buffer_instance *instance, int cpu, int fd, int cpus,
 	return NULL;
 }
 
-int trace_stream_read(struct pid_record_data *pids, int nr_pids, struct timeval *tv)
+int trace_stream_read(struct pid_record_data *pids, int nr_pids, long sleep_us)
 {
-	struct tep_record *record;
-	struct pid_record_data *pid;
 	struct pid_record_data *last_pid;
-	fd_set rfds;
-	int top_rfd = 0;
-	int nr_fd;
+	struct pid_record_data *pid;
+	struct tep_record *record;
+	struct pollfd pollfd[nr_pids];
+	long sleep_ms = sleep_us > 0 ? (sleep_us + 999) / 1000 : sleep_us;
 	int ret;
 	int i;
 
+	if (!nr_pids)
+		return 0;
+
 	last_pid = NULL;
 
  again:
@@ -118,25 +121,18 @@ int trace_stream_read(struct pid_record_data *pids, int nr_pids, struct timeval
 		return 1;
 	}
 
-	nr_fd = 0;
-	FD_ZERO(&rfds);
-
 	for (i = 0; i < nr_pids; i++) {
 		/* Do not process closed pipes */
-		if (pids[i].closed)
+		if (pids[i].closed) {
+			memset(pollfd + i, 0, sizeof(*pollfd));
 			continue;
-		nr_fd++;
-		if (pids[i].brass[0] > top_rfd)
-			top_rfd = pids[i].brass[0];
+		}
 
-		FD_SET(pids[i].brass[0], &rfds);
+		pollfd[i].fd = pids[i].brass[0];
+		pollfd[i].events = POLLIN;
 	}
 
-	if (!nr_fd)
-		return 0;
-
-	ret = select(top_rfd + 1, &rfds, NULL, NULL, tv);
-
+	ret = poll(pollfd, nr_pids, sleep_ms);
 	if (ret > 0)
 		goto again;
 
-- 
2.35.1


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

* [PATCH 07/10] trace-cmd stream: Set default sleep time to half a second
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
                   ` (5 preceding siblings ...)
  2023-01-06 18:39 ` [PATCH 06/10] trace-cmd: Have trace_stream_read() use poll() Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 08/10] trace-cmd library: Return the result of tracefs_cpu_stop() Steven Rostedt
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When in streaming mode, set the default to half a second instead of 1
millisecond (which should be changed for record as well).

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 tracecmd/trace-record.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 03f990e3b2d1..9eb10cd8ccdf 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -7161,11 +7161,15 @@ void trace_stream(int argc, char **argv)
 {
 	struct common_record_context ctx;
 
+	/* Default sleep time is half a second for streaming */
+	sleep_time = 500000;
+
 	parse_record_options(argc, argv, CMD_stream, &ctx);
 	record_trace_command(argc, argv, &ctx);
 	exit(0);
 }
 
+
 void trace_profile(int argc, char **argv)
 {
 	struct common_record_context ctx;
-- 
2.35.1


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

* [PATCH 08/10] trace-cmd library: Return the result of tracefs_cpu_stop()
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
                   ` (6 preceding siblings ...)
  2023-01-06 18:39 ` [PATCH 07/10] trace-cmd stream: Set default sleep time to half a second Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 09/10] trace-cmd record: Set sleep_time to zero at end of recording Steven Rostedt
  2023-01-06 18:39 ` [PATCH 10/10] trace-cmd record: Keep stopping the recording when finished Steven Rostedt
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Ideally, tracefs_cpu_stop() is suppose to return zero if it guaranteed to
stop the recorders (but this may not be true if called from a signal
handler). Return the result of tracefs_cpu_stop() in
tracecmd_stop_recording().

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 lib/trace-cmd/include/private/trace-cmd-private.h |  2 +-
 lib/trace-cmd/trace-recorder.c                    | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
index e8b0989ac67e..05c084ceea41 100644
--- a/lib/trace-cmd/include/private/trace-cmd-private.h
+++ b/lib/trace-cmd/include/private/trace-cmd-private.h
@@ -377,7 +377,7 @@ struct tracecmd_recorder *tracecmd_create_buffer_recorder(const char *file, int
 struct tracecmd_recorder *tracecmd_create_buffer_recorder_maxkb(const char *file, int cpu, unsigned flags, struct tracefs_instance *instance, int maxkb);
 
 int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long sleep);
-void tracecmd_stop_recording(struct tracecmd_recorder *recorder);
+int tracecmd_stop_recording(struct tracecmd_recorder *recorder);
 long tracecmd_flush_recording(struct tracecmd_recorder *recorder, bool finish);
 
 enum tracecmd_msg_flags {
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index db159e5e5d14..23499f308156 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -102,9 +102,9 @@ void tracecmd_free_recorder(struct tracecmd_recorder *recorder)
 	free(recorder);
 }
 
-static void set_nonblock(struct tracecmd_recorder *recorder)
+static int set_nonblock(struct tracecmd_recorder *recorder)
 {
-	tracefs_cpu_stop(recorder->tcpu);
+	return tracefs_cpu_stop(recorder->tcpu);
 }
 
 static struct tracecmd_recorder *
@@ -437,12 +437,12 @@ int tracecmd_start_recording(struct tracecmd_recorder *recorder, unsigned long s
 	return 0;
 }
 
-void tracecmd_stop_recording(struct tracecmd_recorder *recorder)
+int tracecmd_stop_recording(struct tracecmd_recorder *recorder)
 {
 	if (!recorder)
-		return;
-
-	set_nonblock(recorder);
+		return -1;
 
 	recorder->stop = 1;
+
+	return set_nonblock(recorder);
 }
-- 
2.35.1


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

* [PATCH 09/10] trace-cmd record: Set sleep_time to zero at end of recording
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
                   ` (7 preceding siblings ...)
  2023-01-06 18:39 ` [PATCH 08/10] trace-cmd library: Return the result of tracefs_cpu_stop() Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  2023-01-06 18:39 ` [PATCH 10/10] trace-cmd record: Keep stopping the recording when finished Steven Rostedt
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When the recording (of trace-cmd record) or streaming (of trace-cmd stream)
is finished, set sleep_time to zero. This will cause various calls to read
the buffer to not block, as the sleep_time is used in some instances to
determine how long to wait if there's no data.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 tracecmd/trace-record.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 9eb10cd8ccdf..b3614b0706de 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3164,6 +3164,8 @@ static void expand_event_list(void)
 
 static void finish(void)
 {
+	sleep_time = 0;
+
 	/* all done */
 	if (recorder)
 		tracecmd_stop_recording(recorder);
-- 
2.35.1


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

* [PATCH 10/10] trace-cmd record: Keep stopping the recording when finished
  2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
                   ` (8 preceding siblings ...)
  2023-01-06 18:39 ` [PATCH 09/10] trace-cmd record: Set sleep_time to zero at end of recording Steven Rostedt
@ 2023-01-06 18:39 ` Steven Rostedt
  9 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2023-01-06 18:39 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (Google)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The tracecmd_stop_recording() is suppose to force the recorders to finish,
but because it is called from a signal handler, that may not necessarily be
the case (due to race conditions and such). Set an alarm to try again in a
second, then after two seconds, and then three, and so on.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 tracecmd/trace-record.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index b3614b0706de..7f0cebe8cd7a 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3164,11 +3164,21 @@ static void expand_event_list(void)
 
 static void finish(void)
 {
+	static int secs = 1;
+
 	sleep_time = 0;
 
 	/* all done */
-	if (recorder)
+	if (recorder) {
 		tracecmd_stop_recording(recorder);
+		/*
+		 * We could just call the alarm if the above returned non zero,
+		 * as zero is suppose to guarantee that the reader woke up.
+		 * But as this is called from a signal handler, that may not
+		 * necessarily be the case.
+		 */
+		alarm(secs++);
+	}
 	finished = 1;
 }
 
@@ -3181,6 +3191,7 @@ static void flush(void)
 static void do_sig(int sig)
 {
 	switch (sig) {
+	case SIGALRM:
 	case SIGUSR1:
 	case SIGINT:
 		return finish();
@@ -3432,6 +3443,7 @@ static int create_recorder(struct buffer_instance *instance, int cpu,
 		signal(SIGINT, SIG_IGN);
 		signal(SIGUSR1, do_sig);
 		signal(SIGUSR2, do_sig);
+		signal(SIGALRM, do_sig);
 
 		if (rt_prio)
 			set_prio(rt_prio);
-- 
2.35.1


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

end of thread, other threads:[~2023-01-06 18:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-06 18:39 [PATCH 00/10] trace-cmd: Fix trace-cmd stream Steven Rostedt
2023-01-06 18:39 ` [PATCH 01/10] trace-cmd library: Set recorder to nonblock when finished Steven Rostedt
2023-01-06 18:39 ` [PATCH 02/10] trace-cmd stream: Do not block when stopping threads Steven Rostedt
2023-01-06 18:39 ` [PATCH 03/10] trace-cmd stream: Add a flush signal to kick the output Steven Rostedt
2023-01-06 18:39 ` [PATCH 04/10] trace-cmd stream: Do one last flush when finished Steven Rostedt
2023-01-06 18:39 ` [PATCH 05/10] trace-cmd library: Fix read_data() with error from tracefs_cpu_read() Steven Rostedt
2023-01-06 18:39 ` [PATCH 06/10] trace-cmd: Have trace_stream_read() use poll() Steven Rostedt
2023-01-06 18:39 ` [PATCH 07/10] trace-cmd stream: Set default sleep time to half a second Steven Rostedt
2023-01-06 18:39 ` [PATCH 08/10] trace-cmd library: Return the result of tracefs_cpu_stop() Steven Rostedt
2023-01-06 18:39 ` [PATCH 09/10] trace-cmd record: Set sleep_time to zero at end of recording Steven Rostedt
2023-01-06 18:39 ` [PATCH 10/10] trace-cmd record: Keep stopping the recording when finished Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).