All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH] btrfs-progs: do not abuse btrfs_commit_transaction() just to update super blocks
Date: Wed, 18 May 2022 19:23:00 +0800	[thread overview]
Message-ID: <4ee1030eb7d24d13ffa1bda98364de07258ca079.1652872975.git.wqu@suse.com> (raw)

There are several call sites utilizing btrfs_commit_transaction() just
to update members in super blocks, without any metadata update.

This can be problematic for some simple call sites, like zero_log_tree()
or check_and_repair_super_num_devs().

If we have big problems preventing the fs to be mounted in the first
place, and need to clear the log or super block size, but by some other
problems in extent tree, we're unable to allocate new blocks.

Then we fall into a deadlock that, we need to mount (even
ro,rescue=all) to collect extra info, but btrfs-progs can not do any
super block updates.

Fix the problem by allowing the following super blocks only operations
to be done without using btrfs_commit_transaction():

- btrfs_fix_super_size()
- check_and_repair_super_num_devs()
- zero_log_tree().

There are some exceptions in btrfstune.c, related to the csum type
conversion and seed flags.

In those btrfstune cases, we in fact wants to proper error report in
btrfs_commit_transaction(), as those operations are not mount critical,
and any early error can be helpful to expose any problems in the fs.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c            |  8 +-------
 check/mode-common.c     | 16 +++-------------
 kernel-shared/volumes.c | 14 ++------------
 3 files changed, 6 insertions(+), 32 deletions(-)

diff --git a/check/main.c b/check/main.c
index bcb016964e7a..03df343796c2 100644
--- a/check/main.c
+++ b/check/main.c
@@ -9659,17 +9659,11 @@ out:
 
 static int zero_log_tree(struct btrfs_root *root)
 {
-	struct btrfs_trans_handle *trans;
 	int ret;
 
-	trans = btrfs_start_transaction(root, 1);
-	if (IS_ERR(trans)) {
-		ret = PTR_ERR(trans);
-		return ret;
-	}
 	btrfs_set_super_log_root(gfs_info->super_copy, 0);
 	btrfs_set_super_log_root_level(gfs_info->super_copy, 0);
-	ret = btrfs_commit_transaction(trans, root);
+	ret = write_all_supers(fs_info);
 	return ret;
 }
 
diff --git a/check/mode-common.c b/check/mode-common.c
index 2a3018428fc8..f7cb00c0f7b2 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -1628,7 +1628,6 @@ static int get_num_devs_in_chunk_tree(struct btrfs_fs_info *fs_info)
 
 int check_and_repair_super_num_devs(struct btrfs_fs_info *fs_info)
 {
-	struct btrfs_trans_handle *trans;
 	int found_devs;
 	int ret;
 
@@ -1651,22 +1650,13 @@ int check_and_repair_super_num_devs(struct btrfs_fs_info *fs_info)
 
 	/*
 	 * Repair is pretty simple, just reset the super block value and
-	 * commit a new transaction.
+	 * write back all the super blocks.
 	 */
-	trans = btrfs_start_transaction(fs_info->tree_root, 0);
-	if (IS_ERR(trans)) {
-		ret = PTR_ERR(trans);
-		errno = -ret;
-		error("failed to start trans: %m");
-		return ret;
-	}
-
 	btrfs_set_super_num_devices(fs_info->super_copy, found_devs);
-	ret = btrfs_commit_transaction(trans, fs_info->tree_root);
+	ret = write_all_supers(fs_info);
 	if (ret < 0) {
 		errno = -ret;
-		error("failed to commit trans: %m");
-		btrfs_abort_transaction(trans, ret);
+		error("failed to write super blocks: %m");
 		return ret;
 	}
 	printf("Successfully reset super num devices to %u\n", found_devs);
diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 97c09a1a4931..9a3b69903d32 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -2861,7 +2861,6 @@ err:
  */
 int btrfs_fix_super_size(struct btrfs_fs_info *fs_info)
 {
-	struct btrfs_trans_handle *trans;
 	struct btrfs_device *device;
 	struct list_head *dev_list = &fs_info->fs_devices->devices;
 	u64 total_bytes = 0;
@@ -2886,19 +2885,10 @@ int btrfs_fix_super_size(struct btrfs_fs_info *fs_info)
 		return 0;
 
 	btrfs_set_super_total_bytes(fs_info->super_copy, total_bytes);
-
-	/* Commit transaction to update all super blocks */
-	trans = btrfs_start_transaction(fs_info->tree_root, 1);
-	if (IS_ERR(trans)) {
-		ret = PTR_ERR(trans);
-		errno = -ret;
-		error("error starting transaction: %d (%m)", ret);
-		return ret;
-	}
-	ret = btrfs_commit_transaction(trans, fs_info->tree_root);
+	ret = write_all_supers(fs_info);
 	if (ret < 0) {
 		errno = -ret;
-		error("failed to commit current transaction: %d (%m)", ret);
+		error("failed to write super blocks: %d (%m)", ret);
 		return ret;
 	}
 	printf("Fixed super total bytes, old size: %llu new size: %llu\n",
-- 
2.36.1


             reply	other threads:[~2022-05-18 11:23 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-18 11:23 Qu Wenruo [this message]
2022-05-20 13:38 ` [PATCH] btrfs-progs: do not abuse btrfs_commit_transaction() just to update super blocks 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=4ee1030eb7d24d13ffa1bda98364de07258ca079.1652872975.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.