Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: Daniel Vacek <neelx@suse.com>, Chris Mason <clm@fb.com>,
	Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>, Nick Terrell <terrelln@fb.com>
Cc: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] btrfs/defrag: implement compression levels
Date: Fri, 7 Mar 2025 08:24:41 +1030	[thread overview]
Message-ID: <afeb86e8-a1be-43b5-964c-269bd16fcc1f@suse.com> (raw)
In-Reply-To: <20250306131537.972377-1-neelx@suse.com>



在 2025/3/6 23:45, Daniel Vacek 写道:
> The zstd and zlib compression types support setting compression levels.
> Enhance the defrag interface to specify the levels as well.
> 
> Signed-off-by: Daniel Vacek <neelx@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
> v3: Validate the level instead of clamping and fix the comment of the
>      btrfs_ioctl_defrag_range_args structure.
> 
> v2: Fixed the commit message and added an explicit level range clamping.
> 
>   fs/btrfs/btrfs_inode.h     |  1 +
>   fs/btrfs/compression.c     | 10 ++++++++++
>   fs/btrfs/compression.h     |  1 +
>   fs/btrfs/defrag.c          | 24 +++++++++++++++++++-----
>   fs/btrfs/fs.h              |  2 +-
>   fs/btrfs/inode.c           |  9 ++++++---
>   include/uapi/linux/btrfs.h | 16 +++++++++++++---
>   7 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index aa1f55cd81b79..238e4a08a52ae 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -145,6 +145,7 @@ struct btrfs_inode {
>   	 * different from prop_compress and takes precedence if set.
>   	 */
>   	u8 defrag_compress;
> +	s8 defrag_compress_level;
>   
>   	/*
>   	 * Lock for counters and all fields used to determine if the inode is in
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 6d073e69af4e3..4191e9efc6951 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -980,6 +980,16 @@ static int btrfs_compress_set_level(unsigned int type, int level)
>   	return level;
>   }
>   
> +/*
> + * Check whether the @level is within the valid range for the given type.
> + */
> +bool btrfs_compress_level_valid(unsigned int type, int level)
> +{
> +	const struct btrfs_compress_op *ops = btrfs_compress_op[type];
> +
> +	return ops->min_level <= level && level <= ops->max_level;
> +}
> +
>   /* Wrapper around find_get_page(), with extra error message. */
>   int btrfs_compress_filemap_get_folio(struct address_space *mapping, u64 start,
>   				     struct folio **in_folio_ret)
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index 933178f03d8f8..df198623cc084 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -83,6 +83,7 @@ static inline u32 btrfs_calc_input_length(u64 range_end, u64 cur)
>   int __init btrfs_init_compress(void);
>   void __cold btrfs_exit_compress(void);
>   
> +bool btrfs_compress_level_valid(unsigned int type, int level);
>   int btrfs_compress_folios(unsigned int type, int level, struct address_space *mapping,
>   			  u64 start, struct folio **folios, unsigned long *out_folios,
>   			 unsigned long *total_in, unsigned long *total_out);
> diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
> index 968dae9539482..513089b91b7b6 100644
> --- a/fs/btrfs/defrag.c
> +++ b/fs/btrfs/defrag.c
> @@ -1363,6 +1363,7 @@ int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
>   	u64 last_byte;
>   	bool do_compress = (range->flags & BTRFS_DEFRAG_RANGE_COMPRESS);
>   	int compress_type = BTRFS_COMPRESS_ZLIB;
> +	int compress_level = 0;
>   	int ret = 0;
>   	u32 extent_thresh = range->extent_thresh;
>   	pgoff_t start_index;
> @@ -1376,10 +1377,21 @@ int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
>   		return -EINVAL;
>   
>   	if (do_compress) {
> -		if (range->compress_type >= BTRFS_NR_COMPRESS_TYPES)
> -			return -EINVAL;
> -		if (range->compress_type)
> -			compress_type = range->compress_type;
> +		if (range->flags & BTRFS_DEFRAG_RANGE_COMPRESS_LEVEL) {
> +			if (range->compress.type >= BTRFS_NR_COMPRESS_TYPES)
> +				return -EINVAL;
> +			if (range->compress.type) {
> +				compress_type  = range->compress.type;
> +				compress_level = range->compress.level;
> +				if (!btrfs_compress_level_valid(compress_type, compress_level))
> +					return -EINVAL;
> +			}
> +		} else {
> +			if (range->compress_type >= BTRFS_NR_COMPRESS_TYPES)
> +				return -EINVAL;
> +			if (range->compress_type)
> +				compress_type = range->compress_type;
> +		}
>   	}
>   
>   	if (extent_thresh == 0)
> @@ -1430,8 +1442,10 @@ int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
>   			btrfs_inode_unlock(BTRFS_I(inode), 0);
>   			break;
>   		}
> -		if (do_compress)
> +		if (do_compress) {
>   			BTRFS_I(inode)->defrag_compress = compress_type;
> +			BTRFS_I(inode)->defrag_compress_level = compress_level;
> +		}
>   		ret = defrag_one_cluster(BTRFS_I(inode), ra, cur,
>   				cluster_end + 1 - cur, extent_thresh,
>   				newer_than, do_compress, &sectors_defragged,
> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> index be6d5a24bd4e6..2dae7ffd37133 100644
> --- a/fs/btrfs/fs.h
> +++ b/fs/btrfs/fs.h
> @@ -485,7 +485,7 @@ struct btrfs_fs_info {
>   	u64 last_trans_log_full_commit;
>   	unsigned long long mount_opt;
>   
> -	unsigned long compress_type:4;
> +	int compress_type;
>   	int compress_level;
>   	u32 commit_interval;
>   	/*
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index fa04b027d53ac..dd27992ecb431 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -925,6 +925,7 @@ static void compress_file_range(struct btrfs_work *work)
>   	unsigned int poff;
>   	int i;
>   	int compress_type = fs_info->compress_type;
> +	int compress_level = fs_info->compress_level;
>   
>   	inode_should_defrag(inode, start, end, end - start + 1, SZ_16K);
>   
> @@ -1007,13 +1008,15 @@ static void compress_file_range(struct btrfs_work *work)
>   		goto cleanup_and_bail_uncompressed;
>   	}
>   
> -	if (inode->defrag_compress)
> +	if (inode->defrag_compress) {
>   		compress_type = inode->defrag_compress;
> -	else if (inode->prop_compress)
> +		compress_level = inode->defrag_compress_level;
> +	} else if (inode->prop_compress) {
>   		compress_type = inode->prop_compress;
> +	}
>   
>   	/* Compression level is applied here. */
> -	ret = btrfs_compress_folios(compress_type, fs_info->compress_level,
> +	ret = btrfs_compress_folios(compress_type, compress_level,
>   				    mapping, start, folios, &nr_folios, &total_in,
>   				    &total_compressed);
>   	if (ret)
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index d3b222d7af240..dd02160015b2b 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -615,7 +615,9 @@ struct btrfs_ioctl_clone_range_args {
>    */
>   #define BTRFS_DEFRAG_RANGE_COMPRESS 1
>   #define BTRFS_DEFRAG_RANGE_START_IO 2
> +#define BTRFS_DEFRAG_RANGE_COMPRESS_LEVEL 4
>   #define BTRFS_DEFRAG_RANGE_FLAGS_SUPP	(BTRFS_DEFRAG_RANGE_COMPRESS |		\
> +					 BTRFS_DEFRAG_RANGE_COMPRESS_LEVEL |	\
>   					 BTRFS_DEFRAG_RANGE_START_IO)
>   
>   struct btrfs_ioctl_defrag_range_args {
> @@ -640,10 +642,18 @@ struct btrfs_ioctl_defrag_range_args {
>   
>   	/*
>   	 * which compression method to use if turning on compression
> -	 * for this defrag operation.  If unspecified, zlib will
> -	 * be used
> +	 * for this defrag operation. If unspecified, zlib will be
> +	 * used. If compression level is also being specified, set the
> +	 * BTRFS_DEFRAG_RANGE_COMPRESS_LEVEL flag and fill the compress
> +	 * member structure instead of the compress_type field.
>   	 */
> -	__u32 compress_type;
> +	union {
> +		__u32 compress_type;
> +		struct {
> +			__u8 type;
> +			__s8 level;
> +		} compress;
> +	};
>   
>   	/* spare for later */
>   	__u32 unused[4];


      parent reply	other threads:[~2025-03-06 21:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-04 17:14 [PATCH] btrfs/defrag: implement compression levels Daniel Vacek
2025-03-04 21:31 ` Qu Wenruo
2025-03-05  7:02   ` David Sterba
2025-03-05  7:08     ` Daniel Vacek
2025-03-05  7:02   ` Daniel Vacek
2025-03-05  7:05     ` David Sterba
2025-03-05  7:21       ` Daniel Vacek
2025-03-05  7:44     ` Qu Wenruo
2025-03-05  8:01       ` David Sterba
2025-03-05  8:08         ` Qu Wenruo
2025-03-05 10:32 ` [PATCH v2] " Daniel Vacek
2025-03-06  8:27   ` David Sterba
2025-03-06 13:15     ` Daniel Vacek
2025-03-06 13:15 ` [PATCH v3] " Daniel Vacek
2025-03-06 17:16   ` David Sterba
2025-03-06 21:54   ` Qu Wenruo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afeb86e8-a1be-43b5-964c-269bd16fcc1f@suse.com \
    --to=wqu@suse.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neelx@suse.com \
    --cc=terrelln@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox