Re: [PATCH v3] [RESEND] btrfs-progs: add slack space for mkfs --shrink

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2025/5/17 03:05, Leo Martins 写道:
> This patch adds a flag `--shrink-slack-size SIZE` to the mkfs.btrfs
> allowing users to specify slack when shrinking the filesystem.
> Previously if you wanted to use --shrink and include extra space in the
> filesystem you would need to use btrfs resize, however, this requires
> mounting the filesystem which requires CAP_SYS_ADMIN.

Or create the initial fs with shrink, check how large the fs is, then 
re-create a file with extra space, then create the same fs without 
--shrink option.

If you're not happy with two runs, just calculate the size of the source 
directory, add 20% (already very conservative) for metadata, then adds 
the slack space, finally create fs without shrink.

Neither solution requires root privilege.


I'm not a super huge fan of the shrink slack idea, especially it may not 
work as you expected.

The shrink itself is already chunk based, meaning there may be as large 
as 1GiB free space for data/metadata already.

Furthermore even with slack space reserved, the next RW mount may easily 
cause extra chunk allocation to take up the whole slack space.
And if that allocation is for metadata, it may appear that there is no 
slack space at all soon after a RW mount.

Mind to explain the use case with more details?
And I'm also wondering how the feature is working for other filesystems?

I see no shrink like options in mkfs.ext4 nor mkfs.xfs, if you guys can 
handle ext4/xfs without shrinking, why not doing the same for btrfs?

Thanks,
Qu

