From: Jens Axboe <jens.axboe@oracle.com>
To: Arnaldo Carvalho de Melo <acme@redhat.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 14:56:39 +0100 [thread overview]
Message-ID: <20090325135639.GU27476@kernel.dk> (raw)
In-Reply-To: <20090325135142.GD10928@ghostprotocols.net>
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.
--
Jens Axboe
next prev parent reply other threads:[~2009-03-25 13:56 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 [this message]
2009-03-25 14:07 ` Arnaldo Carvalho de Melo
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=20090325135639.GU27476@kernel.dk \
--to=jens.axboe@oracle.com \
--cc=acme@redhat.com \
--cc=fweisbec@gmail.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