Re: [PATCH v2 3/4] btrfs: add per-inode compression levels in xattrs

[Qu Wenruo <wqu@suse.com>]

在 2026/8/9 11:20, koraynilay 写道:
> Add support for specifying algo:level using
> btrfs property set /path/to/file compression "algo:level".
> 
> Add a signed 8 bit prop_compress_level property to btrfs_inode, which
> can support from level -128 to 127, plenty for the currently supported
> algo:level(s).
> 
> Change prop_compression_apply() to use the already present
> btrfs_match_compress_type() and btrfs_compress_str2level() to parse the
> xattr, using kmemdup_nul() to convert the user-supplied xattr value to a
> NUL-terminated string to be used by btrfs_compress_str2level(). The
> same approach was taken in prop_compression_validate() for
> btrfs_compress_is_valid_type().
> 
> Assisted-by: Gemini:3.1-pro antigravity-cli-1.1.5
> Signed-off-by: koraynilay <koray.fra@gmail.com>
> ---
>   fs/btrfs/btrfs_inode.h |  1 +
>   fs/btrfs/inode.c       |  2 ++
>   fs/btrfs/props.c       | 43 +++++++++++++++++++++++++++++++++---------
>   3 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index 1082fa92c145..0a4e567d0109 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -130,6 +130,7 @@ struct btrfs_inode {
>   
>   	/* Cached value of inode property 'compression'. */
>   	u8 prop_compress;
> +	s8 prop_compress_level;
>   
>   	/*
>   	 * Force compression on the file using the defrag ioctl, could be
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 2534cd9284d5..cff4b3e97559 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -938,6 +938,7 @@ static void compress_file_range(struct btrfs_work *work)
>   		compress_level = inode->defrag_compress_level;
>   	} else if (inode->prop_compress) {
>   		compress_type = inode->prop_compress;
> +		compress_level = inode->prop_compress_level;

I'd prefer to have a dedicated patch to set compress_level to the 
default value 0, as a proper bug fix as the first patch of the series.

As you mentioned in the cover-letter, this is in fact fixing a bug in 
the old behavior (mismatched algo and level).

So it's definitely worth a dedicated fix, so that we can backport the 
fix without pulling in the full series for older kernels.

Thanks,
Qu

>   	}
>   
>   	/* Compression level is applied here. */
> @@ -2326,6 +2327,7 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
>   			compress_level = inode->defrag_compress_level;
>   		} else if (inode->prop_compress) {
>   			compress_type = inode->prop_compress;
> +			compress_level = inode->prop_compress_level;
>   		}
>   		cb = btrfs_compress_bio(inode, 0, blocksize, compress_type, compress_level, 0);
>   		if (IS_ERR(cb)) {
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index bb77d46376d4..f79a61a2759f 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -295,20 +295,28 @@ int btrfs_load_inode_props(struct btrfs_inode *inode, struct btrfs_path *path)
>   static int prop_compression_validate(const struct btrfs_inode *inode,
>   				     const char *value, size_t len)
>   {
> +	int ret = -EINVAL;
> +	const char *value_str;
> +
>   	if (!btrfs_inode_can_compress(inode))
>   		return -EINVAL;
>   
>   	if (!value)
>   		return 0;
>   
> -	if (btrfs_compress_is_valid_type(value, len))
> -		return 0;
> -
>   	if ((len == 2 && strncmp("no", value, 2) == 0) ||
>   	    (len == 4 && strncmp("none", value, 4) == 0))
>   		return 0;
>   
> -	return -EINVAL;
> +	value_str = kmemdup_nul(value, len, GFP_KERNEL);
> +	if (!value_str)
> +		return -ENOMEM;
> +
> +	if (btrfs_compress_is_valid_type(value_str))
> +		ret = 0;
> +
> +	kfree(value_str);
> +	return ret;
>   }
>   
>   static int prop_compression_apply(struct btrfs_inode *inode, const char *value,
> @@ -316,6 +324,9 @@ static int prop_compression_apply(struct btrfs_inode *inode, const char *value,
>   {
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	int type;
> +	int level = 0;
> +	int ret = 0;
> +	const char *value_str;
>   
>   	/* Reset to defaults */
>   	if (len == 0) {
> @@ -335,23 +346,37 @@ static int prop_compression_apply(struct btrfs_inode *inode, const char *value,
>   		return 0;
>   	}
>   
> -	if (!strncmp("lzo", value, 3)) {
> +	value_str = kmemdup_nul(value, len, GFP_KERNEL);
> +	if (!value_str)
> +		return -ENOMEM;
> +
> +	if (btrfs_match_compress_type(value_str, "lzo", true)) {
>   		type = BTRFS_COMPRESS_LZO;
>   		btrfs_set_fs_incompat(fs_info, COMPRESS_LZO);
> -	} else if (!strncmp("zlib", value, 4)) {
> +	} else if (btrfs_match_compress_type(value_str, "zlib", true)) {
>   		type = BTRFS_COMPRESS_ZLIB;
> -	} else if (!strncmp("zstd", value, 4)) {
> +		ret = btrfs_compress_str2level(type, value_str + 4, &level);
> +		if (ret < 0)
> +			goto out;
> +	} else if (btrfs_match_compress_type(value_str, "zstd", true)) {
>   		type = BTRFS_COMPRESS_ZSTD;
> +		ret = btrfs_compress_str2level(type, value_str + 4, &level);
> +		if (ret < 0)
> +			goto out;
>   		btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD);
>   	} else {
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out;
>   	}
>   
>   	inode->flags &= ~BTRFS_INODE_NOCOMPRESS;
>   	inode->flags |= BTRFS_INODE_COMPRESS;
>   	inode->prop_compress = type;
> +	inode->prop_compress_level = level;
>   
> -	return 0;
> +out:
> +	kfree(value_str);
> +	return ret;
>   }
>   
>   static bool prop_compression_ignore(const struct btrfs_inode *inode)