Message-ID: <20090325140725.GE10928@ghostprotocols.net>
Date:	Wed, 25 Mar 2009 11:07:25 -0300
From:	Arnaldo Carvalho de Melo <acme@...hat.com>
To:	Jens Axboe <jens.axboe@...cle.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Li Zefan <lizf@...fujitsu.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] blktrace: fix the original blktrace

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@...cle.com> wrote:
> > > 
> > > > On Wed, Mar 25 2009, Ingo Molnar wrote:
> > > > > 
> > > > > * Li Zefan <lizf@...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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists