On Thu, Mar 23, 2017 at 05:29:02PM +1100, NeilBrown wrote:
>
> Currently only dm and md/raid5 bios trigger trace_block_bio_complete().
> Now that we have bio_chain(), it is not possible, in general, for a
> driver to know when the bio is really complete. Only bio_endio()
> knows that.
>
> So move the trace_block_bio_complete() call to bio_endio().
>
> Now trace_block_bio_complete() pairs with trace_block_bio_queue().
> Any bio for which a 'queue' event is traced, will subsequently
> generate a 'complete' event.
>
> There are a few cases where completion tracing is not wanted.
> 1/ If blk_update_request() has already generated a completion
> trace event at the 'request' level, there is no point generating
> one at the bio level too. In this case the bi_sector and bi_size
> will have changed, so the bio level event would be wrong
>
> 2/ If the bio hasn't actually been queued yet, but is being aborted
> early, then a trace event could be confusing. Some filesystems
> call bio_endio() and will need to use a different interface to
> avoid tracing
>
> 3/ The bio_integrity code interposes itself by replacing bi_end_io,
> then restores it and calls bio_endio() again. This would produce
> two identical trace events if left like that.
>
> To handle these, we provide bio_endio_notrace(). This patch only adds
> uses of this in core code. Separate patches will be needed to update
> the filesystems to avoid tracing.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxxx>
> ---
> block/bio-integrity.c | 4 ++--
> block/bio.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> block/blk-core.c | 2 +-
> drivers/md/dm.c | 1 -
> drivers/md/raid5.c | 8 --------
> include/linux/bio.h | 1 +
> 6 files changed, 50 insertions(+), 12 deletions(-)
>
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index 5384713d48bc..28581e2f68fb 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -370,7 +370,7 @@ static void bio_integrity_verify_fn(struct work_struct *work)
>
> /* Restore original bio completion handler */
> bio->bi_end_io = bip->bip_end_io;
> - bio_endio(bio);
> + bio_endio_notrace(bio);
> }
>
> /**
> @@ -397,7 +397,7 @@ void bio_integrity_endio(struct bio *bio)
> */
> if (bio->bi_error) {
> bio->bi_end_io = bip->bip_end_io;
> - bio_endio(bio);
> + bio_endio_notrace(bio);
>
> return;
> }
> diff --git a/block/bio.c b/block/bio.c
> index 5eec5e08417f..c8e5d24abd52 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1811,6 +1811,45 @@ static inline bool bio_remaining_done(struct bio *bio)
> }
>
> /**
> + * bio_endio_notrace - end I/O on a bio without tracing
> + * @bio: bio
> + *
> + * Description:
> + * bio_endio_notrace() will end I/O on the whole bio.
> + * bio_endio_notrace() should only be call if a completion trace
> + * event is not needed. This can be the case if a request-level
> + * completion event has already been generated, if the bio is
> + * being completed early, before it was even queued.
> + *
> + **/
> +void bio_endio_notrace(struct bio *bio)
> +{
> +again:
> + if (!bio_remaining_done(bio))
> + return;
> +
> + /*
> + * Need to have a real endio function for chained bios, otherwise
> + * various corner cases will break (like stacking block devices that
> + * save/restore bi_end_io) - however, we want to avoid unbounded
> + * recursion and blowing the stack. Tail call optimization would
> + * handle this, but compiling with frame pointers also disables
> + * gcc's sibling call optimization.
> + */
> + if (bio->bi_end_io == bio_chain_endio) {
> + bio = __bio_chain_endio(bio);
> + goto again;
> + }
> +
> + if (bio->bi_bdev)
> + trace_block_bio_complete(bdev_get_queue(bio->bi_bdev),
> + bio, bio->bi_error);
The notrace version still traces?
> + if (bio->bi_end_io)
> + bio->bi_end_io(bio);
> +}
> +EXPORT_SYMBOL(bio_endio_notrace);
It isn't a good idea to duplicate bio_endio here, and any change on
bio_endio() may be needed for this _notrace version too in future.
Thanks,
Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
