linux-kernel - Re: [PATCH 03/11] block: block_bio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4F0A431E.3090106@lge.com>
Date:	Mon, 09 Jan 2012 10:30:06 +0900
From:	Namhyung Kim <namhyung.kim@....com>
To:	Tejun Heo <tj@...nel.org>
CC:	axboe@...nel.dk, mingo@...hat.com, rostedt@...dmis.org,
	fweisbec@...il.com, teravest@...gle.com, slavapestov@...gle.com,
	ctalbott@...gle.com, dsharp@...gle.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/11] block: block_bio_complete tracepoint was missing

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@...nel.org>
> ---
>   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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/