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

[Yordan Karadzhov <y.karadz@gmail.com>]

On 24.06.21 г. 19:45, Steven Rostedt wrote:
> On Thu, 24 Jun 2021 17:51:01 +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>
>>
> 
> So I did some investigations here, and found that splice() is not
> guaranteed to work on a terminal output.
> 
> To overcome this, I decided to have the print create another pipe, and
> pass that to the stream code. The if something was read, it would read
> the pipe.
> 
> It required changing the return of the functions to return the amount
> transferred, so that I can differentiate between "EOF" with nothing
> read, and something read. Because I couldn't get the pipe to not block
> on read if there was no content in it. :-/
> 
> Anyway, I submitted a v4 with the changes I made, and it appears to
> work. I haven't tested it that much, so it may still have bugs.
> 

Hi Steve,

I tested v4 of the patch that you submitted and it doesn't work correct in my use case. Also it seems greatly 
over-complicated to me. Note that when you call tracefs_trace_pipe_stream() inside a loop you will keep opening and 
closing the "trace_pipe" and the pipe over and over again.

See the example code below. Is this what you are aiming? Also is this guaranteed to always work?

Thanks!
Yordan


#define _GNU_SOURCE
#include <fcntl.h>
#include <unistd.h>
#include <signal.h>
#include <stdbool.h>
#include <stdio.h>
#include <errno.h>

volatile bool keep_going = true;

static void finish(int sig)
{
	keep_going = false;
}

int main(int argc, char **argv)
{
	int splice_flags = SPLICE_F_MORE | SPLICE_F_MOVE;
	int brass1[2], brass2[2], fd, ret;
	char buf[BUFSIZ];

	signal(SIGINT, finish);

	fd = open(argv[1], O_RDONLY);
	if (fd < 0) {
		perror("open");
		return 1;
	}

	if (pipe(brass1) < 0){
		perror("pipe A");
		return 1;
	}

	if (pipe(brass2) < 0){
		perror("pipe B");
		return 1;
	}

	errno = 0;
	while (keep_going) {
		ret = splice(fd, NULL,
			     brass1[1], NULL,
			     BUFSIZ, splice_flags);
		if (ret < 0) {
			perror("splice A");
			return 1;
		}
		ret = splice(brass1[0], NULL,
			     brass2[1], NULL,
			     BUFSIZ, splice_flags);
		if (ret < 0) {
			perror("splice B");
			return 1;
		}

		ret = read(brass2[0], buf, BUFSIZ);
		if (ret < 0) {
			perror("read");
			return 1;
		}

		ret = write(STDOUT_FILENO, buf, ret);
		if (ret < 0) {
			perror("write");
			return 1;
		}
	}

	close(fd);
	close(brass1[0]);
	close(brass1[1]);
	close(brass2[0]);
	close(brass2[1]);

	return 0;
}



> We could optimize it to just use the splice output first (stream
> first), and if that returns with EINVAL, which is the expected error on
> a non functioning stdout, we could then switch to this alternate method.
> 
> Although, the stream would have read some of the pipe and lost the
> data, which is not what we want, so a test of stdout would need to be
> made with splice first. :-/
> 
> -- Steve
>