All of lore.kernel.org
 help / color / mirror / Atom feed
From: fdmanana@kernel.org
To: linux-btrfs@vger.kernel.org
Subject: [PATCH v2 13/13] btrfs: do not BUG_ON() on tree mod log failures at btrfs_del_ptr()
Date: Thu,  8 Jun 2023 11:27:49 +0100	[thread overview]
Message-ID: <a48727b926b88fc790a90c8e69008d4d87035ea6.1686219923.git.fdmanana@suse.com> (raw)
In-Reply-To: <cover.1686219923.git.fdmanana@suse.com>

From: Filipe Manana <fdmanana@suse.com>

At btrfs_del_ptr(), instead of doing a BUG_ON() in case we fail to record
tree mod log operations, do a transaction abort and return the error to
the callers. There's really no need for the BUG_ON() as we can release all
resources in the context of all callers, and we have to abort because other
future tree searches that use the tree mod log (btrfs_search_old_slot())
may get inconsistent results if other operations modify the tree after
that failure and before the tree mod log based search.

This implies btrfs_del_ptr() return an int instead of void, and making all
callers check for returned errors.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ctree.c | 52 ++++++++++++++++++++++++++++++++++++------------
 fs/btrfs/ctree.h |  4 ++--
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 0188cf6e30bf..29c5fa252eb1 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1139,7 +1139,12 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
 		if (btrfs_header_nritems(right) == 0) {
 			btrfs_clear_buffer_dirty(trans, right);
 			btrfs_tree_unlock(right);
-			btrfs_del_ptr(root, path, level + 1, pslot + 1);
+			ret = btrfs_del_ptr(trans, root, path, level + 1, pslot + 1);
+			if (ret < 0) {
+				free_extent_buffer_stale(right);
+				right = NULL;
+				goto out;
+			}
 			root_sub_used(root, right->len);
 			btrfs_free_tree_block(trans, btrfs_root_id(root), right,
 					      0, 1);
@@ -1192,7 +1197,12 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
 	if (btrfs_header_nritems(mid) == 0) {
 		btrfs_clear_buffer_dirty(trans, mid);
 		btrfs_tree_unlock(mid);
-		btrfs_del_ptr(root, path, level + 1, pslot);
+		ret = btrfs_del_ptr(trans, root, path, level + 1, pslot);
+		if (ret < 0) {
+			free_extent_buffer_stale(mid);
+			mid = NULL;
+			goto out;
+		}
 		root_sub_used(root, mid->len);
 		btrfs_free_tree_block(trans, btrfs_root_id(root), mid, 0, 1);
 		free_extent_buffer_stale(mid);
@@ -4440,8 +4450,8 @@ int btrfs_duplicate_item(struct btrfs_trans_handle *trans,
  *
  * This is exported for use inside btrfs-progs, don't un-export it.
  */
-void btrfs_del_ptr(struct btrfs_root *root, struct btrfs_path *path, int level,
-		   int slot)
+int btrfs_del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+		  struct btrfs_path *path, int level, int slot)
 {
 	struct extent_buffer *parent = path->nodes[level];
 	u32 nritems;
@@ -4452,7 +4462,10 @@ void btrfs_del_ptr(struct btrfs_root *root, struct btrfs_path *path, int level,
 		if (level) {
 			ret = btrfs_tree_mod_log_insert_move(parent, slot,
 					slot + 1, nritems - slot - 1);
-			BUG_ON(ret < 0);
+			if (ret < 0) {
+				btrfs_abort_transaction(trans, ret);
+				return ret;
+			}
 		}
 		memmove_extent_buffer(parent,
 			      btrfs_node_key_ptr_offset(parent, slot),
@@ -4462,7 +4475,10 @@ void btrfs_del_ptr(struct btrfs_root *root, struct btrfs_path *path, int level,
 	} else if (level) {
 		ret = btrfs_tree_mod_log_insert_key(parent, slot,
 						    BTRFS_MOD_LOG_KEY_REMOVE);
-		BUG_ON(ret < 0);
+		if (ret < 0) {
+			btrfs_abort_transaction(trans, ret);
+			return ret;
+		}
 	}
 
 	nritems--;
@@ -4478,6 +4494,7 @@ void btrfs_del_ptr(struct btrfs_root *root, struct btrfs_path *path, int level,
 		fixup_low_keys(path, &disk_key, level + 1);
 	}
 	btrfs_mark_buffer_dirty(parent);
+	return 0;
 }
 
 /*
@@ -4490,13 +4507,17 @@ void btrfs_del_ptr(struct btrfs_root *root, struct btrfs_path *path, int level,
  * The path must have already been setup for deleting the leaf, including
  * all the proper balancing.  path->nodes[1] must be locked.
  */
-static noinline void btrfs_del_leaf(struct btrfs_trans_handle *trans,
-				    struct btrfs_root *root,
-				    struct btrfs_path *path,
-				    struct extent_buffer *leaf)
+static noinline int btrfs_del_leaf(struct btrfs_trans_handle *trans,
+				   struct btrfs_root *root,
+				   struct btrfs_path *path,
+				   struct extent_buffer *leaf)
 {
+	int ret;
+
 	WARN_ON(btrfs_header_generation(leaf) != trans->transid);
-	btrfs_del_ptr(root, path, 1, path->slots[1]);
+	ret = btrfs_del_ptr(trans, root, path, 1, path->slots[1]);
+	if (ret < 0)
+		return ret;
 
 	/*
 	 * btrfs_free_extent is expensive, we want to make sure we
@@ -4509,6 +4530,7 @@ static noinline void btrfs_del_leaf(struct btrfs_trans_handle *trans,
 	atomic_inc(&leaf->refs);
 	btrfs_free_tree_block(trans, btrfs_root_id(root), leaf, 0, 1);
 	free_extent_buffer_stale(leaf);
+	return 0;
 }
 /*
  * delete the item at the leaf level in path.  If that empties
@@ -4558,7 +4580,9 @@ int btrfs_del_items(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 			btrfs_set_header_level(leaf, 0);
 		} else {
 			btrfs_clear_buffer_dirty(trans, leaf);
-			btrfs_del_leaf(trans, root, path, leaf);
+			ret = btrfs_del_leaf(trans, root, path, leaf);
+			if (ret < 0)
+				return ret;
 		}
 	} else {
 		int used = leaf_space_used(leaf, 0, nritems);
@@ -4619,7 +4643,9 @@ int btrfs_del_items(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 
 			if (btrfs_header_nritems(leaf) == 0) {
 				path->slots[1] = slot;
-				btrfs_del_leaf(trans, root, path, leaf);
+				ret = btrfs_del_leaf(trans, root, path, leaf);
+				if (ret < 0)
+					return ret;
 				free_extent_buffer(leaf);
 				ret = 0;
 			} else {
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 5af61480dde6..f2d2b313bde5 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -541,8 +541,8 @@ int btrfs_copy_root(struct btrfs_trans_handle *trans,
 		      struct extent_buffer **cow_ret, u64 new_root_objectid);
 int btrfs_block_can_be_shared(struct btrfs_root *root,
 			      struct extent_buffer *buf);
-void btrfs_del_ptr(struct btrfs_root *root, struct btrfs_path *path, int level,
-		   int slot);
+int btrfs_del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+		  struct btrfs_path *path, int level, int slot);
 void btrfs_extend_item(struct btrfs_path *path, u32 data_size);
 void btrfs_truncate_item(struct btrfs_path *path, u32 new_size, int from_end);
 int btrfs_split_item(struct btrfs_trans_handle *trans,
-- 
2.34.1


  parent reply	other threads:[~2023-06-08 10:28 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-07 19:24 [PATCH 00/13] btrfs: some fixes and updates around handling errors for tree mod log operations fdmanana
2023-06-07 19:24 ` [PATCH 01/13] btrfs: add missing error handling when logging operation while COWing extent buffer fdmanana
2023-06-08  9:25   ` Qu Wenruo
2023-06-07 19:24 ` [PATCH 02/13] btrfs: fix extent buffer leak after failure tree mod log failure at split_node() fdmanana
2023-06-08  8:40   ` Qu Wenruo
2023-06-07 19:24 ` [PATCH 03/13] btrfs: avoid tree mod log ENOMEM failures when we don't need to log fdmanana
2023-06-07 19:24 ` [PATCH 04/13] btrfs: do not BUG_ON() on tree mod log failure at __btrfs_cow_block() fdmanana
2023-06-08  8:48   ` Qu Wenruo
2023-06-07 19:24 ` [PATCH 05/13] btrfs: do not BUG_ON() on tree mod log failure at balance_level() fdmanana
2023-06-08  8:52   ` Qu Wenruo
2023-06-07 19:24 ` [PATCH 06/13] btrfs: rename enospc label to out " fdmanana
2023-06-08  8:53   ` Qu Wenruo
2023-06-08  9:21   ` Anand Jain
2023-06-07 19:24 ` [PATCH 07/13] btrfs: avoid unnecessarily setting the fs to RO and error state " fdmanana
2023-06-07 19:24 ` [PATCH 08/13] btrfs: abort transaction at balance_level() when left child is missing fdmanana
2023-06-08  8:57   ` Qu Wenruo
2023-06-08  9:47     ` Filipe Manana
2023-06-08 12:26       ` David Sterba
2023-06-07 19:24 ` [PATCH 09/13] btrfs: abort transaction at update_ref_for_cow() when ref count is zero fdmanana
2023-06-08  8:58   ` Qu Wenruo
2023-06-08  9:47     ` Filipe Manana
2023-06-07 19:24 ` [PATCH 10/13] btrfs: do not BUG_ON() on tree mod log failures at push_nodes_for_insert() fdmanana
2023-06-08  9:02   ` Qu Wenruo
2023-06-08  9:46     ` Filipe Manana
2023-06-08 10:19       ` Qu Wenruo
2023-06-07 19:24 ` [PATCH 11/13] btrfs: do not BUG_ON() on tree mod log failure at insert_new_root() fdmanana
2023-06-08  9:11   ` Qu Wenruo
2023-06-07 19:24 ` [PATCH 12/13] btrfs: do not BUG_ON() on tree mod log failures at insert_ptr() fdmanana
2023-06-08  9:16   ` Qu Wenruo
2023-06-08  9:43     ` Filipe Manana
2023-06-07 19:24 ` [PATCH 13/13] btrfs: do not BUG_ON() on tree mod log failures at btrfs_del_ptr() fdmanana
2023-06-08  9:19   ` Qu Wenruo
2023-06-08 10:27 ` [PATCH v2 00/13] btrfs: some fixes and updates around handling errors for tree mod log operations fdmanana
2023-06-08 10:27   ` [PATCH v2 01/13] btrfs: add missing error handling when logging operation while COWing extent buffer fdmanana
2023-06-08 10:27   ` [PATCH v2 02/13] btrfs: fix extent buffer leak after tree mod log failure at split_node() fdmanana
2023-06-08 10:27   ` [PATCH v2 03/13] btrfs: avoid tree mod log ENOMEM failures when we don't need to log fdmanana
2023-06-08 10:27   ` [PATCH v2 04/13] btrfs: do not BUG_ON() on tree mod log failure at __btrfs_cow_block() fdmanana
2023-06-08 10:27   ` [PATCH v2 05/13] btrfs: do not BUG_ON() on tree mod log failure at balance_level() fdmanana
2023-06-08 10:27   ` [PATCH v2 06/13] btrfs: rename enospc label to out " fdmanana
2023-06-08 10:27   ` [PATCH v2 07/13] btrfs: avoid unnecessarily setting the fs to RO and error state " fdmanana
2023-06-08 10:51     ` Qu Wenruo
2023-06-08 11:00       ` Filipe Manana
2023-06-08 11:04         ` Qu Wenruo
2023-06-08 10:27   ` [PATCH v2 08/13] btrfs: abort transaction at balance_level() when left child is missing fdmanana
2023-06-08 10:37     ` Qu Wenruo
2023-06-08 10:52     ` Qu Wenruo
2023-06-08 10:27   ` [PATCH v2 09/13] btrfs: abort transaction at update_ref_for_cow() when ref count is zero fdmanana
2023-06-08 10:52     ` Qu Wenruo
2023-06-08 10:27   ` [PATCH v2 10/13] btrfs: do not BUG_ON() on tree mod log failures at push_nodes_for_insert() fdmanana
2023-06-08 10:27   ` [PATCH v2 11/13] btrfs: do not BUG_ON() on tree mod log failure at insert_new_root() fdmanana
2023-06-08 10:27   ` [PATCH v2 12/13] btrfs: do not BUG_ON() on tree mod log failures at insert_ptr() fdmanana
2023-06-08 10:27   ` fdmanana [this message]
2023-06-09 16:51   ` [PATCH v2 00/13] btrfs: some fixes and updates around handling errors for tree mod log operations David Sterba
2023-06-09 17:20   ` 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=a48727b926b88fc790a90c8e69008d4d87035ea6.1686219923.git.fdmanana@suse.com \
    --to=fdmanana@kernel.org \
    --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.