[PATCH] iomap: turn iomap_want_unshare_iter into an inline function

[PATCH] iomap: turn iomap_want_unshare_iter into an inline function

[Christoph Hellwig]

iomap_want_unshare_iter currently sits in fs/iomap/buffered-io.c, which
depends on CONFIG_BLOCK.  It is also in used in fs/dax.c whіch has no
such dependency.  Given that it is a trivial check turn it into an inline
in include/linux/iomap.h to fix the DAX && !BLOCK build.

Fixes: 6ef6a0e821d3 ("iomap: share iomap_unshare_iter predicate code with fsdax")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/buffered-io.c | 17 -----------------
 include/linux/iomap.h  | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 604f786be4bc54..ef0b68bccbb612 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -1270,23 +1270,6 @@ void iomap_write_delalloc_release(struct inode *inode, loff_t start_byte,
 }
 EXPORT_SYMBOL_GPL(iomap_write_delalloc_release);
 
-bool iomap_want_unshare_iter(const struct iomap_iter *iter)
-{
-	/*
-	 * Don't bother with blocks that are not shared to start with; or
-	 * mappings that cannot be shared, such as inline data, delalloc
-	 * reservations, holes or unwritten extents.
-	 *
-	 * Note that we use srcmap directly instead of iomap_iter_srcmap as
-	 * unsharing requires providing a separate source map, and the presence
-	 * of one is a good indicator that unsharing is needed, unlike
-	 * IOMAP_F_SHARED which can be set for any data that goes into the COW
-	 * fork for XFS.
-	 */
-	return (iter->iomap.flags & IOMAP_F_SHARED) &&
-		iter->srcmap.type == IOMAP_MAPPED;
-}
-
 static loff_t iomap_unshare_iter(struct iomap_iter *iter)
 {
 	struct iomap *iomap = &iter->iomap;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index e04c060e8fe185..664c5f2f0aaa2d 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -270,6 +270,25 @@ static inline loff_t iomap_last_written_block(struct inode *inode, loff_t pos,
 	return round_up(pos + written, i_blocksize(inode));
 }
 
+/*
+ * Check if the range needs to be unshared for a FALLOC_FL_UNSHARE_RANGE
+ * operation.
+ *
+ * Don't bother with blocks that are not shared to start with; or mappings that
+ * cannot be shared, such as inline data, delalloc reservations, holes or
+ * unwritten extents.
+ *
+ * Note that we use srcmap directly instead of iomap_iter_srcmap as unsharing
+ * requires providing a separate source map, and the presence of one is a good
+ * indicator that unsharing is needed, unlike IOMAP_F_SHARED which can be set
+ * for any data that goes into the COW fork for XFS.
+ */
+static inline bool iomap_want_unshare_iter(const struct iomap_iter *iter)
+{
+	return (iter->iomap.flags & IOMAP_F_SHARED) &&
+		iter->srcmap.type == IOMAP_MAPPED;
+}
+
 ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
 		const struct iomap_ops *ops, void *private);
 int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops);
-- 
2.45.2

Re: [PATCH] iomap: turn iomap_want_unshare_iter into an inline function

[Brian Foster]

On Tue, Oct 15, 2024 at 06:13:50AM +0200, Christoph Hellwig wrote:
> iomap_want_unshare_iter currently sits in fs/iomap/buffered-io.c, which
> depends on CONFIG_BLOCK.  It is also in used in fs/dax.c whіch has no
> such dependency.  Given that it is a trivial check turn it into an inline
> in include/linux/iomap.h to fix the DAX && !BLOCK build.
> 
> Fixes: 6ef6a0e821d3 ("iomap: share iomap_unshare_iter predicate code with fsdax")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/iomap/buffered-io.c | 17 -----------------
>  include/linux/iomap.h  | 19 +++++++++++++++++++
>  2 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 604f786be4bc54..ef0b68bccbb612 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1270,23 +1270,6 @@ void iomap_write_delalloc_release(struct inode *inode, loff_t start_byte,
>  }
>  EXPORT_SYMBOL_GPL(iomap_write_delalloc_release);
>  
> -bool iomap_want_unshare_iter(const struct iomap_iter *iter)
> -{
> -	/*
> -	 * Don't bother with blocks that are not shared to start with; or
> -	 * mappings that cannot be shared, such as inline data, delalloc
> -	 * reservations, holes or unwritten extents.
> -	 *
> -	 * Note that we use srcmap directly instead of iomap_iter_srcmap as
> -	 * unsharing requires providing a separate source map, and the presence
> -	 * of one is a good indicator that unsharing is needed, unlike
> -	 * IOMAP_F_SHARED which can be set for any data that goes into the COW
> -	 * fork for XFS.
> -	 */
> -	return (iter->iomap.flags & IOMAP_F_SHARED) &&
> -		iter->srcmap.type == IOMAP_MAPPED;
> -}
> -
>  static loff_t iomap_unshare_iter(struct iomap_iter *iter)
>  {
>  	struct iomap *iomap = &iter->iomap;
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index e04c060e8fe185..664c5f2f0aaa2d 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -270,6 +270,25 @@ static inline loff_t iomap_last_written_block(struct inode *inode, loff_t pos,
>  	return round_up(pos + written, i_blocksize(inode));
>  }
>  
> +/*
> + * Check if the range needs to be unshared for a FALLOC_FL_UNSHARE_RANGE
> + * operation.
> + *
> + * Don't bother with blocks that are not shared to start with; or mappings that
> + * cannot be shared, such as inline data, delalloc reservations, holes or
> + * unwritten extents.
> + *
> + * Note that we use srcmap directly instead of iomap_iter_srcmap as unsharing
> + * requires providing a separate source map, and the presence of one is a good
> + * indicator that unsharing is needed, unlike IOMAP_F_SHARED which can be set
> + * for any data that goes into the COW fork for XFS.
> + */
> +static inline bool iomap_want_unshare_iter(const struct iomap_iter *iter)
> +{
> +	return (iter->iomap.flags & IOMAP_F_SHARED) &&
> +		iter->srcmap.type == IOMAP_MAPPED;
> +}
> +
>  ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
>  		const struct iomap_ops *ops, void *private);
>  int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops);
> -- 
> 2.45.2
> 
> 

