diff mbox series

[10/11] iomap: add a IOMAP_DIO_UNALIGNED flag

Message ID 20210118193516.2915706-11-hch@lst.de (mailing list archive)
State New
Headers show
Series [01/11] xfs: factor out a xfs_ilock_iocb helper | expand

Commit Message

Christoph Hellwig Jan. 18, 2021, 7:35 p.m. UTC
Add a flag to signal an I/O that is not file system block aligned.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/direct-io.c  | 7 +++++++
 include/linux/iomap.h | 8 ++++++++
 2 files changed, 15 insertions(+)

Comments

Dave Chinner Jan. 18, 2021, 8:45 p.m. UTC | #1
On Mon, Jan 18, 2021 at 08:35:15PM +0100, Christoph Hellwig wrote:
> Add a flag to signal an I/O that is not file system block aligned.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/iomap/direct-io.c  | 7 +++++++
>  include/linux/iomap.h | 8 ++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 32dbbf7dd4aadb..d93019ee4c9e3e 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -485,6 +485,13 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  		iomap_flags |= IOMAP_NOWAIT;
>  	}
>  
> +	if (dio_flags & IOMAP_DIO_UNALIGNED) {
> +		ret = -EAGAIN;
> +		if (pos >= dio->i_size)
> +			goto out_free_dio;

This also needs to check for pos+len > dio->i_size on a write as
iomap_dio_rw_actor will do unconditional sub-block zeroing in that
case, too.

> +		iomap_flags |= IOMAP_UNALIGNED;
> +	}
> +
>  	ret = filemap_write_and_wait_range(mapping, pos, end);
>  	if (ret)
>  		goto out_free_dio;
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index b322598dc10ec0..2fa94ec9583d0a 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -122,6 +122,7 @@ struct iomap_page_ops {
>  #define IOMAP_FAULT		(1 << 3) /* mapping for page fault */
>  #define IOMAP_DIRECT		(1 << 4) /* direct I/O */
>  #define IOMAP_NOWAIT		(1 << 5) /* do not block */
> +#define IOMAP_UNALIGNED		(1 << 6) /* do not allocate blocks */
>  
>  struct iomap_ops {
>  	/*
> @@ -262,6 +263,13 @@ struct iomap_dio_ops {
>   */
>  #define IOMAP_DIO_FORCE_WAIT	(1 << 0)
>  
> +/*
> + * Direct I/O that is not aligned to the file system block.  Do not allocate
> + * blocks and do not zero partial blocks, fall back to the caller by returning
> + * -EAGAIN instead.
> + */
> +#define IOMAP_DIO_UNALIGNED	(1 << 1)

I'd describe it a little bit differently, clearly indicating that
this is for optional behaviour and not needed on all unaligned DIO.

/*
 * Filesystems may need to special case DIO that is not aligned to
 * block boundaries. If they set IOMAP_DIO_UNALIGNED on an unaligned
 * IO, then do not allocate blocks or zero partial blocks, but
 * instead fall back to the caller by returning -EAGAIN so they can
 * handle these conditions correctly.
 */

Cheers,

Dave.
Matthew Wilcox Jan. 18, 2021, 9:41 p.m. UTC | #2
On Mon, Jan 18, 2021 at 08:35:15PM +0100, Christoph Hellwig wrote:
> Add a flag to signal an I/O that is not file system block aligned.
> +	if (dio_flags & IOMAP_DIO_UNALIGNED) {

There are a number of things that DIO has to be aligned to -- memory
addresses, for example.  Can we be a little more verbose about what is
unaligned here?  eg

	if (dio_flags & IOMAP_DIO_FS_UNALIGNED)

(or FSBLK_UNALIGNED, or ... something).
Christoph Hellwig Jan. 20, 2021, 4:40 p.m. UTC | #3
I've renamed it to IOMAP_DIO_SUBBLOCK
Darrick J. Wong Jan. 20, 2021, 6:47 p.m. UTC | #4
On Mon, Jan 18, 2021 at 08:35:15PM +0100, Christoph Hellwig wrote:
> Add a flag to signal an I/O that is not file system block aligned.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/iomap/direct-io.c  | 7 +++++++
>  include/linux/iomap.h | 8 ++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 32dbbf7dd4aadb..d93019ee4c9e3e 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -485,6 +485,13 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  		iomap_flags |= IOMAP_NOWAIT;
>  	}
>  
> +	if (dio_flags & IOMAP_DIO_UNALIGNED) {
> +		ret = -EAGAIN;
> +		if (pos >= dio->i_size)
> +			goto out_free_dio;
> +		iomap_flags |= IOMAP_UNALIGNED;
> +	}
> +
>  	ret = filemap_write_and_wait_range(mapping, pos, end);
>  	if (ret)
>  		goto out_free_dio;
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index b322598dc10ec0..2fa94ec9583d0a 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -122,6 +122,7 @@ struct iomap_page_ops {
>  #define IOMAP_FAULT		(1 << 3) /* mapping for page fault */
>  #define IOMAP_DIRECT		(1 << 4) /* direct I/O */
>  #define IOMAP_NOWAIT		(1 << 5) /* do not block */
> +#define IOMAP_UNALIGNED		(1 << 6) /* do not allocate blocks */
>  
>  struct iomap_ops {
>  	/*
> @@ -262,6 +263,13 @@ struct iomap_dio_ops {
>   */
>  #define IOMAP_DIO_FORCE_WAIT	(1 << 0)
>  
> +/*
> + * Direct I/O that is not aligned to the file system block.  Do not allocate
> + * blocks and do not zero partial blocks, fall back to the caller by returning
> + * -EAGAIN instead.
> + */
> +#define IOMAP_DIO_UNALIGNED	(1 << 1)

The code changes look fine, but as for the name, I found it a little
confusing even after changing it to IOMAP_DIO_SUBBLOCK.

See my reply to patch 11 for more details.

--D

> +
>  ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
>  		unsigned int flags);
> -- 
> 2.29.2
>
diff mbox series

Patch

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 32dbbf7dd4aadb..d93019ee4c9e3e 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -485,6 +485,13 @@  __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 		iomap_flags |= IOMAP_NOWAIT;
 	}
 
+	if (dio_flags & IOMAP_DIO_UNALIGNED) {
+		ret = -EAGAIN;
+		if (pos >= dio->i_size)
+			goto out_free_dio;
+		iomap_flags |= IOMAP_UNALIGNED;
+	}
+
 	ret = filemap_write_and_wait_range(mapping, pos, end);
 	if (ret)
 		goto out_free_dio;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index b322598dc10ec0..2fa94ec9583d0a 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -122,6 +122,7 @@  struct iomap_page_ops {
 #define IOMAP_FAULT		(1 << 3) /* mapping for page fault */
 #define IOMAP_DIRECT		(1 << 4) /* direct I/O */
 #define IOMAP_NOWAIT		(1 << 5) /* do not block */
+#define IOMAP_UNALIGNED		(1 << 6) /* do not allocate blocks */
 
 struct iomap_ops {
 	/*
@@ -262,6 +263,13 @@  struct iomap_dio_ops {
  */
 #define IOMAP_DIO_FORCE_WAIT	(1 << 0)
 
+/*
+ * Direct I/O that is not aligned to the file system block.  Do not allocate
+ * blocks and do not zero partial blocks, fall back to the caller by returning
+ * -EAGAIN instead.
+ */
+#define IOMAP_DIO_UNALIGNED	(1 << 1)
+
 ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
 		unsigned int flags);