[PATCH v5] libtracefs: Add APIs for data streaming

[PATCH v5] libtracefs: Add APIs for data streaming

[Steven Rostedt]

From: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>

The new APIs can be used to dump the content of "trace_pipe" into a
file or directly to "stdout". The "splice" system call is used to
moves the data without copying. The new functionality is essentially
identical to what 'trace-cmd show -p' does.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
[ Updated to fix writing to STDOUT ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Changes since v4:

 - Simplified. Have tracefs_trace_pipe_stream() test the output file
   descriptor if it is safe to splice to. If it is, then it splices to
   the output. If not, it then simply reads the trace and writes it.

 - The tracefs_trace_pipe_print() goes back to just calling
   tracefs_trace_pipe_stream().

 Documentation/libtracefs-stream.txt | 106 +++++++++++++++++++++
 include/tracefs-local.h             |   1 +
 include/tracefs.h                   |   5 +
 src/tracefs-tools.c                 | 143 ++++++++++++++++++++++++++++
 4 files changed, 255 insertions(+)
 create mode 100644 Documentation/libtracefs-stream.txt

diff --git a/Documentation/libtracefs-stream.txt b/Documentation/libtracefs-stream.txt
new file mode 100644
index 0000000..b9692e3
--- /dev/null
+++ b/Documentation/libtracefs-stream.txt
@@ -0,0 +1,106 @@
+libtracefs(3)
+=============
+
+NAME
+----
+tracefs_trace_pipe_stream, tracefs_trace_pipe_print -
+redirect the stream of trace data to an output or stdout.
+
+SYNOPSIS
+--------
+[verse]
+--
+*#include <tracefs.h>*
+
+ssize_t tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
+ssize_t tracefs_trace_pipe_print(struct tracefs_instance *instance);
+
+--
+
+DESCRIPTION
+-----------
+If NULL is passed as _instance_, the top trace instance is used.
+The user can interrupt the streaming of the data by pressing Ctrl-c.
+
+The _tracefs_trace_pipe_stream()_ function redirects the stream of trace data to an output
+file. The "splice" system call is used to moves the data without copying between kernel
+address space and user address space. The _fd_ is the file descriptor of the output file
+and _flags_ is a bit mask of flags to be passed to the "splice" system call.
+
+The _tracefs_trace_pipe_print()_ function is similar to _tracefs_trace_pipe_stream()_, but
+the stream of trace data is redirected to stdout.
+
+
+RETURN VALUE
+------------
+The _tracefs_trace_pipe_stream()_, and _tracefs_trace_pipe_print()_ functions return the
+number of bytes transfered if the operation is successful, or -1 in case of an error.
+
+EXAMPLE
+-------
+[source,c]
+--
+#include <unistd.h>
+#include <signal.h>
+
+#include <tracefs.h>
+
+void stop(int sig)
+{
+	tracefs_trace_pipe_stop(NULL);
+}
+
+int main()
+{
+	mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
+	const char *filename = "trace.txt";
+	int fd = creat(filename, mode);
+	int ret;
+
+	signal(SIGINT, stop);
+	ret = tracefs_trace_pipe_stream(fd, NULL, SPLICE_F_NONBLOCK);
+	close(fd);
+
+	return ret;
+}
+--
+FILES
+-----
+[verse]
+--
+*tracefs.h*
+	Header file to include in order to have access to the library APIs.
+*-ltracefs*
+	Linker switch to add when building a program that uses the library.
+--
+
+SEE ALSO
+--------
+_libtracefs(3)_,
+_libtraceevent(3)_,
+_trace-cmd(1)_,
+Documentation/trace/ftrace.rst from the Linux kernel tree
+
+AUTHOR
+------
+[verse]
+--
+*Steven Rostedt* <rostedt@goodmis.org>
+*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
+--
+REPORTING BUGS
+--------------
+Report bugs to  <linux-trace-devel@vger.kernel.org>
+
+LICENSE
+-------
+libtracefs is Free Software licensed under the GNU LGPL 2.1
+
+RESOURCES
+---------
+https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
+
+COPYING
+-------
+Copyright \(C) 2021 VMware, Inc. Free use of this software is granted under
+the terms of the GNU Public License (GPL).
diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index 9e3aa18..73698e8 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -30,6 +30,7 @@ struct tracefs_instance {
 	int				ftrace_notrace_fd;
 	int				ftrace_marker_fd;
 	int				ftrace_marker_raw_fd;
+	bool				pipe_keep_going;
 };
 
 extern pthread_mutex_t toplevel_lock;
diff --git a/include/tracefs.h b/include/tracefs.h
index e29b550..23a3c7d 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -184,4 +184,9 @@ int tracefs_function_notrace(struct tracefs_instance *instance, const char *filt
 /* Control library logs */
 void tracefs_set_loglevel(enum tep_loglevel level);
 
+ssize_t tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
+ssize_t tracefs_trace_pipe_print(struct tracefs_instance *instance);
+void tracefs_trace_pipe_stop(struct tracefs_instance *instance);
+
+
 #endif /* _TRACE_FS_H */
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 0cbb56d..48ef928 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -912,3 +912,146 @@ int tracefs_function_notrace(struct tracefs_instance *instance, const char *filt
 	tracefs_put_tracing_file(filter_path);
 	return ret;
 }
+
+static bool splice_safe(int fd, int pfd)
+{
+	int ret;
+
+	errno = 0;
+	ret = splice(pfd, NULL, fd, NULL,
+		     10, SPLICE_F_NONBLOCK | SPLICE_F_MOVE);
+
+	return !ret || (ret < 0 && errno == EAGAIN);
+}
+
+static ssize_t read_trace_pipe(bool *keep_going, int in_fd, int out_fd)
+{
+	char buf[BUFSIZ];
+	ssize_t bread = 0;
+	int ret;
+
+	do {
+		ret = read(in_fd, buf, BUFSIZ);
+		if (ret > 0) {
+			ret = write(STDOUT_FILENO, buf, ret);
+			if (ret > 0)
+				bread += ret;
+		}
+	} while (ret > 0 && *(volatile bool *)keep_going);
+
+	if (ret < 0 && (errno == EAGAIN || errno == EINTR))
+		ret = 0;
+
+	return ret < 0 ? ret : bread;
+}
+
+static bool top_pipe_keep_going;
+
+/**
+ * tracefs_trace_pipe_stream - redirect the stream of trace data to an output
+ * file. The "splice" system call is used to moves the data without copying
+ * between kernel address space and user address space. The user can interrupt
+ * the streaming of the data by pressing Ctrl-c.
+ * @fd: The file descriptor of the output file.
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ * @flags: flags to be passed to the "splice" system call.
+ *
+ * Returns -1 in case of an error or number of bytes transferred otherwise.
+ */
+ssize_t tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
+				 int flags)
+{
+	bool *keep_going = instance ? &instance->pipe_keep_going :
+				      &top_pipe_keep_going;
+	const char *file = "trace_pipe";
+	int brass[2], in_fd, ret = -1;
+	int oflags = flags & SPLICE_F_NONBLOCK ? O_NONBLOCK: 0;
+	off_t data_size;
+	ssize_t bread = 0;
+
+	(*(volatile bool *)keep_going) = true;
+
+	in_fd = tracefs_instance_file_open(instance, file, O_RDONLY | oflags);
+	if (in_fd < 0) {
+		tracefs_warning("Failed to open 'trace_pipe'.");
+		return ret;
+	}
+
+	if(pipe(brass) < 0) {
+		tracefs_warning("Failed to open pipe.");
+		goto close_file;
+	}
+
+	data_size = fcntl(brass[0], F_GETPIPE_SZ);
+	if (data_size <= 0) {
+		tracefs_warning("Failed to open pipe (size=0).");
+		goto close_all;
+	}
+
+	/* Test if the output is splice safe */
+	if (!splice_safe(fd, brass[0])) {
+		close(brass[0]);
+		close(brass[1]);
+		return read_trace_pipe(keep_going, in_fd, fd);
+	}
+
+	errno = 0;
+
+	while (*(volatile bool *)keep_going) {
+		ret = splice(in_fd, NULL,
+			     brass[1], NULL,
+			     data_size, flags);
+		if (ret < 0)
+			break;
+
+		ret = splice(brass[0], NULL,
+			     fd, NULL,
+			     data_size, flags);
+		if (ret < 0)
+			break;
+		bread += ret;
+	}
+
+	/*
+	 * Do not return error in the case when the "splice" system call
+	 * was interrupted by the user (pressing Ctrl-c).
+	 * Or if NONBLOCK was specified.
+	 */
+	if (!keep_going || errno == EAGAIN || errno == EINTR)
+		ret = 0;
+
+ close_all:
+	close(brass[0]);
+	close(brass[1]);
+ close_file:
+	close(in_fd);
+
+	return ret ? ret : bread;
+}
+
+/**
+ * tracefs_trace_pipe_print - redirect the stream of trace data to "stdout".
+ * The "splice" system call is used to moves the data without copying
+ * between kernel address space and user address space.
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ *
+ * Returns -1 in case of an error or number of bytes transferred otherwise.
+ */
+
+ssize_t tracefs_trace_pipe_print(struct tracefs_instance *instance)
+{
+	return tracefs_trace_pipe_stream(STDOUT_FILENO, instance,
+					 SPLICE_F_NONBLOCK);
+}
+
+/**
+ * tracefs_trace_pipe_stop - stop the streaming of trace data.
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ */
+void tracefs_trace_pipe_stop(struct tracefs_instance *instance)
+{
+	if (instance)
+		instance->pipe_keep_going = false;
+	else
+		top_pipe_keep_going = false;
+}
-- 
2.29.2

Re: [PATCH v5] libtracefs: Add APIs for data streaming

[Yordan Karadzhov]

On 25.06.21 г. 21:27, Steven Rostedt wrote:
> From: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>
> 
> The new APIs can be used to dump the content of "trace_pipe" into a
> file or directly to "stdout". The "splice" system call is used to
> moves the data without copying. The new functionality is essentially
> identical to what 'trace-cmd show -p' does.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> [ Updated to fix writing to STDOUT ]
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Changes since v4:
> 
>   - Simplified. Have tracefs_trace_pipe_stream() test the output file
>     descriptor if it is safe to splice to. If it is, then it splices to
>     the output. If not, it then simply reads the trace and writes it.
> 
>   - The tracefs_trace_pipe_print() goes back to just calling
>     tracefs_trace_pipe_stream().
> 
>   Documentation/libtracefs-stream.txt | 106 +++++++++++++++++++++
>   include/tracefs-local.h             |   1 +
>   include/tracefs.h                   |   5 +
>   src/tracefs-tools.c                 | 143 ++++++++++++++++++++++++++++
>   4 files changed, 255 insertions(+)
>   create mode 100644 Documentation/libtracefs-stream.txt
> 
> diff --git a/Documentation/libtracefs-stream.txt b/Documentation/libtracefs-stream.txt
> new file mode 100644
> index 0000000..b9692e3
> --- /dev/null
> +++ b/Documentation/libtracefs-stream.txt
> @@ -0,0 +1,106 @@
> +libtracefs(3)
> +=============
> +
> +NAME
> +----
> +tracefs_trace_pipe_stream, tracefs_trace_pipe_print -
> +redirect the stream of trace data to an output or stdout.
> +
> +SYNOPSIS
> +--------
> +[verse]
> +--
> +*#include <tracefs.h>*
> +
> +ssize_t tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
> +ssize_t tracefs_trace_pipe_print(struct tracefs_instance *instance);
> +
> +--
> +
> +DESCRIPTION
> +-----------
> +If NULL is passed as _instance_, the top trace instance is used.
> +The user can interrupt the streaming of the data by pressing Ctrl-c.
> +
> +The _tracefs_trace_pipe_stream()_ function redirects the stream of trace data to an output
> +file. The "splice" system call is used to moves the data without copying between kernel
> +address space and user address space. The _fd_ is the file descriptor of the output file
> +and _flags_ is a bit mask of flags to be passed to the "splice" system call.
> +
> +The _tracefs_trace_pipe_print()_ function is similar to _tracefs_trace_pipe_stream()_, but
> +the stream of trace data is redirected to stdout.
> +
> +
> +RETURN VALUE
> +------------
> +The _tracefs_trace_pipe_stream()_, and _tracefs_trace_pipe_print()_ functions return the
> +number of bytes transfered if the operation is successful, or -1 in case of an error.
> +
> +EXAMPLE
> +-------
> +[source,c]
> +--
> +#include <unistd.h>
> +#include <signal.h>
> +
> +#include <tracefs.h>
> +
> +void stop(int sig)
> +{
> +	tracefs_trace_pipe_stop(NULL);
> +}
> +
> +int main()
> +{
> +	mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
> +	const char *filename = "trace.txt";
> +	int fd = creat(filename, mode);
> +	int ret;
> +
> +	signal(SIGINT, stop);
> +	ret = tracefs_trace_pipe_stream(fd, NULL, SPLICE_F_NONBLOCK);
> +	close(fd);
> +
> +	return ret;
> +}
> +--
> +FILES
> +-----
> +[verse]
> +--
> +*tracefs.h*
> +	Header file to include in order to have access to the library APIs.
> +*-ltracefs*
> +	Linker switch to add when building a program that uses the library.
> +--
> +
> +SEE ALSO
> +--------
> +_libtracefs(3)_,
> +_libtraceevent(3)_,
> +_trace-cmd(1)_,
> +Documentation/trace/ftrace.rst from the Linux kernel tree
> +
> +AUTHOR
> +------
> +[verse]
> +--
> +*Steven Rostedt* <rostedt@goodmis.org>
> +*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
> +--
> +REPORTING BUGS
> +--------------
> +Report bugs to  <linux-trace-devel@vger.kernel.org>
> +
> +LICENSE
> +-------
> +libtracefs is Free Software licensed under the GNU LGPL 2.1
> +
> +RESOURCES
> +---------
> +https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
> +
> +COPYING
> +-------
> +Copyright \(C) 2021 VMware, Inc. Free use of this software is granted under
> +the terms of the GNU Public License (GPL).
> diff --git a/include/tracefs-local.h b/include/tracefs-local.h
> index 9e3aa18..73698e8 100644
> --- a/include/tracefs-local.h
> +++ b/include/tracefs-local.h
> @@ -30,6 +30,7 @@ struct tracefs_instance {
>   	int				ftrace_notrace_fd;
>   	int				ftrace_marker_fd;
>   	int				ftrace_marker_raw_fd;
> +	bool				pipe_keep_going;
>   };
>   
>   extern pthread_mutex_t toplevel_lock;
> diff --git a/include/tracefs.h b/include/tracefs.h
> index e29b550..23a3c7d 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -184,4 +184,9 @@ int tracefs_function_notrace(struct tracefs_instance *instance, const char *filt
>   /* Control library logs */
>   void tracefs_set_loglevel(enum tep_loglevel level);
>   
> +ssize_t tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
> +ssize_t tracefs_trace_pipe_print(struct tracefs_instance *instance);
> +void tracefs_trace_pipe_stop(struct tracefs_instance *instance);
> +
> +
>   #endif /* _TRACE_FS_H */
> diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
> index 0cbb56d..48ef928 100644
> --- a/src/tracefs-tools.c
> +++ b/src/tracefs-tools.c
> @@ -912,3 +912,146 @@ int tracefs_function_notrace(struct tracefs_instance *instance, const char *filt
>   	tracefs_put_tracing_file(filter_path);
>   	return ret;
>   }
> +
> +static bool splice_safe(int fd, int pfd)
> +{
> +	int ret;
> +
> +	errno = 0;
> +	ret = splice(pfd, NULL, fd, NULL,
> +		     10, SPLICE_F_NONBLOCK | SPLICE_F_MOVE);
> +
> +	return !ret || (ret < 0 && errno == EAGAIN);
> +}
> +
> +static ssize_t read_trace_pipe(bool *keep_going, int in_fd, int out_fd)
> +{
> +	char buf[BUFSIZ];
> +	ssize_t bread = 0;
> +	int ret;
> +
> +	do {
> +		ret = read(in_fd, buf, BUFSIZ);
> +		if (ret > 0) {
> +			ret = write(STDOUT_FILENO, buf, ret);

this should be
			ret = write(in_fd, buf, ret);

> +			if (ret > 0)
> +				bread += ret;
> +		}
> +	} while (ret > 0 && *(volatile bool *)keep_going);
> +

Nit: When looping you check the value of 'ret' 3 times.
      The same can be done with only 2 checks.

	while (*(volatile bool *)keep_going) {
		ret = read(in_fd, buf, BUFSIZ);
		if (ret <= 0)
			break;

		ret = write(out_fd, buf, ret);
		if (ret <= 0)
			break;

		bread += ret;
	}

> +	if (ret < 0 && (errno == EAGAIN || errno == EINTR))
> +		ret = 0;
> +
> +	return ret < 0 ? ret : bread;
> +}
> +
> +static bool top_pipe_keep_going;
> +
> +/**
> + * tracefs_trace_pipe_stream - redirect the stream of trace data to an output
> + * file. The "splice" system call is used to moves the data without copying
> + * between kernel address space and user address space. The user can interrupt
> + * the streaming of the data by pressing Ctrl-c.
> + * @fd: The file descriptor of the output file.
> + * @instance: ftrace instance, can be NULL for top tracing instance.
> + * @flags: flags to be passed to the "splice" system call.
> + *
> + * Returns -1 in case of an error or number of bytes transferred otherwise.
> + */
> +ssize_t tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
> +				 int flags)
> +{
> +	bool *keep_going = instance ? &instance->pipe_keep_going :
> +				      &top_pipe_keep_going;
> +	const char *file = "trace_pipe";
> +	int brass[2], in_fd, ret = -1;
> +	int oflags = flags & SPLICE_F_NONBLOCK ? O_NONBLOCK: 0;
> +	off_t data_size;
> +	ssize_t bread = 0;
> +
> +	(*(volatile bool *)keep_going) = true;
> +
> +	in_fd = tracefs_instance_file_open(instance, file, O_RDONLY | oflags);
> +	if (in_fd < 0) {
> +		tracefs_warning("Failed to open 'trace_pipe'.");
> +		return ret;
> +	}
> +
> +	if(pipe(brass) < 0) {
> +		tracefs_warning("Failed to open pipe.");
> +		goto close_file;
> +	}
> +
> +	data_size = fcntl(brass[0], F_GETPIPE_SZ);
> +	if (data_size <= 0) {
> +		tracefs_warning("Failed to open pipe (size=0).");
> +		goto close_all;
> +	}
> +
> +	/* Test if the output is splice safe */
> +	if (!splice_safe(fd, brass[0])) {
> +		close(brass[0]);
> +		close(brass[1]);
> +		return read_trace_pipe(keep_going, in_fd, fd);

we must close 'in_fd' before returning.

> +	}
> +
> +	errno = 0;
> +
> +	while (*(volatile bool *)keep_going) {
> +		ret = splice(in_fd, NULL,
> +			     brass[1], NULL,
> +			     data_size, flags);
> +		if (ret < 0)
> +			break;
> +
> +		ret = splice(brass[0], NULL,
> +			     fd, NULL,
> +			     data_size, flags);
> +		if (ret < 0)
> +			break;
> +		bread += ret;
> +	}
> +
> +	/*
> +	 * Do not return error in the case when the "splice" system call
> +	 * was interrupted by the user (pressing Ctrl-c).
> +	 * Or if NONBLOCK was specified.
> +	 */
> +	if (!keep_going || errno == EAGAIN || errno == EINTR)
> +		ret = 0;
> +
> + close_all:
> +	close(brass[0]);
> +	close(brass[1]);
> + close_file:
> +	close(in_fd);
> +
> +	return ret ? ret : bread;
> +}
> +
> +/**
> + * tracefs_trace_pipe_print - redirect the stream of trace data to "stdout".
> + * The "splice" system call is used to moves the data without copying
> + * between kernel address space and user address space.
> + * @instance: ftrace instance, can be NULL for top tracing instance.
> + *
> + * Returns -1 in case of an error or number of bytes transferred otherwise.
> + */
> +
> +ssize_t tracefs_trace_pipe_print(struct tracefs_instance *instance)
> +{
> +	return tracefs_trace_pipe_stream(STDOUT_FILENO, instance,
> +					 SPLICE_F_NONBLOCK);
> +}
> +

In fact I want to use this function in "blocking" mode. I would like to keep listening for new trace data until the user 
presses 'Ctrl-c'. Can we make this function to take a second parameter that will be the splice flags?

Thanks a lot!
Yordan

> +/**
> + * tracefs_trace_pipe_stop - stop the streaming of trace data.
> + * @instance: ftrace instance, can be NULL for top tracing instance.
> + */
> +void tracefs_trace_pipe_stop(struct tracefs_instance *instance)
> +{
> +	if (instance)
> +		instance->pipe_keep_going = false;
> +	else
> +		top_pipe_keep_going = false;
> +}
> 

Re: [PATCH v5] libtracefs: Add APIs for data streaming

[Steven Rostedt]

On Mon, 28 Jun 2021 11:58:10 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:
> > +
> > +static ssize_t read_trace_pipe(bool *keep_going, int in_fd, int out_fd)
> > +{
> > +	char buf[BUFSIZ];
> > +	ssize_t bread = 0;
> > +	int ret;
> > +
> > +	do {
> > +		ret = read(in_fd, buf, BUFSIZ);
> > +		if (ret > 0) {
> > +			ret = write(STDOUT_FILENO, buf, ret);  
> 
> this should be

Doh! I know I was hard coding that in some places for debugging, and
that may have been me not cleaning it up. Thanks, will fix.


> 			ret = write(in_fd, buf, ret);
> 
> > +			if (ret > 0)
> > +				bread += ret;
> > +		}
> > +	} while (ret > 0 && *(volatile bool *)keep_going);
> > +  
> 
> Nit: When looping you check the value of 'ret' 3 times.
>       The same can be done with only 2 checks.
> 
> 	while (*(volatile bool *)keep_going) {
> 		ret = read(in_fd, buf, BUFSIZ);
> 		if (ret <= 0)
> 			break;
> 
> 		ret = write(out_fd, buf, ret);
> 		if (ret <= 0)
> 			break;
> 
> 		bread += ret;
> 	}

Nice, I got you thinking about optimizations ;-)

I'll update it.


> 
> > +	if (ret < 0 && (errno == EAGAIN || errno == EINTR))
> > +		ret = 0;
> > +
> > +	return ret < 0 ? ret : bread;
> > +}
> > +
> > +static bool top_pipe_keep_going;
> > +
> > +/**
> > + * tracefs_trace_pipe_stream - redirect the stream of trace data to an output
> > + * file. The "splice" system call is used to moves the data without copying
> > + * between kernel address space and user address space. The user can interrupt
> > + * the streaming of the data by pressing Ctrl-c.
> > + * @fd: The file descriptor of the output file.
> > + * @instance: ftrace instance, can be NULL for top tracing instance.
> > + * @flags: flags to be passed to the "splice" system call.
> > + *
> > + * Returns -1 in case of an error or number of bytes transferred otherwise.
> > + */
> > +ssize_t tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
> > +				 int flags)
> > +{
> > +	bool *keep_going = instance ? &instance->pipe_keep_going :
> > +				      &top_pipe_keep_going;
> > +	const char *file = "trace_pipe";
> > +	int brass[2], in_fd, ret = -1;
> > +	int oflags = flags & SPLICE_F_NONBLOCK ? O_NONBLOCK: 0;
> > +	off_t data_size;
> > +	ssize_t bread = 0;
> > +
> > +	(*(volatile bool *)keep_going) = true;
> > +
> > +	in_fd = tracefs_instance_file_open(instance, file, O_RDONLY | oflags);
> > +	if (in_fd < 0) {
> > +		tracefs_warning("Failed to open 'trace_pipe'.");
> > +		return ret;
> > +	}
> > +
> > +	if(pipe(brass) < 0) {
> > +		tracefs_warning("Failed to open pipe.");
> > +		goto close_file;
> > +	}
> > +
> > +	data_size = fcntl(brass[0], F_GETPIPE_SZ);
> > +	if (data_size <= 0) {
> > +		tracefs_warning("Failed to open pipe (size=0).");
> > +		goto close_all;
> > +	}
> > +
> > +	/* Test if the output is splice safe */
> > +	if (!splice_safe(fd, brass[0])) {
> > +		close(brass[0]);
> > +		close(brass[1]);
> > +		return read_trace_pipe(keep_going, in_fd, fd);  
> 
> we must close 'in_fd' before returning.

Good catch. I was planning on closing it in read_trace_pipe() and
forgot. I'll close it here.

> 
> > +	}
> > +
> > +	errno = 0;
> > +
> > +	while (*(volatile bool *)keep_going) {
> > +		ret = splice(in_fd, NULL,
> > +			     brass[1], NULL,
> > +			     data_size, flags);
> > +		if (ret < 0)
> > +			break;
> > +
> > +		ret = splice(brass[0], NULL,
> > +			     fd, NULL,
> > +			     data_size, flags);
> > +		if (ret < 0)
> > +			break;
> > +		bread += ret;
> > +	}
> > +
> > +	/*
> > +	 * Do not return error in the case when the "splice" system call
> > +	 * was interrupted by the user (pressing Ctrl-c).
> > +	 * Or if NONBLOCK was specified.
> > +	 */
> > +	if (!keep_going || errno == EAGAIN || errno == EINTR)
> > +		ret = 0;
> > +
> > + close_all:
> > +	close(brass[0]);
> > +	close(brass[1]);
> > + close_file:
> > +	close(in_fd);
> > +
> > +	return ret ? ret : bread;
> > +}
> > +
> > +/**
> > + * tracefs_trace_pipe_print - redirect the stream of trace data to "stdout".
> > + * The "splice" system call is used to moves the data without copying
> > + * between kernel address space and user address space.
> > + * @instance: ftrace instance, can be NULL for top tracing instance.
> > + *
> > + * Returns -1 in case of an error or number of bytes transferred otherwise.
> > + */
> > +
> > +ssize_t tracefs_trace_pipe_print(struct tracefs_instance *instance)
> > +{
> > +	return tracefs_trace_pipe_stream(STDOUT_FILENO, instance,
> > +					 SPLICE_F_NONBLOCK);
> > +}
> > +  
> 
> In fact I want to use this function in "blocking" mode. I would like to keep listening for new trace data until the user 
> presses 'Ctrl-c'. Can we make this function to take a second parameter that will be the splice flags?

Yeah, I was going to ask you if you wanted this to have flags too.

I was also thinking that we should use the O_ flags and not the
SPLICE_F_ flags, as splice is more niche, and O_ is POSIX. We can check
the flags and change them for splice inside these functions.

Are you OK with that?

-- Steve


> 
> Thanks a lot!
> Yordan
> 

Re: [PATCH v5] libtracefs: Add APIs for data streaming

[Yordan Karadzhov]

On 28.06.21 г. 16:13, Steven Rostedt wrote:

> Are you OK with that?

Yes, with the minor corrections I sent, everything seems to work fine on my side.

Please push upstream, so that I can send the trace-cruncher patches.

Thanks!
Yordan

Re: [PATCH v5] libtracefs: Add APIs for data streaming

[Steven Rostedt]

On Mon, 28 Jun 2021 11:58:10 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> Nit: When looping you check the value of 'ret' 3 times.
>       The same can be done with only 2 checks.
> 
> 	while (*(volatile bool *)keep_going) {
> 		ret = read(in_fd, buf, BUFSIZ);
> 		if (ret <= 0)
> 			break;
> 
> 		ret = write(out_fd, buf, ret);
> 		if (ret <= 0)
> 			break;
> 
> 		bread += ret;
> 	}

I ended up doing it this way:

	while (*(volatile bool *)keep_going) {
		int r;
		ret = read(in_fd, buf, BUFSIZ);
		if (ret <= 0)
			break;
		r = ret;
		ret = write(out_fd, buf, r);
		if (ret < 0)
			break;
		bread += ret;
		/* Stop if we can't write what was read */
		if (ret < r)
			break;
	}


Because if it can't write the amount that was read, it shouldn't
continue any more. And since the documentation states, it returns what
was transferred, we should only return the amount that was written. Of
course, then we get stuck with a case that the buffer was read, and
lost. But if the write fails to write everything given to it, there's
probably other issues with the system (target ran out of disk space?).
(Hmm, I may state the above in a comment there).

Anyway, I'll be posting the v6 soon.

-- Steve