linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Yordan Karadzhov <y.karadz@gmail.com>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH v3] libtracefs: Add APIs for data streaming
Date: Fri, 25 Jun 2021 09:46:24 -0400	[thread overview]
Message-ID: <20210625094624.6da9a90f@oasis.local.home> (raw)
In-Reply-To: <835ff5f9-5fed-0dfb-2d75-dd9158fb63a7@gmail.com>

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.

> 			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;
> }

  reply	other threads:[~2021-06-25 13:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24 14:51 [PATCH v3] libtracefs: Add APIs for data streaming Yordan Karadzhov (VMware)
2021-06-24 16:45 ` Steven Rostedt
2021-06-25 13:17   ` Yordan Karadzhov
2021-06-25 13:46     ` Steven Rostedt [this message]
2021-06-25 13:57       ` Yordan Karadzhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210625094624.6da9a90f@oasis.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=y.karadz@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).