diff mbox series

[1/2] btrfs-progs: change-csum: fix the wrong metadata space reservation

Message ID 0a232d728355ecc03465933b70b2b3d682858820.1718693318.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: fix csum metadata reservation and add a basic test case for itb | expand

Commit Message

Qu Wenruo June 18, 2024, 6:50 a.m. UTC
[BUG]
`btrfstune --csum` would always fail for a newly created btrfs:

 # truncates -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() is 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.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/tune/change-csum.c b/tune/change-csum.c
index f5fc3c7f32cb..371e0aa76fc1 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -224,14 +224,26 @@  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 +260,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 +316,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;