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

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

On 25.06.21 г. 16:46, Steven Rostedt wrote:
> On Fri, 25 Jun 2021 16:17:14 +0300
> Yordan Karadzhov <y.karadz@gmail.com> wrote:
> 
>> 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.
> 
> Good point. I could update it to pull out the main work of the splice
> logic and have both functions use that, and not have one call the other.
> 
> What didn't work with your use case?
> 
>>
>> See the example code below. Is this what you are aiming? Also is this guaranteed to always work?
> 
> Similar but I'll comment below. Note, the real issue I had in testing
> my code was blocking. The read on the pipe would block on EOF even with
> a NONBLOCK flag :-/ Which is not what I wanted, and to fix that, it
> made the code a bit more complex.
> 
>>
>> 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);
> 
> The above can block, and if it does, and you hit "ctrl-C" it will
> return with ret < 0 and have errno set to EINTR.
> 
>> 		if (ret < 0) {
> 
> With a Ctrl-C, this would error.

Yes, Isn't this what we want. We want to stop the loop.

Thanks!
Y.

> 
>> 			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);
> 
> Same here, with the blocking issue.
> 
> -- Steve
> 
>> 		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;
>> }