From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH 1/2] btrfs-progs: change-csum: fix the wrong metadata space reservation
Date: Tue, 18 Jun 2024 16:20:48 +0930 [thread overview]
Message-ID: <0a232d728355ecc03465933b70b2b3d682858820.1718693318.git.wqu@suse.com> (raw)
In-Reply-To: <cover.1718693318.git.wqu@suse.com>
[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 --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;
--
2.45.2
next prev parent reply other threads:[~2024-06-18 6:51 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-18 6:50 [PATCH 0/2] btrfs-progs: fix csum metadata reservation and add a basic test case for itb Qu Wenruo
2024-06-18 6:50 ` Qu Wenruo [this message]
2024-06-18 6:50 ` [PATCH 2/2] btrfs-progs: misc-tests: add a test case for basic csum conversion Qu Wenruo
2024-06-19 1:53 ` [PATCH 0/2] btrfs-progs: fix csum metadata reservation and add a basic test case for itb David Sterba
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=0a232d728355ecc03465933b70b2b3d682858820.1718693318.git.wqu@suse.com \
--to=wqu@suse.com \
--cc=linux-btrfs@vger.kernel.org \
/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