linux-kernel - Re: [PATCH 1/2] block: introduce BIO_IN

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: <20120109015628.GB16360@mtj.dyndns.org>
Date:	Sun, 8 Jan 2012 17:56:28 -0800
From:	Tejun Heo <tj@...nel.org>
To:	Namhyung Kim <namhyung@...il.com>
Cc:	Jens Axboe <axboe@...nel.dk>, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] block: introduce BIO_IN_FLIGHT flag

On Tue, Dec 27, 2011 at 11:28:32PM +0900, Namhyung Kim wrote:
> BIO_IN_FLIGHT flag is used for tracing block I/O completion.
> This patch fixes tracing bio-based devices - except DM which
> inserts completion tracepoint explicitly - that could not be
> traced such event using blktrace.
> 
> It won't affect tracing normal (request-based) disk devices
> and nested bio handling paths.
> 
> Signed-off-by: Namhyung Kim <namhyung@...il.com>
> ---
>  block/blk-core.c          |    5 +++++
>  fs/bio.c                  |    7 +++++++
>  include/linux/blk_types.h |    1 +
>  3 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 1b4fd93af2c0..f61310323954 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -165,6 +165,9 @@ static void req_bio_endio(struct request *rq, struct bio *bio,
>  	if (unlikely(rq->cmd_flags & REQ_QUIET))
>  		set_bit(BIO_QUIET, &bio->bi_flags);
>  
> +	/* completion event was already reported in blk_update_request */
> +	clear_bit(BIO_IN_FLIGHT, &bio->bi_flags);
> +
>  	bio->bi_size -= nbytes;
>  	bio->bi_sector += (nbytes >> 9);
>  
> @@ -1662,6 +1665,8 @@ void generic_make_request(struct bio *bio)
p>  	do {
>  		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
>  
> +		set_bit(BIO_IN_FLIGHT, &bio->bi_flags);
> +
>  		q->make_request_fn(q, bio);
>  
>  		bio = bio_list_pop(current->bio_list);
> diff --git a/fs/bio.c b/fs/bio.c
> index b1fe82cf88cf..10fb7a1d74ba 100644
> --- a/fs/bio.c
> +++ b/fs/bio.c
> @@ -1447,6 +1447,13 @@ void bio_endio(struct bio *bio, int error)
>  	else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
>  		error = -EIO;
>  
> +	if (test_bit(BIO_IN_FLIGHT, &bio->bi_flags)) {
> +		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
> +
> +		trace_block_bio_complete(q, bio, error);
> +		clear_bit(BIO_IN_FLIGHT, &bio->bi_flags);
> +	}

This really should be filtered from the consumer side.  Not on TP
itself.  Doing it like this makes the TP useless for other consumers
and adds unnecessary flag and operations on it even when the TP is not
in use.  Why not just trigger the TP on boi_endio() and let blktrace
probe filter out bounced or cloned completions?

Thanks.

--
tejun
--
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/