Re: [PATCHv4 5/9] block: add a helper function for dio alignment

Damien Le Moal
2022-05-26T01:54:10Z

On 2022/05/26 10:06, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> This will make it easier to add more complex acceptable alignment
> criteria in the future.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> block/fops.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/block/fops.c b/block/fops.c
> index b9b83030e0df..bd6c2e13a4e3 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -42,6 +42,16 @@ static unsigned int dio_bio_write_op(struct kiocb *iocb)
> return op;
> }
>
> +static int blkdev_dio_aligned(struct block_device *bdev, loff_t pos,
> + struct iov_iter *iter)
> +{
> + if ((pos | iov_iter_alignment(iter)) &
> + (bdev_logical_block_size(bdev) - 1))
> + return -EINVAL;
> +
> + return 0;
> +}

Nit: The name of this helper really suggest a bool return, which would be a more
flexible interface I think:

if (!blkdev_dio_aligned(bdev, pos, iter))
return -EINVAL; <-- or whatever error code the caller wants.

And that will allow you to get rid of the ret variable in a least
__blkdev_direct_IO_async.

> +
> #define DIO_INLINE_BIO_VECS 4
>
> static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb,
> @@ -54,9 +64,9 @@ static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb,
> struct bio bio;
> ssize_t ret;
>
> - if ((pos | iov_iter_alignment(iter)) &
> - (bdev_logical_block_size(bdev) - 1))
> - return -EINVAL;
> + ret = blkdev_dio_aligned(bdev, pos, iter);
> + if (ret)
> + return ret;
>
> if (nr_pages <= DIO_INLINE_BIO_VECS)
> vecs = inline_vecs;
> @@ -171,11 +181,11 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
> bool is_read = (iov_iter_rw(iter) == READ), is_sync;
> unsigned int opf = is_read ? REQ_OP_READ : dio_bio_write_op(iocb);
> loff_t pos = iocb->ki_pos;
> - int ret = 0;
> + int ret;
>
> - if ((pos | iov_iter_alignment(iter)) &
> - (bdev_logical_block_size(bdev) - 1))
> - return -EINVAL;
> + ret = blkdev_dio_aligned(bdev, pos, iter);
> + if (ret)
> + return ret;
>
> if (iocb->ki_flags & IOCB_ALLOC_CACHE)
> opf |= REQ_ALLOC_CACHE;
> @@ -296,11 +306,11 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
> struct blkdev_dio *dio;
> struct bio *bio;
> loff_t pos = iocb->ki_pos;
> - int ret = 0;
> + int ret;
>
> - if ((pos | iov_iter_alignment(iter)) &
> - (bdev_logical_block_size(bdev) - 1))
> - return -EINVAL;
> + ret = blkdev_dio_aligned(bdev, pos, iter);
> + if (ret)
> + return ret;
>
> if (iocb->ki_flags & IOCB_ALLOC_CACHE)
> opf |= REQ_ALLOC_CACHE;

--
Damien Le Moal
Western Digital Research