From: Namhyung Kim <namhyung.kim@lge.com>
To: Tejun Heo <tj@kernel.org>
Cc: Namhyung Kim <namhyung.kim@lge.com>,
axboe@kernel.dk, mingo@redhat.com, rostedt@goodmis.org,
fweisbec@gmail.com, teravest@google.com, slavapestov@google.com,
ctalbott@google.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 03/11] block: block_bio_complete tracepoint was missing
Date: Mon, 09 Jan 2012 11:33:27 +0900 [thread overview]
Message-ID: <4F0A51F7.9040206@lge.com> (raw)
In-Reply-To: <20120109014949.GA16360@mtj.dyndns.org>
Hi,
2012-01-09 10:49 AM, Tejun Heo wrote:
> Hello,
>
> On Mon, Jan 09, 2012 at 10:30:06AM +0900, Namhyung Kim wrote:
>> Just adding the TP unconditionally will produce duplicated (in some
>> sense) reports about the completion. For example, normal request
>> based IO reports whole request completion prior to its bio's, and
>> further
>
> Request and bio completions are separate events. There's nothing
> wrong with reporting them separately. In fact, I think they should be
> reported separately.
>
>> , some of nested block IO handling routines - bounced bio and
>> btrfs with compression, etc - call bio_endio() more than once. Also
>> there are cases that bio fails before it's enqueued for some reason.
>
> They are actually separate bio's being completed. I don't think
> trying to put extra semantics on TP itself is a good idea. In
> general, TP signals that such event happened with sufficient
> information and it's the consumers' responsibility to make sense of
> what's going on. BIO_CLONED/BOUNCED are there.
I see.
>> I have no idea about the ioblame can deal with all of such corner
>> cases. However it might confuse blktrace somewhat, I guess.
>
> Unless someone is doing memcpy() on bio's, ioblame should be okay. It
> only considers bio's which went through actual submission.
>
>> I already posted similar patch a couple of weeks ago, but didn't
>> receive a comment yet. [1] Please take a look this too :)
>
> I'll reply there but don't think imposing such extra logic on TP is a
> good idea.
I'll reply on that thread too. :)
>> After a quick glance, the ioblame seems to carry all IO accounting
>> info through the first bio in the request. If so, why don't you use
>> the request structure for that?
>
> Because there are bio based drivers which don't use requests at all.
What I thought for such drivers was dynamic allocation in their
->make_request_fn, but because we don't have a block_bio_issue TP,
Nevermind. :)
Thanks,
Namhyung Kim
next prev parent reply other threads:[~2012-01-09 2:33 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-05 23:42 [RFC PATCHSET RESEND] ioblame: statistical IO analyzer Tejun Heo
2012-01-05 23:42 ` [PATCH 01/11] trace_event_filter: factorize filter creation Tejun Heo
2012-01-05 23:42 ` [PATCH 02/11] trace_event_filter: add trace_event_filter_*() interface Tejun Heo
2012-01-05 23:42 ` [PATCH 03/11] block: block_bio_complete tracepoint was missing Tejun Heo
2012-01-09 1:30 ` Namhyung Kim
2012-01-09 1:49 ` Tejun Heo
2012-01-09 2:33 ` Namhyung Kim [this message]
2012-01-05 23:42 ` [PATCH 04/11] block: add @req to bio_{front|back}_merge tracepoints Tejun Heo
2012-01-05 23:42 ` [PATCH 05/11] block: abstract disk iteration into disk_iter Tejun Heo
2012-01-05 23:42 ` [PATCH 06/11] writeback: move struct wb_writeback_work to writeback.h Tejun Heo
2012-01-05 23:42 ` [PATCH 07/11] writeback: add more tracepoints Tejun Heo
2012-01-05 23:42 ` [PATCH 08/11] block: add block_touch_buffer tracepoint Tejun Heo
2012-01-05 23:42 ` [PATCH 09/11] vfs: add fcheck tracepoint Tejun Heo
2012-01-05 23:42 ` [PATCH 10/11] stacktrace: implement save_stack_trace_quick() Tejun Heo
2012-01-05 23:42 ` [PATCH 11/11] block, trace: implement ioblame IO statistical analyzer Tejun Heo
2012-01-06 9:00 ` [RFC PATCHSET RESEND] ioblame: statistical IO analyzer Namhyung Kim
2012-01-06 16:02 ` Tejun Heo
2012-01-06 16:33 ` Tejun Heo
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=4F0A51F7.9040206@lge.com \
--to=namhyung.kim@lge.com \
--cc=axboe@kernel.dk \
--cc=ctalbott@google.com \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
--cc=slavapestov@google.com \
--cc=teravest@google.com \
--cc=tj@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.