diff mbox series

[v2,33/42] btrfs: handle extent reference errors in do_relocation

Message ID 9c5ebe55c78553d1a07559f63d26a446d5642bc3.1605284383.git.josef@toxicpanda.com (mailing list archive)
State New, archived
Headers show
Series [v2,01/42] btrfs: allow error injection for btrfs_search_slot and btrfs_cow_block | expand

Commit Message

Josef Bacik Nov. 13, 2020, 4:23 p.m. UTC
We can already deal with errors appropriately from do_relocation, simply
handle any errors that come from changing the refs at this point
cleanly.  We have to abort the transaction if we fail here as we've
modified metadata at this point.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/relocation.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Nikolay Borisov Nov. 24, 2020, 1:15 p.m. UTC | #1
On 13.11.20 г. 18:23 ч., Josef Bacik wrote:
> We can already deal with errors appropriately from do_relocation, simply
> handle any errors that come from changing the refs at this point
> cleanly.  We have to abort the transaction if we fail here as we've
> modified metadata at this point.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/relocation.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 74d52fc457c7..bc63c4bb4057 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -2447,10 +2447,12 @@ static int do_relocation(struct btrfs_trans_handle *trans,
>  			btrfs_init_tree_ref(&ref, node->level,
>  					    btrfs_header_owner(upper->eb));
>  			ret = btrfs_inc_extent_ref(trans, &ref);
> -			BUG_ON(ret);
> -
> -			ret = btrfs_drop_subtree(trans, root, eb, upper->eb);
> -			BUG_ON(ret);
> +			if (!ret)
> +				btrfs_drop_subtree(trans, root, eb, upper->eb);
> +			if (ret) {
> +				btrfs_abort_transaction(trans, ret);
> +				err = ret;
> +			}

nit: I'd prefer this: 

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index bc63c4bb4057..ce063c83d337 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2447,12 +2447,12 @@ static int do_relocation(struct btrfs_trans_handle *trans,
                        btrfs_init_tree_ref(&ref, node->level,
                                            btrfs_header_owner(upper->eb));
                        ret = btrfs_inc_extent_ref(trans, &ref);
-                       if (!ret)
-                               btrfs_drop_subtree(trans, root, eb, upper->eb);
                        if (ret) {
                                btrfs_abort_transaction(trans, ret);
                                err = ret;
+                               goto next;
                        }
+                       btrfs_drop_subtree(trans, root, eb, upper->eb);

One less conditional statement to worry about

>  		}
>  next:
>  		if (!upper->pending)
>
diff mbox series

Patch

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 74d52fc457c7..bc63c4bb4057 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2447,10 +2447,12 @@  static int do_relocation(struct btrfs_trans_handle *trans,
 			btrfs_init_tree_ref(&ref, node->level,
 					    btrfs_header_owner(upper->eb));
 			ret = btrfs_inc_extent_ref(trans, &ref);
-			BUG_ON(ret);
-
-			ret = btrfs_drop_subtree(trans, root, eb, upper->eb);
-			BUG_ON(ret);
+			if (!ret)
+				btrfs_drop_subtree(trans, root, eb, upper->eb);
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
+				err = ret;
+			}
 		}
 next:
 		if (!upper->pending)