* [PATCH v2 1/3] btrfs-progs: csum-change: add error handling for search old csums
2024-06-21 5:16 [PATCH v2 0/3] btrfs-progs: btrfs-progs: csum-change enhancement Qu Wenruo
@ 2024-06-21 5:16 ` Qu Wenruo
2024-06-21 5:16 ` [PATCH v2 2/3] btrfs-progs: csum-change: add leaf based threshold Qu Wenruo
2024-06-21 5:16 ` [PATCH v2 3/3] btrfs-progs: misc-tests: add a basic resume test using error injection Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2024-06-21 5:16 UTC (permalink / raw)
To: linux-btrfs
Inside delete_old_data_csums(), after calling btrfs_search_slot() there
is no error handling at all.
Fix it by doing a proper error detection and abort the current transaction.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
tune/change-csum.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tune/change-csum.c b/tune/change-csum.c
index 0e7db20f5e6d..0f95cdb25533 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -395,6 +395,13 @@ static int delete_old_data_csums(struct btrfs_fs_info *fs_info)
int nr;
ret = btrfs_search_slot(trans, csum_root, &last_key, &path, -1, 1);
+ if (ret < 0) {
+ errno = -ret;
+ error("failed to search the last old csum item: %m");
+ btrfs_abort_transaction(trans, ret);
+ return ret;
+ }
+ assert(ret > 0);
nr = btrfs_header_nritems(path.nodes[0]);
/* No item left (empty csum tree), exit. */
--
2.45.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/3] btrfs-progs: csum-change: add leaf based threshold
2024-06-21 5:16 [PATCH v2 0/3] btrfs-progs: btrfs-progs: csum-change enhancement Qu Wenruo
2024-06-21 5:16 ` [PATCH v2 1/3] btrfs-progs: csum-change: add error handling for search old csums Qu Wenruo
@ 2024-06-21 5:16 ` Qu Wenruo
2024-06-21 5:16 ` [PATCH v2 3/3] btrfs-progs: misc-tests: add a basic resume test using error injection Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2024-06-21 5:16 UTC (permalink / raw)
To: linux-btrfs
For "btrfstune --csum", currently we do the following operations in just
one transaction for each:
- Delete old data csums
- Change new data csums objectid
Both opeartion can modify up to GiB or even TiB level of metadata, doing
them in just one transaction is definitely going to cause problems.
This patch adds a leaf number based threshold (32 leaves), after
modifying/deleteing this many leaves, we commit a transaction to avoid
huge amount of dirty leaves piling up.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
tune/change-csum.c | 68 +++++++++++++++++++++++++++++++++++-----------
1 file changed, 52 insertions(+), 16 deletions(-)
diff --git a/tune/change-csum.c b/tune/change-csum.c
index 0f95cdb25533..66fdd207335c 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -371,29 +371,48 @@ static int generate_new_data_csums(struct btrfs_fs_info *fs_info, u16 new_csum_t
return generate_new_data_csums_range(fs_info, 0, new_csum_type);
}
+/* After deleting/modifying this many leaves, commit a transaction. */
+#define CSUM_CHANGE_LEAVES_THRESHOLD 32
+
static int delete_old_data_csums(struct btrfs_fs_info *fs_info)
{
struct btrfs_root *csum_root = btrfs_csum_root(fs_info, 0);
- struct btrfs_trans_handle *trans;
+ struct btrfs_trans_handle *trans = 0;
struct btrfs_path path = { 0 };
struct btrfs_key last_key;
+ unsigned int deleted_leaves = 0;
int ret;
last_key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
last_key.type = BTRFS_EXTENT_CSUM_KEY;
last_key.offset = (u64)-1;
- trans = btrfs_start_transaction(csum_root, 1);
- if (IS_ERR(trans)) {
- ret = PTR_ERR(trans);
- errno = -ret;
- error("failed to start transaction to delete old data csums: %m");
- return ret;
- }
while (true) {
int start_slot;
int nr;
+ if (deleted_leaves >= CSUM_CHANGE_LEAVES_THRESHOLD) {
+ assert(trans);
+ ret = btrfs_commit_transaction(trans, csum_root);
+ if (ret < 0) {
+ errno = -ret;
+ error(
+ "failed to commit transaction to delete old data csums: %m");
+ return ret;
+ }
+ trans = NULL;
+ deleted_leaves = 0;
+ }
+ if (!trans) {
+ trans = btrfs_start_transaction(csum_root, 1);
+ if (IS_ERR(trans)) {
+ ret = PTR_ERR(trans);
+ errno = -ret;
+ error(
+ "failed to start transaction to delete old data csums: %m");
+ return ret;
+ }
+ }
ret = btrfs_search_slot(trans, csum_root, &last_key, &path, -1, 1);
if (ret < 0) {
errno = -ret;
@@ -428,6 +447,7 @@ static int delete_old_data_csums(struct btrfs_fs_info *fs_info)
break;
}
btrfs_release_path(&path);
+ deleted_leaves++;
}
btrfs_release_path(&path);
if (ret < 0)
@@ -445,9 +465,10 @@ static int delete_old_data_csums(struct btrfs_fs_info *fs_info)
static int change_csum_objectids(struct btrfs_fs_info *fs_info)
{
struct btrfs_root *csum_root = btrfs_csum_root(fs_info, 0);
- struct btrfs_trans_handle *trans;
+ struct btrfs_trans_handle *trans = NULL;
struct btrfs_path path = { 0 };
struct btrfs_key last_key;
+ unsigned int changed_leaves = 0;
u64 super_flags;
int ret = 0;
@@ -455,17 +476,32 @@ static int change_csum_objectids(struct btrfs_fs_info *fs_info)
last_key.type = BTRFS_EXTENT_CSUM_KEY;
last_key.offset = (u64)-1;
- trans = btrfs_start_transaction(csum_root, 1);
- if (IS_ERR(trans)) {
- ret = PTR_ERR(trans);
- errno = -ret;
- error("failed to start transaction to change csum objectids: %m");
- return ret;
- }
while (true) {
struct btrfs_key found_key;
int nr;
+ if (changed_leaves >= CSUM_CHANGE_LEAVES_THRESHOLD) {
+ assert(trans);
+ ret = btrfs_commit_transaction(trans, csum_root);
+ if (ret < 0) {
+ errno = -ret;
+ error(
+ "failed to commit transaction to change data csum objectid: %m");
+ return ret;
+ }
+ trans = NULL;
+ changed_leaves = 0;
+ }
+ if (!trans) {
+ trans = btrfs_start_transaction(csum_root, 1);
+ if (IS_ERR(trans)) {
+ ret = PTR_ERR(trans);
+ errno = -ret;
+ error(
+ "failed to start transaction to change csum objectids: %m");
+ return ret;
+ }
+ }
ret = btrfs_search_slot(trans, csum_root, &last_key, &path, 0, 1);
if (ret < 0)
goto out;
--
2.45.2
^ permalink raw reply related [flat|nested] 4+ messages in thread