Re: [RFC 5/7] block: export blkdev_atomic_write_valid() and refactor api

[John Garry <john.g.garry@oracle.com>]

On 30/11/2023 13:53, Ojaswin Mujoo wrote:
> Export the blkdev_atomic_write_valid() function so that other filesystems
> can call it as a part of validating the atomic write operation.
> 
> Further, refactor the api to accept a len argument instead of iov_iter to
> make it easier to call from other places.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

I was actually thinking of moving this functionality to vfs and maybe 
also calling earlier in write path, as the code is really common to 
blkdev and FSes.

However, Christoph Hellwig was not so happy about current interface with 
power-of-2 requirement et al, so I was going to wait until that 
discussion is concluded before deciding.

Thanks,
John

> ---
>   block/fops.c           | 18 ++++++++++--------
>   include/linux/blkdev.h |  2 ++
>   2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/block/fops.c b/block/fops.c
> index 516669ad69e5..5dae95c49720 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -41,8 +41,7 @@ static bool blkdev_dio_unaligned(struct block_device *bdev, loff_t pos,
>   		!bdev_iter_is_aligned(bdev, iter);
>   }
>   
> -static bool blkdev_atomic_write_valid(struct block_device *bdev, loff_t pos,
> -			      struct iov_iter *iter)
> +bool blkdev_atomic_write_valid(struct block_device *bdev, loff_t pos, size_t len)
>   {
>   	unsigned int atomic_write_unit_min_bytes =
>   			queue_atomic_write_unit_min_bytes(bdev_get_queue(bdev));
> @@ -53,16 +52,17 @@ static bool blkdev_atomic_write_valid(struct block_device *bdev, loff_t pos,
>   		return false;
>   	if (pos % atomic_write_unit_min_bytes)
>   		return false;
> -	if (iov_iter_count(iter) % atomic_write_unit_min_bytes)
> +	if (len % atomic_write_unit_min_bytes)
>   		return false;
> -	if (!is_power_of_2(iov_iter_count(iter)))
> +	if (!is_power_of_2(len))
>   		return false;
> -	if (iov_iter_count(iter) > atomic_write_unit_max_bytes)
> +	if (len > atomic_write_unit_max_bytes)
>   		return false;
> -	if (pos % iov_iter_count(iter))
> +	if (pos % len)
>   		return false;
>   	return true;
>   }
> +EXPORT_SYMBOL_GPL(blkdev_atomic_write_valid);
>   
>   #define DIO_INLINE_BIO_VECS 4
>   
> @@ -81,7 +81,8 @@ static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb,
>   	if (blkdev_dio_unaligned(bdev, pos, iter))
>   		return -EINVAL;
>   
> -	if (atomic_write && !blkdev_atomic_write_valid(bdev, pos, iter))
> +	if (atomic_write &&
> +	    !blkdev_atomic_write_valid(bdev, pos, iov_iter_count(iter)))
>   		return -EINVAL;
>   
>   	if (nr_pages <= DIO_INLINE_BIO_VECS)
> @@ -348,7 +349,8 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
>   	if (blkdev_dio_unaligned(bdev, pos, iter))
>   		return -EINVAL;
>   
> -	if (atomic_write && !blkdev_atomic_write_valid(bdev, pos, iter))
> +	if (atomic_write &&
> +	    !blkdev_atomic_write_valid(bdev, pos, iov_iter_count(iter)))
>   		return -EINVAL;
>   
>   	if (iocb->ki_flags & IOCB_ALLOC_CACHE)
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index f70988083734..5a3124fc191f 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1566,6 +1566,8 @@ static inline int early_lookup_bdev(const char *pathname, dev_t *dev)
>   int freeze_bdev(struct block_device *bdev);
>   int thaw_bdev(struct block_device *bdev);
>   
> +bool blkdev_atomic_write_valid(struct block_device *bdev, loff_t pos, size_t len);
> +
>   struct io_comp_batch {
>   	struct request *req_list;
>   	bool need_ts;