> 
> The new syntax is:
> `mkfs.btrfs --shrink --shrink-slack-size SIZE`
> 
> Where slack size is an argument specifying the desired
> free space to add to a shrunk fs. If not provided, the default
> slack size is 0.
> 
> V3:
> - warn if block device size < fs size
> V2:
> - change --shrink[=SLACK SIZE] to --shrink-slack-size SIZE
> - check for slack size alignment
> - fix formatting
> - remove new_size > device size warning message
> 
> 
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> Reviewed-by: Mark Harmstone <maharmstone@fb.com>
> ---
>   mkfs/main.c    | 26 +++++++++++++++++++++++++-
>   mkfs/rootdir.c | 23 ++++++++++++++++++++++-
>   mkfs/rootdir.h |  2 +-
>   3 files changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/mkfs/main.c b/mkfs/main.c
> index dc73de47..715e939c 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -461,6 +461,8 @@ static const char * const mkfs_usage[] = {
>   	OPTLINE("", "- default - the SUBDIR will be a subvolume and also set as default (can be specified only once)"),
>   	OPTLINE("", "- default-ro - like 'default' and is created as read-only subvolume (can be specified only once)"),
>   	OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"),
> +	OPTLINE("--shrink-slack-size SIZE",
> +		"(with --shrink) include extra slack space after shrinking (default 0)"),
>   	OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"),
>   	OPTLINE("-f|--force", "force overwrite of existing filesystem"),
>   	"",
> @@ -1173,6 +1175,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>   	int i;
>   	bool ssd = false;
>   	bool shrink_rootdir = false;
> +	u64 shrink_slack_size = 0;
>   	u64 source_dir_size = 0;
>   	u64 min_dev_size;
>   	u64 shrink_size;
> @@ -1217,6 +1220,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>   		int c;
>   		enum {
>   			GETOPT_VAL_SHRINK = GETOPT_VAL_FIRST,
> +			GETOPT_VAL_SHRINK_SLACK_SIZE,
>   			GETOPT_VAL_CHECKSUM,
>   			GETOPT_VAL_GLOBAL_ROOTS,
>   			GETOPT_VAL_DEVICE_UUID,
> @@ -1247,6 +1251,8 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>   			{ "quiet", 0, NULL, 'q' },
>   			{ "verbose", 0, NULL, 'v' },
>   			{ "shrink", no_argument, NULL, GETOPT_VAL_SHRINK },
> +			{ "shrink-slack-size", required_argument, NULL,
> +			  GETOPT_VAL_SHRINK_SLACK_SIZE },
>   			{ "compress", required_argument, NULL,
>   				GETOPT_VAL_COMPRESS },
>   #if EXPERIMENTAL
> @@ -1383,6 +1389,9 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>   			case GETOPT_VAL_SHRINK:
>   				shrink_rootdir = true;
>   				break;
> +			case GETOPT_VAL_SHRINK_SLACK_SIZE:
> +				shrink_slack_size = arg_strtou64_with_suffix(optarg);
> +				break;
>   			case GETOPT_VAL_CHECKSUM:
>   				csum_type = parse_csum_type(optarg);
>   				break;
> @@ -1430,6 +1439,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>   		ret = 1;
>   		goto error;
>   	}
> +	if (shrink_slack_size > 0 && !shrink_rootdir) {
> +		error("the option --shrink-slack-size must be used with --shrink");
> +		ret = 1;
> +		goto error;
> +
> +	}
>   	if (!list_empty(&subvols) && source_dir == NULL) {
>   		error("option --subvol must be used with --rootdir");
>   		ret = 1;
> @@ -2108,8 +2123,17 @@ raid_groups:
>   
>   		if (shrink_rootdir) {
>   			pr_verbose(LOG_DEFAULT, "  Shrink:           yes\n");
> +			if (shrink_slack_size > 0) {
> +				pr_verbose(
> +					LOG_DEFAULT,
> +					"  Shrink slack:           %llu (%s)\n",
> +					shrink_slack_size,
> +					pretty_size(shrink_slack_size));
> +			}
>   			ret = btrfs_mkfs_shrink_fs(fs_info, &shrink_size,
> -						   shrink_rootdir);
> +						   shrink_rootdir,
> +						   shrink_slack_size);
> +
>   			if (ret < 0) {
>   				errno = -ret;
>   				error("error while shrinking filesystem: %m");
> diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
> index 19273947..5634d8c2 100644
> --- a/mkfs/rootdir.c
> +++ b/mkfs/rootdir.c
> @@ -17,6 +17,8 @@
>    */
>   
>   #include "kerncompat.h"
> +#include <linux/fs.h>
> +#include <sys/ioctl.h>
>   #include <sys/stat.h>
>   #include <sys/xattr.h>
>   #include <dirent.h>
> @@ -52,6 +54,7 @@
>   #include "common/root-tree-utils.h"
>   #include "common/path-utils.h"
>   #include "common/rbtree-utils.h"
> +#include "common/units.h"
>   #include "mkfs/rootdir.h"
>   
>   #define LZO_LEN 4
> @@ -1924,9 +1927,10 @@ err:
>   }
>   
>   int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
> -			 bool shrink_file_size)
> +			 bool shrink_file_size, u64 slack_size)
>   {
>   	u64 new_size;
> +	u64 blk_device_size;
>   	struct btrfs_device *device;
>   	struct list_head *cur;
>   	struct stat file_stat;
> @@ -1954,6 +1958,14 @@ int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
>   		return -EUCLEAN;
>   	}
>   
> +	if (!IS_ALIGNED(slack_size, fs_info->sectorsize)) {
> +		error("slack size %llu not aligned to %u",
> +				slack_size, fs_info->sectorsize);
> +		return -EUCLEAN;
> +	}
> +
> +	new_size += slack_size;
> +
>   	device = list_entry(fs_info->fs_devices->devices.next,
>   			   struct btrfs_device, dev_list);
>   	ret = set_device_size(fs_info, device, new_size);
> @@ -1968,6 +1980,15 @@ int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
>   			error("failed to stat devid %llu: %m", device->devid);
>   			return ret;
>   		}
> +		if (S_ISBLK(file_stat.st_mode)) {
> +			ioctl(device->fd, BLKGETSIZE64, &blk_device_size);
> +			if (blk_device_size < new_size) {
> +				warning("blkdev size %llu (%s) is smaller than fs size %llu (%s)",
> +					blk_device_size,
> +					pretty_size(blk_device_size), new_size,
> +					pretty_size(new_size));
> +			}
> +		}
>   		if (!S_ISREG(file_stat.st_mode))
>   			return ret;
>   		ret = ftruncate(device->fd, new_size);
> diff --git a/mkfs/rootdir.h b/mkfs/rootdir.h
> index b32fda5b..1eee3824 100644
> --- a/mkfs/rootdir.h
> +++ b/mkfs/rootdir.h
> @@ -52,6 +52,6 @@ int btrfs_mkfs_fill_dir(struct btrfs_trans_handle *trans, const char *source_dir
>   u64 btrfs_mkfs_size_dir(const char *dir_name, u32 sectorsize, u64 min_dev_size,
>   			u64 meta_profile, u64 data_profile);
>   int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
> -			 bool shrink_file_size);
> +			 bool shrink_file_size, u64 slack_size);
>   
>   #endif