* [PATCH v2] libtracefs: Add APIs for data streaming
@ 2021-06-23 12:05 Yordan Karadzhov (VMware)
2021-06-23 12:45 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Yordan Karadzhov (VMware) @ 2021-06-23 12:05 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Yordan Karadzhov (VMware)
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>
---
Changes in v2:
- The setting of the signal disposition is removed from the library
and is now responsibility of the caller. This in fact gives more
options for the user.
- The example provided in the documentation is updated accordingly.
Documentation/libtracefs-stream.txt | 106 ++++++++++++++++++++++++++++
include/tracefs-local.h | 1 +
include/tracefs.h | 5 ++
src/tracefs-tools.c | 98 +++++++++++++++++++++++++
4 files changed, 210 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..24c2e47
--- /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>*
+
+int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
+int 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 0 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, 0);
+ 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..833f136 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;
+ int pipe_keep_going;
};
extern pthread_mutex_t toplevel_lock;
diff --git a/include/tracefs.h b/include/tracefs.h
index e29b550..3c6741b 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);
+int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
+int 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..0c0c44f 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -912,3 +912,101 @@ int tracefs_function_notrace(struct tracefs_instance *instance, const char *filt
tracefs_put_tracing_file(filter_path);
return ret;
}
+
+static int 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 0 otherwise.
+ */
+int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
+ int flags)
+{
+ int *keep_going = instance ? &instance->pipe_keep_going :
+ &top_pipe_keep_going;
+ const char *file = "trace_pipe";
+ int brass[2], in_fd, ret = -1;
+ off_t data_size;
+
+ in_fd = tracefs_instance_file_open(instance, file, O_RDONLY);
+ 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;
+ }
+
+ *keep_going = true;
+ while (*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;
+ }
+
+ /*
+ * Do not return error in the case when the "splice" system call
+ * was interrupted by the user (pressing Ctrl-c).
+ */
+ if (!keep_going)
+ ret = 0;
+
+ close_all:
+ close(brass[0]);
+ close(brass[1]);
+ close_file:
+ close(in_fd);
+
+ return ret;
+}
+
+/**
+ * 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 0 otherwise.
+ */
+
+int tracefs_trace_pipe_print(struct tracefs_instance *instance)
+{
+ return tracefs_trace_pipe_stream(STDOUT_FILENO,
+ instance,
+ SPLICE_F_MORE | SPLICE_F_MOVE);
+}
+
+/**
+ * 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.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] libtracefs: Add APIs for data streaming
2021-06-23 12:05 [PATCH v2] libtracefs: Add APIs for data streaming Yordan Karadzhov (VMware)
@ 2021-06-23 12:45 ` Steven Rostedt
2021-06-23 13:02 ` Yordan Karadzhov
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2021-06-23 12:45 UTC (permalink / raw)
To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel
On Wed, 23 Jun 2021 15:05:26 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 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>
> ---
>
> Changes in v2:
> - The setting of the signal disposition is removed from the library
> and is now responsibility of the caller. This in fact gives more
> options for the user.
> - The example provided in the documentation is updated accordingly.
>
> Documentation/libtracefs-stream.txt | 106 ++++++++++++++++++++++++++++
> include/tracefs-local.h | 1 +
> include/tracefs.h | 5 ++
> src/tracefs-tools.c | 98 +++++++++++++++++++++++++
> 4 files changed, 210 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..24c2e47
> --- /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>*
> +
> +int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
> +int 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 0 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, 0);
> + 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..833f136 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;
> + int pipe_keep_going;
> };
>
> extern pthread_mutex_t toplevel_lock;
> diff --git a/include/tracefs.h b/include/tracefs.h
> index e29b550..3c6741b 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);
>
> +int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
> +int 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..0c0c44f 100644
> --- a/src/tracefs-tools.c
> +++ b/src/tracefs-tools.c
> @@ -912,3 +912,101 @@ int tracefs_function_notrace(struct tracefs_instance *instance, const char *filt
> tracefs_put_tracing_file(filter_path);
> return ret;
> }
> +
> +static int 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 0 otherwise.
> + */
> +int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
> + int flags)
> +{
> + int *keep_going = instance ? &instance->pipe_keep_going :
> + &top_pipe_keep_going;
I'm thinking we should invert the above. That is, have a stop instead of
"keep_going", where it is false by default (when the instance is created).
That's because, if we start here, and the SIGINT comes in before we get
here, it keep_going might get set to false, and missed.
> + const char *file = "trace_pipe";
> + int brass[2], in_fd, ret = -1;
> + off_t data_size;
> +
> + in_fd = tracefs_instance_file_open(instance, file, O_RDONLY);
> + 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;
> + }
> +
> + *keep_going = true;
Then we get here and we set it back to true.
Perhaps we should have a:
instance->pipe_stop
flag that gets set by the user.
And instead of setting the variable here, add a,
tracefs_trace_pipe_start(instance)
to match the tracefs_trace_pipe_stop().
If the user stops it, we should make it so that they need to start it again.
-- Steve
> + while (*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;
> + }
> +
> + /*
> + * Do not return error in the case when the "splice" system call
> + * was interrupted by the user (pressing Ctrl-c).
> + */
> + if (!keep_going)
> + ret = 0;
> +
> + close_all:
> + close(brass[0]);
> + close(brass[1]);
> + close_file:
> + close(in_fd);
> +
> + return ret;
> +}
> +
> +/**
> + * 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 0 otherwise.
> + */
> +
> +int tracefs_trace_pipe_print(struct tracefs_instance *instance)
> +{
> + return tracefs_trace_pipe_stream(STDOUT_FILENO,
> + instance,
> + SPLICE_F_MORE | SPLICE_F_MOVE);
> +}
> +
> +/**
> + * 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;
> +}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] libtracefs: Add APIs for data streaming
2021-06-23 12:45 ` Steven Rostedt
@ 2021-06-23 13:02 ` Yordan Karadzhov
2021-06-23 13:19 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Yordan Karadzhov @ 2021-06-23 13:02 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel
On 23.06.21 г. 15:45, Steven Rostedt wrote:
> On Wed, 23 Jun 2021 15:05:26 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
>
>> 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>
>> ---
>>
>> Changes in v2:
>> - The setting of the signal disposition is removed from the library
>> and is now responsibility of the caller. This in fact gives more
>> options for the user.
>> - The example provided in the documentation is updated accordingly.
>>
>> Documentation/libtracefs-stream.txt | 106 ++++++++++++++++++++++++++++
>> include/tracefs-local.h | 1 +
>> include/tracefs.h | 5 ++
>> src/tracefs-tools.c | 98 +++++++++++++++++++++++++
>> 4 files changed, 210 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..24c2e47
>> --- /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>*
>> +
>> +int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
>> +int 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 0 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, 0);
>> + 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..833f136 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;
>> + int pipe_keep_going;
>> };
>>
>> extern pthread_mutex_t toplevel_lock;
>> diff --git a/include/tracefs.h b/include/tracefs.h
>> index e29b550..3c6741b 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);
>>
>> +int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance, int flags);
>> +int 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..0c0c44f 100644
>> --- a/src/tracefs-tools.c
>> +++ b/src/tracefs-tools.c
>> @@ -912,3 +912,101 @@ int tracefs_function_notrace(struct tracefs_instance *instance, const char *filt
>> tracefs_put_tracing_file(filter_path);
>> return ret;
>> }
>> +
>> +static int 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 0 otherwise.
>> + */
>> +int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
>> + int flags)
>> +{
>> + int *keep_going = instance ? &instance->pipe_keep_going :
>> + &top_pipe_keep_going;
>
> I'm thinking we should invert the above. That is, have a stop instead of
> "keep_going", where it is false by default (when the instance is created).
>
> That's because, if we start here, and the SIGINT comes in before we get
> here, it keep_going might get set to false, and missed.
I intentionally tried to avoid having any dependency of the initialization value of the "keep_going" flag. We have to
consider the case when the user creates one instance and then calls this function multiple times in a row.
>
>> + const char *file = "trace_pipe";
>> + int brass[2], in_fd, ret = -1;
>> + off_t data_size;
>> +
>> + in_fd = tracefs_instance_file_open(instance, file, O_RDONLY);
>> + 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;
>> + }
>> +
>> + *keep_going = true;
>
> Then we get here and we set it back to true.
>
> Perhaps we should have a:
>
> instance->pipe_stop
>
> flag that gets set by the user.
>
> And instead of setting the variable here, add a,
>
> tracefs_trace_pipe_start(instance)
>
> to match the tracefs_trace_pipe_stop().
>
> If the user stops it, we should make it so that they need to start it again.
I do not understand your point with the "start" function. tracefs_trace_pipe_stream() itself is the start.
Thanks!
Yordan
>
> -- Steve
>
>
>> + while (*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;
>> + }
>> +
>> + /*
>> + * Do not return error in the case when the "splice" system call
>> + * was interrupted by the user (pressing Ctrl-c).
>> + */
>> + if (!keep_going)
>> + ret = 0;
>> +
>> + close_all:
>> + close(brass[0]);
>> + close(brass[1]);
>> + close_file:
>> + close(in_fd);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * 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 0 otherwise.
>> + */
>> +
>> +int tracefs_trace_pipe_print(struct tracefs_instance *instance)
>> +{
>> + return tracefs_trace_pipe_stream(STDOUT_FILENO,
>> + instance,
>> + SPLICE_F_MORE | SPLICE_F_MOVE);
>> +}
>> +
>> +/**
>> + * 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;
>> +}
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] libtracefs: Add APIs for data streaming
2021-06-23 13:02 ` Yordan Karadzhov
@ 2021-06-23 13:19 ` Steven Rostedt
2021-06-23 13:23 ` Yordan Karadzhov
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2021-06-23 13:19 UTC (permalink / raw)
To: Yordan Karadzhov; +Cc: linux-trace-devel
On Wed, 23 Jun 2021 16:02:00 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:
> > I'm thinking we should invert the above. That is, have a stop instead of
> > "keep_going", where it is false by default (when the instance is created).
> >
> > That's because, if we start here, and the SIGINT comes in before we get
> > here, it keep_going might get set to false, and missed.
>
> I intentionally tried to avoid having any dependency of the initialization value of the "keep_going" flag. We have to
> consider the case when the user creates one instance and then calls this function multiple times in a row.
If anything then, make it the first thing that gets done, and not just
before the loop.
That is:
+int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
+ int flags)
+{
+ int *keep_going = instance ? &instance->pipe_keep_going :
+ &top_pipe_keep_going;
+ const char *file = "trace_pipe";
+ int brass[2], in_fd, ret = -1;
+ off_t data_size;
*keep_going = true;
+
+ in_fd = tracefs_instance_file_open(instance, file, O_RDONLY);
+ 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;
+ }
+
+ *keep_going = true;
And not here.
+ while (*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;
+ }
And I think we need to make it volatile, otherwise the compiler is free to
ignore it. Because the compiler does not need to know about threads.
*keep_going = true;
while (*keep_going) {
[ do something ]
}
To the compiler that is the same as:
while (1) {
[ do something ]
}
And is free to make that change when optimizing.
What needs to be done is:
(*(volatile bool *)keep_going) = true;
and
while (*(volatile bool *)keep_going) {
That way the compiler knows that the value can change from outside its
knowledge.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] libtracefs: Add APIs for data streaming
2021-06-23 13:19 ` Steven Rostedt
@ 2021-06-23 13:23 ` Yordan Karadzhov
0 siblings, 0 replies; 5+ messages in thread
From: Yordan Karadzhov @ 2021-06-23 13:23 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel
On 23.06.21 г. 16:19, Steven Rostedt wrote:
> On Wed, 23 Jun 2021 16:02:00 +0300
> Yordan Karadzhov <y.karadz@gmail.com> wrote:
>
>>> I'm thinking we should invert the above. That is, have a stop instead of
>>> "keep_going", where it is false by default (when the instance is created).
>>>
>>> That's because, if we start here, and the SIGINT comes in before we get
>>> here, it keep_going might get set to false, and missed.
>>
>> I intentionally tried to avoid having any dependency of the initialization value of the "keep_going" flag. We have to
>> consider the case when the user creates one instance and then calls this function multiple times in a row.
>
> If anything then, make it the first thing that gets done, and not just
> before the loop.
>
> That is:
>
>
> +int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
> + int flags)
> +{
> + int *keep_going = instance ? &instance->pipe_keep_going :
> + &top_pipe_keep_going;
> + const char *file = "trace_pipe";
> + int brass[2], in_fd, ret = -1;
> + off_t data_size;
>
> *keep_going = true;
>
> +
> + in_fd = tracefs_instance_file_open(instance, file, O_RDONLY);
> + 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;
> + }
> +
> + *keep_going = true;
>
> And not here.
>
> + while (*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;
> + }
>
>
> And I think we need to make it volatile, otherwise the compiler is free to
> ignore it. Because the compiler does not need to know about threads.
>
> *keep_going = true;
> while (*keep_going) {
> [ do something ]
> }
>
> To the compiler that is the same as:
>
> while (1) {
> [ do something ]
> }
>
> And is free to make that change when optimizing.
>
> What needs to be done is:
>
> (*(volatile bool *)keep_going) = true;
>
> and
>
> while (*(volatile bool *)keep_going) {
>
> That way the compiler knows that the value can change from outside its
> knowledge.
>
OK, I will make the changes and will send v3.
Thanks a lot!
Yordan
> -- Steve
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-23 13:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23 12:05 [PATCH v2] libtracefs: Add APIs for data streaming Yordan Karadzhov (VMware)
2021-06-23 12:45 ` Steven Rostedt
2021-06-23 13:02 ` Yordan Karadzhov
2021-06-23 13:19 ` Steven Rostedt
2021-06-23 13:23 ` Yordan Karadzhov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.