Re: [PATCH] iomap: turn iomap_want_unshare_iter into an inline function

[Christian Brauner]

On Tue, 15 Oct 2024 06:13:50 +0200, Christoph Hellwig wrote:
> iomap_want_unshare_iter currently sits in fs/iomap/buffered-io.c, which
> depends on CONFIG_BLOCK.  It is also in used in fs/dax.c whіch has no
> such dependency.  Given that it is a trivial check turn it into an inline
> in include/linux/iomap.h to fix the DAX && !BLOCK build.
> 
> 

Applied to the vfs.iomap branch of the vfs/vfs.git tree.
Patches in the vfs.iomap branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.iomap

[1/1] iomap: turn iomap_want_unshare_iter into an inline function
      https://git.kernel.org/vfs/vfs/c/13586180450a

Re: [PATCH] iomap: turn iomap_want_unshare_iter into an inline function

[Darrick J. Wong]

On Tue, Oct 15, 2024 at 06:13:50AM +0200, Christoph Hellwig wrote:
> iomap_want_unshare_iter currently sits in fs/iomap/buffered-io.c, which
> depends on CONFIG_BLOCK.  It is also in used in fs/dax.c whіch has no
> such dependency.  Given that it is a trivial check turn it into an inline
> in include/linux/iomap.h to fix the DAX && !BLOCK build.
> 
> Fixes: 6ef6a0e821d3 ("iomap: share iomap_unshare_iter predicate code with fsdax")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Heh, whoops.  I forgot (a) that DAX && !BLOCK is a thing; and that
FS_DAX != DAX and was puzzling over this report yesterday.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/iomap/buffered-io.c | 17 -----------------
>  include/linux/iomap.h  | 19 +++++++++++++++++++
>  2 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 604f786be4bc54..ef0b68bccbb612 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1270,23 +1270,6 @@ void iomap_write_delalloc_release(struct inode *inode, loff_t start_byte,
>  }
>  EXPORT_SYMBOL_GPL(iomap_write_delalloc_release);
>  
> -bool iomap_want_unshare_iter(const struct iomap_iter *iter)
> -{
> -	/*
> -	 * Don't bother with blocks that are not shared to start with; or
> -	 * mappings that cannot be shared, such as inline data, delalloc
> -	 * reservations, holes or unwritten extents.
> -	 *
> -	 * Note that we use srcmap directly instead of iomap_iter_srcmap as
> -	 * unsharing requires providing a separate source map, and the presence
> -	 * of one is a good indicator that unsharing is needed, unlike
> -	 * IOMAP_F_SHARED which can be set for any data that goes into the COW
> -	 * fork for XFS.
> -	 */
> -	return (iter->iomap.flags & IOMAP_F_SHARED) &&
> -		iter->srcmap.type == IOMAP_MAPPED;
> -}
> -
>  static loff_t iomap_unshare_iter(struct iomap_iter *iter)
>  {
>  	struct iomap *iomap = &iter->iomap;
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index e04c060e8fe185..664c5f2f0aaa2d 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -270,6 +270,25 @@ static inline loff_t iomap_last_written_block(struct inode *inode, loff_t pos,
>  	return round_up(pos + written, i_blocksize(inode));
>  }
>  
> +/*
> + * Check if the range needs to be unshared for a FALLOC_FL_UNSHARE_RANGE
> + * operation.
> + *
> + * Don't bother with blocks that are not shared to start with; or mappings that
> + * cannot be shared, such as inline data, delalloc reservations, holes or
> + * unwritten extents.
> + *
> + * Note that we use srcmap directly instead of iomap_iter_srcmap as unsharing
> + * requires providing a separate source map, and the presence of one is a good
> + * indicator that unsharing is needed, unlike IOMAP_F_SHARED which can be set
> + * for any data that goes into the COW fork for XFS.
> + */
> +static inline bool iomap_want_unshare_iter(const struct iomap_iter *iter)
> +{
> +	return (iter->iomap.flags & IOMAP_F_SHARED) &&
> +		iter->srcmap.type == IOMAP_MAPPED;
> +}
> +
>  ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
>  		const struct iomap_ops *ops, void *private);
>  int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops);
> -- 
> 2.45.2
> 
>