From ef53fbdc8e57a168e4733cd1e1a3702a552f66dd Mon Sep 17 00:00:00 2001 From: Qu Wenruo Date: Tue, 18 Jun 2024 15:11:39 +0930 Subject: [PATCH] btrfs-progs: change-csum: fix the wrong metadata space reservation [BUG] 'btrfstune --csum' would always fail for a newly created btrfs: # truncate -s 1G test.img # ./mkfs.btrfs -f test.img # ./btrsftune --csum xxhash test.img WARNING: Experimental build with unstable or unfinished features WARNING: Switching checksums is experimental, do not use for valuable data! Proceed to switch checksums ERROR: failed to start transaction: Unknown error -28 ERROR: failed to start transaction: No space left on device ERROR: failed to generate new data csums: No space left on device ERROR: btrfstune failed [CAUSE] After commit e79f18a4a75f ("btrfs-progs: introduce a basic metadata free space reservation check"), btrfs_start_transaction() would check the metadata space. But at the time of introduction of csum conversion, the parameter for btrfs_start_transaction() was incorrect. The 2nd parameter is the *number* of items to be added (if we're deleting items, just pass 1). However commit 08a3bd7694b6 ("btrfs-progs: tune: add the ability to generate new data checksums") is using the item size, not the number of items to be added. This means we're passing a number 8 * nodesize times larger than the original size, no wonder we would error out with -ENOSPC. [FIX] Use proper calcuation to convert the new csum item size to number of leaves needed, and double it just in case. Pull-request: #820 Signed-off-by: Qu Wenruo Signed-off-by: David Sterba --- tune/change-csum.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tune/change-csum.c b/tune/change-csum.c index f5fc3c7f..0e7db20f 100644 --- a/tune/change-csum.c +++ b/tune/change-csum.c @@ -224,14 +224,25 @@ out: * item. */ #define CSUM_CHANGE_BYTES_THRESHOLD (SZ_2M) + +static unsigned int calc_csum_change_nr_items(struct btrfs_fs_info *fs_info, + u16 new_csum_type) +{ + const u32 new_csum_size = btrfs_csum_type_size(new_csum_type); + const u32 csum_item_size = CSUM_CHANGE_BYTES_THRESHOLD / + fs_info->sectorsize * new_csum_size; + + return round_up(csum_item_size, fs_info->nodesize) / fs_info->nodesize * 2; +} + static int generate_new_data_csums_range(struct btrfs_fs_info *fs_info, u64 start, u16 new_csum_type) { + const unsigned int nr_items = calc_csum_change_nr_items(fs_info, new_csum_type); struct btrfs_root *csum_root = btrfs_csum_root(fs_info, 0); struct btrfs_trans_handle *trans; struct btrfs_path path = { 0 }; struct btrfs_key key; - const u32 new_csum_size = btrfs_csum_type_size(new_csum_type); void *csum_buffer; u64 converted_bytes = 0; u64 last_csum; @@ -248,9 +259,7 @@ static int generate_new_data_csums_range(struct btrfs_fs_info *fs_info, u64 star if (!csum_buffer) return -ENOMEM; - trans = btrfs_start_transaction(csum_root, - CSUM_CHANGE_BYTES_THRESHOLD / fs_info->sectorsize * - new_csum_size); + trans = btrfs_start_transaction(csum_root, nr_items); if (IS_ERR(trans)) { ret = PTR_ERR(trans); errno = -ret; @@ -306,9 +315,7 @@ static int generate_new_data_csums_range(struct btrfs_fs_info *fs_info, u64 star return -EUCLEAN; if (ret < 0) goto out; - trans = btrfs_start_transaction(csum_root, - CSUM_CHANGE_BYTES_THRESHOLD / - fs_info->sectorsize * new_csum_size); + trans = btrfs_start_transaction(csum_root, nr_items); if (IS_ERR(trans)) { ret = PTR_ERR(trans); goto out;