1. Patchew
  2. linux
  3. [PATCH v9 0/8] block atomic writes for xfs
  4. View patch

[PATCH v9 2/8] fs/block: Check for IOCB_DIRECT in generic_atomic_write_valid()

John Garry posted 8 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v9 2/8] fs/block: Check for IOCB_DIRECT in generic_atomic_write_valid()
Posted by John Garry 1 month, 1 week ago 
Currently FMODE_CAN_ATOMIC_WRITE is set if the bdev can atomic write and
the file is open for direct IO. This does not work if the file is not
opened for direct IO, yet fcntl(O_DIRECT) is used on the fd later.

Change to check for direct IO on a per-IO basis in
generic_atomic_write_valid(). Since we want to report -EOPNOTSUPP for
non-direct IO for an atomic write, change to return an error code.

Relocate the block fops atomic write checks to the common write path, as to
catch non-direct IO.

Fixes: c34fc6f26ab8 ("fs: Initial atomic write support")
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/fops.c       | 18 ++++++++++--------
 fs/read_write.c    | 13 ++++++++-----
 include/linux/fs.h |  2 +-
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 968b47b615c4..2d01c9007681 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -36,11 +36,8 @@ static blk_opf_t dio_bio_write_op(struct kiocb *iocb)
 }
 
 static bool blkdev_dio_invalid(struct block_device *bdev, struct kiocb *iocb,
-				struct iov_iter *iter, bool is_atomic)
+				struct iov_iter *iter)
 {
-	if (is_atomic && !generic_atomic_write_valid(iocb, iter))
-		return true;
-
 	return iocb->ki_pos & (bdev_logical_block_size(bdev) - 1) ||
 		!bdev_iter_is_aligned(bdev, iter);
 }
@@ -368,13 +365,12 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
 static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 {
 	struct block_device *bdev = I_BDEV(iocb->ki_filp->f_mapping->host);
-	bool is_atomic = iocb->ki_flags & IOCB_ATOMIC;
 	unsigned int nr_pages;
 
 	if (!iov_iter_count(iter))
 		return 0;
 
-	if (blkdev_dio_invalid(bdev, iocb, iter, is_atomic))
+	if (blkdev_dio_invalid(bdev, iocb, iter))
 		return -EINVAL;
 
 	nr_pages = bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS + 1);
@@ -383,7 +379,7 @@ static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 			return __blkdev_direct_IO_simple(iocb, iter, bdev,
 							nr_pages);
 		return __blkdev_direct_IO_async(iocb, iter, bdev, nr_pages);
-	} else if (is_atomic) {
+	} else if (iocb->ki_flags & IOCB_ATOMIC) {
 		return -EINVAL;
 	}
 	return __blkdev_direct_IO(iocb, iter, bdev, bio_max_segs(nr_pages));
@@ -625,7 +621,7 @@ static int blkdev_open(struct inode *inode, struct file *filp)
 	if (!bdev)
 		return -ENXIO;
 
-	if (bdev_can_atomic_write(bdev) && filp->f_flags & O_DIRECT)
+	if (bdev_can_atomic_write(bdev))
 		filp->f_mode |= FMODE_CAN_ATOMIC_WRITE;
 
 	ret = bdev_open(bdev, mode, filp->private_data, NULL, filp);
@@ -700,6 +696,12 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if ((iocb->ki_flags & (IOCB_NOWAIT | IOCB_DIRECT)) == IOCB_NOWAIT)
 		return -EOPNOTSUPP;
 
+	if (iocb->ki_flags & IOCB_ATOMIC) {
+		ret = generic_atomic_write_valid(iocb, from);
+		if (ret)
+			return ret;
+	}
+
 	size -= iocb->ki_pos;
 	if (iov_iter_count(from) > size) {
 		shorted = iov_iter_count(from) - size;
diff --git a/fs/read_write.c b/fs/read_write.c
index 2c3263530828..befec0b5c537 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1830,18 +1830,21 @@ int generic_file_rw_checks(struct file *file_in, struct file *file_out)
 	return 0;
 }
 
-bool generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter)
+int generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter)
 {
 	size_t len = iov_iter_count(iter);
 
 	if (!iter_is_ubuf(iter))
-		return false;
+		return -EINVAL;
 
 	if (!is_power_of_2(len))
-		return false;
+		return -EINVAL;
 
 	if (!IS_ALIGNED(iocb->ki_pos, len))
-		return false;
+		return -EINVAL;
 
-	return true;
+	if (!(iocb->ki_flags & IOCB_DIRECT))
+		return -EOPNOTSUPP;
+
+	return 0;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fbfa032d1d90..ba47fb283730 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3721,6 +3721,6 @@ static inline bool vfs_empty_path(int dfd, const char __user *path)
 	return !c;
 }
 
-bool generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter);
+int generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter);
 
 #endif /* _LINUX_FS_H */
