Re: [PATCH 1/2] btrfs: export a helper for compression hard check

[Filipe Manana <fdmanana@kernel.org>]

On Fri, Apr 15, 2022 at 04:04:05PM +0800, Chung-Chiang Cheng wrote:
> inode_can_compress will be used outside of inode.c to check the
> availability of setting compression flag by xattr. This patch moves
> this function as an internal helper and renames it to
> btrfs_inode_can_compress.

Btw, the idea was to export the function in a patch following the bug fix
patch. That would imply temporarily duplicating the validation logic in
the bug fix patch, and then the followup patch would export
inode_can_compress() and make inode.c and props.c use it.

That makes the backport to stable easier.

Alternatively, IMO, since it's such a short and trivial change, both
patches could be combined in a single patch. Not everyone might agree
with that however.

Either way, I'm fine with it, thanks.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

> 
> Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com>
> ---
>  fs/btrfs/btrfs_inode.h | 11 +++++++++++
>  fs/btrfs/inode.c       | 15 ++-------------
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index 47e72d72f7d0..32131a5d321b 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -384,6 +384,17 @@ static inline bool btrfs_inode_in_log(struct btrfs_inode *inode, u64 generation)
>  	return ret;
>  }
>  
> +/*
> + * Check if the inode has flags compatible with compression
> + */
> +static inline bool btrfs_inode_can_compress(const struct btrfs_inode *inode)
> +{
> +	if (inode->flags & BTRFS_INODE_NODATACOW ||
> +	    inode->flags & BTRFS_INODE_NODATASUM)
> +		return false;
> +	return true;
> +}
> +
>  struct btrfs_dio_private {
>  	struct inode *inode;
>  
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 17d5557f98ec..99725e5508f9 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -480,17 +480,6 @@ static noinline int add_async_extent(struct async_chunk *cow,
>  	return 0;
>  }
>  
> -/*
> - * Check if the inode has flags compatible with compression
> - */
> -static inline bool inode_can_compress(struct btrfs_inode *inode)
> -{
> -	if (inode->flags & BTRFS_INODE_NODATACOW ||
> -	    inode->flags & BTRFS_INODE_NODATASUM)
> -		return false;
> -	return true;
> -}
> -
>  /*
>   * Check if the inode needs to be submitted to compression, based on mount
>   * options, defragmentation, properties or heuristics.
> @@ -500,7 +489,7 @@ static inline int inode_need_compress(struct btrfs_inode *inode, u64 start,
>  {
>  	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>  
> -	if (!inode_can_compress(inode)) {
> +	if (!btrfs_inode_can_compress(inode)) {
>  		WARN(IS_ENABLED(CONFIG_BTRFS_DEBUG),
>  			KERN_ERR "BTRFS: unexpected compression for ino %llu\n",
>  			btrfs_ino(inode));
> @@ -2020,7 +2009,7 @@ int btrfs_run_delalloc_range(struct btrfs_inode *inode, struct page *locked_page
>  		       (zoned && btrfs_is_data_reloc_root(inode->root)));
>  		ret = run_delalloc_nocow(inode, locked_page, start, end,
>  					 page_started, nr_written);
> -	} else if (!inode_can_compress(inode) ||
> +	} else if (!btrfs_inode_can_compress(inode) ||
>  		   !inode_need_compress(inode, start, end)) {
>  		if (zoned)
>  			ret = run_delalloc_zoned(inode, locked_page, start, end,
> -- 
> 2.34.1
>