public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@redhat.com>
To: Jens Axboe <jens.axboe@oracle.com>
Cc: Ingo Molnar <mingo@elte.hu>, Li Zefan <lizf@cn.fujitsu.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] blktrace: fix the original blktrace
Date: Wed, 25 Mar 2009 11:07:25 -0300	[thread overview]
Message-ID: <20090325140725.GE10928@ghostprotocols.net> (raw)
In-Reply-To: <20090325135639.GU27476@kernel.dk>

Em Wed, Mar 25, 2009 at 02:56:39PM +0100, Jens Axboe escreveu:
> On Wed, Mar 25 2009, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Mar 25, 2009 at 12:47:20PM +0100, Ingo Molnar escreveu:
> > > 
> > > * Jens Axboe <jens.axboe@oracle.com> wrote:
> > > 
> > > > On Wed, Mar 25 2009, Ingo Molnar wrote:
> > > > > 
> > > > > * Li Zefan <lizf@cn.fujitsu.com> wrote:
> > > > > 
> > > > > > Currently the original blktrace, which is using relay and is used 
> > > > > > via ioctl, is broken. You can use ftrace to see the output of 
> > > > > > blktrace, but user-space blktrace is unusable.
> > > > > 
> > > > > good catch, thanks Li!
> > > > > 
> > > > > Jens, Arnaldo, do these fixes look good to you too?
> > > > 
> > > > Look fine, but I'm very worried about the state of the current 
> > > > code. I mean, this last round of fixes got the timestamp correct 
> > > > and made blktrace work again. [...]
> > > 
> > > Correct. I tested it on a 16way box (see the blkparse output below). 
> > > If you can see any sort of anomaly please let us know so we can fix 
> > > it.
> > > 
> > > > [...] Those are something that should have been caught even before 
> > > > the whole thing was posted, let alone merged. When do you plan on 
> > > > pushing this upstream? Looks like 2.6.31 to me, it's clearly not 
> > > > ready for 2.6.30 by any stretch.
> > > 
> > > Well, apparently nobody tried ioctl based blktrace+blkparse on -tip 
> > > or linux-next in the past ~1 month. The relayfs portion was (meant 
> > > to be) kept largely untouched but this bug still crept in.
> > > 
> > > Li started looking into it and found the bug. I tested the ftrace 
> > > plugin side regularly, but you are right that this bug took too long 
> > > to find (over a month) - from now on i'll check the ioctl side more 
> > > regularly too, for all tracing and relayfs changes as well.
> > 
> > I got sidetracked by other stuff and didn't managed to continue working
> > on it, but at the same time testing by somebody else is needed. There is
> > no bug-free code, let alone in the first iteration, even less for
> > something I wasn't familiar with and depended on people like Li to do a
> > careful review.
> > 
> > Li fixed bugs and continued the work, getting things I wanted working
> > and made the initial steps to get done, like being able to just fed
> > trace_pipe into blkparse and get the same result as btrace.
> > 
> > So I apologise for not having continued working on it more often to
> > catch myself the bugs I introduced, and give a big thank you to Li for
> > the thorough review, bugfixes and improvements.
> 
> Dont apologise, there's nothing wrong with that. The only thing wrong
> would be for the unfinished code to get merged, when it clearly wasn't
> ready.

So in this case I guess the right approach is to keep this on a -tip
branch and delay it getting merged to .31?

- Arnaldo

  reply	other threads:[~2009-03-25 14:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-25  9:18 [PATCH 1/3] blktrace: fix timestamp in binary output Li Zefan
2009-03-25  9:19 ` [PATCH 2/3] blktrace: fix a race when creating blk_tree_root in debugfs Li Zefan
2009-03-25 11:54   ` [tip:tracing/blktrace] " Li Zefan
2009-03-25  9:21 ` [PATCH 3/3] blktrace: fix the original blktrace Li Zefan
2009-03-25 10:14   ` Ingo Molnar
2009-03-25 10:17     ` Jens Axboe
2009-03-25 11:47       ` Ingo Molnar
2009-03-25 13:51         ` Arnaldo Carvalho de Melo
2009-03-25 13:56           ` Jens Axboe
2009-03-25 14:07             ` Arnaldo Carvalho de Melo [this message]
2009-03-26  2:13       ` Li Zefan
2009-03-26  8:29         ` Jens Axboe
2009-03-26 13:37           ` Ingo Molnar
2009-03-26 15:14             ` Arnaldo Carvalho de Melo
2009-03-26 15:44               ` Jens Axboe
2009-03-26 17:07                 ` Arnaldo Carvalho de Melo
2009-03-27  0:21                 ` Li Zefan
2009-03-25 11:54   ` [tip:tracing/blktrace] " Li Zefan
2009-03-25 11:54 ` [tip:tracing/blktrace] blktrace: fix timestamp in binary output Li Zefan

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=20090325140725.GE10928@ghostprotocols.net \
    --to=acme@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=mingo@elte.hu \
    --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