Re: [PATCH] block: improve error checking in blkdev_bio_end_io_async()

[Pavel Begunkov <asml.silence@gmail.com>]

On 10/19/21 19:59, Jens Axboe wrote:
> Track the current error status of the dio with DIO_ERROR in the flags,
> which can then avoid diving into dio->bio for the fast path of not
> having any errors. This reduces the overhead of the function nicely,
> which was previously dominated by this seemingly cheap check:
> 
>       4.55%     -1.13%  [kernel.vmlinux]  [k] blkdev_bio_end_io_async

Jens, something gone wrong here. blkdev_bio_end_io_async() is a
function from my not yet published branch, the perf here is for it,
but the patch tackles blkdev_bio_end_io().

P.s. once blkdev_bio_end_io_async() doesn't need it, and once in
normal blkdev_bio_end_io() will be a slow-ish path, so probably
we don't care much.


> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> ---
> 
> diff --git a/block/fops.c b/block/fops.c
> index d4f4fffb7d32..21d265caecff 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -127,6 +127,7 @@ enum {
>   	DIO_MULTI_BIO		= 1,
>   	DIO_SHOULD_DIRTY	= 2,
>   	DIO_IS_SYNC		= 4,
> +	DIO_ERROR		= 8,
>   };
>   
>   struct blkdev_dio {
> @@ -147,8 +148,10 @@ static void blkdev_bio_end_io(struct bio *bio)
>   	struct blkdev_dio *dio = bio->bi_private;
>   	unsigned int flags = dio->flags;
>   
> -	if (bio->bi_status && !dio->bio.bi_status)
> +	if (!(flags & DIO_ERROR) && !dio->bio.bi_status) {
>   		dio->bio.bi_status = bio->bi_status;
> +		flags |= DIO_ERROR;
> +	}
>   
>   	if (!(flags & DIO_MULTI_BIO) || atomic_dec_and_test(&dio->ref)) {
>   		if (!(flags & DIO_IS_SYNC)) {
> @@ -157,7 +160,7 @@ static void blkdev_bio_end_io(struct bio *bio)
>   
>   			WRITE_ONCE(iocb->private, NULL);
>   
> -			if (likely(!dio->bio.bi_status)) {
> +			if (likely(!(flags & DIO_ERROR))) {
>   				ret = dio->size;
>   				iocb->ki_pos += ret;
>   			} else {
> 

-- 
Pavel Begunkov