Re: [PATCH 03/11] block: block_bio_complete tracepoint was missing

[Namhyung Kim]

2012-01-06 8:42 AM, Tejun Heo wrote:
> block_bio_complete tracepoint was defined but not invoked anywhere.
> Fix it.
>
> Signed-off-by: Tejun Heo<tj@xxxxxxxxxx>
> ---
>   fs/bio.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/fs/bio.c b/fs/bio.c
> index b1fe82c..96548da 100644
> --- a/fs/bio.c
> +++ b/fs/bio.c
> @@ -1447,6 +1447,9 @@ void bio_endio(struct bio *bio, int error)
>   	else if (!test_bit(BIO_UPTODATE,&bio->bi_flags))
>   		error = -EIO;
>
> +	if (bio->bi_bdev)
> +		trace_block_bio_complete(bdev_get_queue(bio->bi_bdev),
> +					 bio, error);
>   	if (bio->bi_end_io)
>   		bio->bi_end_io(bio, error);
>   }

Hi,

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, 
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.

I have no idea about the ioblame can deal with all of such corner cases. 
However it might confuse blktrace somewhat, I guess.

I already posted similar patch a couple of weeks ago, but didn't receive 
a comment yet. [1] Please take a look this 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?


Thanks,
Namhyung Kim


[1] https://lkml.org/lkml/2011/12/27/111
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/