linux-kernel - Re: [RFC 5/7] block: export blkdev_atomic_write

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: <cc43b1ba-e9ea-4ff1-b616-be3c11960eea@oracle.com>
Date:   Fri, 1 Dec 2023 10:47:59 +0000
From:   John Garry <john.g.garry@...cle.com>
To:     Ojaswin Mujoo <ojaswin@...ux.ibm.com>, linux-ext4@...r.kernel.org,
        Theodore Ts'o <tytso@....edu>
Cc:     Ritesh Harjani <ritesh.list@...il.com>,
        linux-kernel@...r.kernel.org,
        "Darrick J . Wong" <djwong@...nel.org>,
        linux-block@...r.kernel.org, linux-xfs@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, dchinner@...hat.com
Subject: Re: [RFC 5/7] block: export blkdev_atomic_write_valid() and refactor
 api

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@...ux.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;