-- 
2.31.1
Re: [PATCH v9 2/8] fs/block: Check for IOCB_DIRECT in generic_atomic_write_valid()
Posted by Darrick J. Wong 1 month, 1 week ago 
On Wed, Oct 16, 2024 at 10:03:19AM +0000, John Garry wrote:
> Currently FMODE_CAN_ATOMIC_WRITE is set if the bdev can atomic write and
> the file is open for direct IO. This does not work if the file is not
> opened for direct IO, yet fcntl(O_DIRECT) is used on the fd later.
> 
> Change to check for direct IO on a per-IO basis in
> generic_atomic_write_valid(). Since we want to report -EOPNOTSUPP for
> non-direct IO for an atomic write, change to return an error code.
> 
> Relocate the block fops atomic write checks to the common write path, as to
> catch non-direct IO.
> 
> Fixes: c34fc6f26ab8 ("fs: Initial atomic write support")
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: John Garry <john.g.garry@oracle.com>

Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  block/fops.c       | 18 ++++++++++--------
>  fs/read_write.c    | 13 ++++++++-----
>  include/linux/fs.h |  2 +-
>  3 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/block/fops.c b/block/fops.c
> index 968b47b615c4..2d01c9007681 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -36,11 +36,8 @@ static blk_opf_t dio_bio_write_op(struct kiocb *iocb)
>  }
>  
>  static bool blkdev_dio_invalid(struct block_device *bdev, struct kiocb *iocb,
> -				struct iov_iter *iter, bool is_atomic)
> +				struct iov_iter *iter)
>  {
> -	if (is_atomic && !generic_atomic_write_valid(iocb, iter))
> -		return true;
> -
>  	return iocb->ki_pos & (bdev_logical_block_size(bdev) - 1) ||
>  		!bdev_iter_is_aligned(bdev, iter);
>  }
> @@ -368,13 +365,12 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
>  static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>  {
>  	struct block_device *bdev = I_BDEV(iocb->ki_filp->f_mapping->host);
> -	bool is_atomic = iocb->ki_flags & IOCB_ATOMIC;
>  	unsigned int nr_pages;
>  
>  	if (!iov_iter_count(iter))
>  		return 0;
>  
> -	if (blkdev_dio_invalid(bdev, iocb, iter, is_atomic))
> +	if (blkdev_dio_invalid(bdev, iocb, iter))
>  		return -EINVAL;
>  
>  	nr_pages = bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS + 1);
> @@ -383,7 +379,7 @@ static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>  			return __blkdev_direct_IO_simple(iocb, iter, bdev,
>  							nr_pages);
>  		return __blkdev_direct_IO_async(iocb, iter, bdev, nr_pages);
> -	} else if (is_atomic) {
> +	} else if (iocb->ki_flags & IOCB_ATOMIC) {
>  		return -EINVAL;
>  	}
>  	return __blkdev_direct_IO(iocb, iter, bdev, bio_max_segs(nr_pages));
> @@ -625,7 +621,7 @@ static int blkdev_open(struct inode *inode, struct file *filp)
>  	if (!bdev)
>  		return -ENXIO;
>  
> -	if (bdev_can_atomic_write(bdev) && filp->f_flags & O_DIRECT)
> +	if (bdev_can_atomic_write(bdev))
>  		filp->f_mode |= FMODE_CAN_ATOMIC_WRITE;
>  
>  	ret = bdev_open(bdev, mode, filp->private_data, NULL, filp);
> @@ -700,6 +696,12 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  	if ((iocb->ki_flags & (IOCB_NOWAIT | IOCB_DIRECT)) == IOCB_NOWAIT)
>  		return -EOPNOTSUPP;
>  
> +	if (iocb->ki_flags & IOCB_ATOMIC) {
> +		ret = generic_atomic_write_valid(iocb, from);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	size -= iocb->ki_pos;
>  	if (iov_iter_count(from) > size) {
>  		shorted = iov_iter_count(from) - size;
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 2c3263530828..befec0b5c537 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -1830,18 +1830,21 @@ int generic_file_rw_checks(struct file *file_in, struct file *file_out)
>  	return 0;
>  }
>  
> -bool generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter)
> +int generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter)
>  {
>  	size_t len = iov_iter_count(iter);
>  
>  	if (!iter_is_ubuf(iter))
> -		return false;
> +		return -EINVAL;
>  
>  	if (!is_power_of_2(len))
> -		return false;
> +		return -EINVAL;
>  
>  	if (!IS_ALIGNED(iocb->ki_pos, len))
> -		return false;
> +		return -EINVAL;
>  
> -	return true;
> +	if (!(iocb->ki_flags & IOCB_DIRECT))
> +		return -EOPNOTSUPP;
> +
> +	return 0;
>  }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index fbfa032d1d90..ba47fb283730 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3721,6 +3721,6 @@ static inline bool vfs_empty_path(int dfd, const char __user *path)
>  	return !c;
>  }
>  
> -bool generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter);
> +int generic_atomic_write_valid(struct kiocb *iocb, struct iov_iter *iter);
>  
>  #endif /* _LINUX_FS_H */
> -- 
> 2.31.1
> 
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.