public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH trace-cmd 2/2] trace-recorder: better error handling during copy
Date: Wed, 23 Mar 2016 15:02:16 +0800	[thread overview]
Message-ID: <20160323070216.GN28183@pxdev.xzpeter.org> (raw)
In-Reply-To: <20160322091956.2c42eefe@gandalf.local.home>

On Tue, Mar 22, 2016 at 09:19:56AM -0400, Steven Rostedt wrote:
> On Tue, 22 Mar 2016 09:14:33 -0400
> Peter Xu <peterx@redhat.com> wrote:
> 
> > Currently we have two ways to copy data, one is splice, one is read +
> > write. For both, dump more information when we got errors during the
> > copy. Also, when we update_fd(), we should make sure all bytes written,
> > and update written bytes only.
> > 
> > These information might be important to better diagnose when the copy
> > got wrong, like no space error, or connection error when copying data to
> > remote sockets. In the past, we just got silence errors without notice.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  trace-recorder.c | 34 ++++++++++++++++++++++++----------
> >  1 file changed, 24 insertions(+), 10 deletions(-)
> > 
> > diff --git a/trace-recorder.c b/trace-recorder.c
> > index 49b04ea..7d6feb0 100644
> > --- a/trace-recorder.c
> > +++ b/trace-recorder.c
> > @@ -334,13 +334,14 @@ static inline void update_fd(struct tracecmd_recorder *recorder, int size)
> >   */
> >  static long splice_data(struct tracecmd_recorder *recorder)
> >  {
> > -	long ret;
> > +	long ret, written;
> >  
> >  	ret = splice(recorder->trace_fd, NULL, recorder->brass[1], NULL,
> >  		     recorder->page_size, 1 /* SPLICE_F_MOVE */);
> >  	if (ret < 0) {
> >  		if (errno != EAGAIN && errno != EINTR) {
> > -			warning("recorder error in splice input");
> > +			warning("recorder error in splice input: %s",
> > +				strerror(errno));
> 
> I'm wondering if we should add a "pwarning()" helper function that will
> do the stderror(error) for us.

Ah, I just found that __vwarning() will dump errno string, and
warning() is calling that. So it explained why I got one more line
when got errors... ;)

How about remove all the strerror() things, and just keep capturing
the written size? Let me send a v2 for this patch.

Thanks.

-- peterx

  reply	other threads:[~2016-03-23  7:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-22  7:40 [PATCH trace-cmd 0/2] better error handling during copy Peter Xu
2016-03-22  7:40 ` [PATCH trace-cmd 1/2] trace-cmd-listen: remove useless printf Peter Xu
2016-03-22 13:18   ` Steven Rostedt
2016-03-23  6:39     ` Peter Xu
2016-03-22  7:40 ` [PATCH trace-cmd 2/2] trace-recorder: better error handling during copy Peter Xu
2016-03-22 13:19   ` Steven Rostedt
2016-03-23  7:02     ` Peter Xu [this message]
2016-03-23  7:05     ` [PATCH trace-cmd v2] " Peter Xu

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=20160323070216.GN28183@pxdev.xzpeter.org \
    --to=peterx@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /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