diff mbox

btrfs: handle errors while updating refcounts in update_ref_for_cow

Message ID 20171121185849.22602-1-jeffm@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeff Mahoney Nov. 21, 2017, 6:58 p.m. UTC 
From: Jeff Mahoney <jeffm@suse.com>

Since commit fb235dc06fa (btrfs: qgroup: Move half of the qgroup
accounting time out of commit trans) the assumption that
btrfs_add_delayed_{data,tree}_ref can only return 0 or -ENOMEM has
been false.  The qgroup operations call into btrfs_search_slot
and friends and can now return the full spectrum of error codes.

Fortunately, the fix here is easy since update_ref_for_cow failing
is already handled so we just need to bail early with the error
code.

Fixes: fb235dc06fa (btrfs: qgroup: Move half of the qgroup accounting ...)
Cc: <stable@vger.kernel.org> # v4.11+
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 fs/btrfs/ctree.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Edmund Nadolski Nov. 21, 2017, 9:40 p.m. UTC | #1 
On 11/21/2017 11:58 AM, jeffm@suse.com wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> Since commit fb235dc06fa (btrfs: qgroup: Move half of the qgroup
> accounting time out of commit trans) the assumption that
> btrfs_add_delayed_{data,tree}_ref can only return 0 or -ENOMEM has
> been false.  The qgroup operations call into btrfs_search_slot
> and friends and can now return the full spectrum of error codes.
> 
> Fortunately, the fix here is easy since update_ref_for_cow failing
> is already handled so we just need to bail early with the error
> code.
> 
> Fixes: fb235dc06fa (btrfs: qgroup: Move half of the qgroup accounting ...)
> Cc: <stable@vger.kernel.org> # v4.11+
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>

Reviewed-by: Edmund Nadolski <enadolski@suse.com>

> ---
>  fs/btrfs/ctree.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 531e0a8645b0..1e74cf826532 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1032,14 +1032,17 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
>  		     root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID) &&
>  		    !(flags & BTRFS_BLOCK_FLAG_FULL_BACKREF)) {
>  			ret = btrfs_inc_ref(trans, root, buf, 1);
> -			BUG_ON(ret); /* -ENOMEM */
> +			if (ret)
> +				return ret;
>  
>  			if (root->root_key.objectid ==
>  			    BTRFS_TREE_RELOC_OBJECTID) {
>  				ret = btrfs_dec_ref(trans, root, buf, 0);
> -				BUG_ON(ret); /* -ENOMEM */
> +				if (ret)
> +					return ret;
>  				ret = btrfs_inc_ref(trans, root, cow, 1);
> -				BUG_ON(ret); /* -ENOMEM */
> +				if (ret)
> +					return ret;
>  			}
>  			new_flags |= BTRFS_BLOCK_FLAG_FULL_BACKREF;
>  		} else {
> @@ -1049,7 +1052,8 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
>  				ret = btrfs_inc_ref(trans, root, cow, 1);
>  			else
>  				ret = btrfs_inc_ref(trans, root, cow, 0);
> -			BUG_ON(ret); /* -ENOMEM */
> +			if (ret)
> +				return ret;
>  		}
>  		if (new_flags != 0) {
>  			int level = btrfs_header_level(buf);
> @@ -1068,9 +1072,11 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
>  				ret = btrfs_inc_ref(trans, root, cow, 1);
>  			else
>  				ret = btrfs_inc_ref(trans, root, cow, 0);
> -			BUG_ON(ret); /* -ENOMEM */
> +			if (ret)
> +				return ret;
>  			ret = btrfs_dec_ref(trans, root, buf, 1);
> -			BUG_ON(ret); /* -ENOMEM */
> +			if (ret)
> +				return ret;
>  		}
>  		clean_tree_block(fs_info, buf);
>  		*last_ref = 1;
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Qu Wenruo Nov. 22, 2017, 9:06 a.m. UTC | #2 
On 2017年11月22日 02:58, jeffm@suse.com wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> Since commit fb235dc06fa (btrfs: qgroup: Move half of the qgroup
> accounting time out of commit trans) the assumption that
> btrfs_add_delayed_{data,tree}_ref can only return 0 or -ENOMEM has
> been false.  The qgroup operations call into btrfs_search_slot
> and friends and can now return the full spectrum of error codes.
> 
> Fortunately, the fix here is easy since update_ref_for_cow failing
> is already handled so we just need to bail early with the error
> code.
> 
> Fixes: fb235dc06fa (btrfs: qgroup: Move half of the qgroup accounting ...)
> Cc: <stable@vger.kernel.org> # v4.11+
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>

Looks good.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/ctree.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 531e0a8645b0..1e74cf826532 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1032,14 +1032,17 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
>  		     root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID) &&
>  		    !(flags & BTRFS_BLOCK_FLAG_FULL_BACKREF)) {
>  			ret = btrfs_inc_ref(trans, root, buf, 1);
> -			BUG_ON(ret); /* -ENOMEM */
> +			if (ret)
> +				return ret;
>  
>  			if (root->root_key.objectid ==
>  			    BTRFS_TREE_RELOC_OBJECTID) {
>  				ret = btrfs_dec_ref(trans, root, buf, 0);
> -				BUG_ON(ret); /* -ENOMEM */
> +				if (ret)
> +					return ret;
>  				ret = btrfs_inc_ref(trans, root, cow, 1);
> -				BUG_ON(ret); /* -ENOMEM */
> +				if (ret)
> +					return ret;
>  			}
>  			new_flags |= BTRFS_BLOCK_FLAG_FULL_BACKREF;
>  		} else {
> @@ -1049,7 +1052,8 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
>  				ret = btrfs_inc_ref(trans, root, cow, 1);
>  			else
>  				ret = btrfs_inc_ref(trans, root, cow, 0);
> -			BUG_ON(ret); /* -ENOMEM */
> +			if (ret)
> +				return ret;
>  		}
>  		if (new_flags != 0) {
>  			int level = btrfs_header_level(buf);
> @@ -1068,9 +1072,11 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
>  				ret = btrfs_inc_ref(trans, root, cow, 1);
>  			else
>  				ret = btrfs_inc_ref(trans, root, cow, 0);
> -			BUG_ON(ret); /* -ENOMEM */
> +			if (ret)
> +				return ret;
>  			ret = btrfs_dec_ref(trans, root, buf, 1);
> -			BUG_ON(ret); /* -ENOMEM */
> +			if (ret)
> +				return ret;
>  		}
>  		clean_tree_block(fs_info, buf);
>  		*last_ref = 1;
>
diff mbox

Patch

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 531e0a8645b0..1e74cf826532 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1032,14 +1032,17 @@  static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
 		     root->root_key.objectid == BTRFS_TREE_RELOC_OBJECTID) &&
 		    !(flags & BTRFS_BLOCK_FLAG_FULL_BACKREF)) {
 			ret = btrfs_inc_ref(trans, root, buf, 1);
-			BUG_ON(ret); /* -ENOMEM */
+			if (ret)
+				return ret;
 
 			if (root->root_key.objectid ==
 			    BTRFS_TREE_RELOC_OBJECTID) {
 				ret = btrfs_dec_ref(trans, root, buf, 0);
-				BUG_ON(ret); /* -ENOMEM */
+				if (ret)
+					return ret;
 				ret = btrfs_inc_ref(trans, root, cow, 1);
-				BUG_ON(ret); /* -ENOMEM */
+				if (ret)
+					return ret;
 			}
 			new_flags |= BTRFS_BLOCK_FLAG_FULL_BACKREF;
 		} else {
@@ -1049,7 +1052,8 @@  static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
 				ret = btrfs_inc_ref(trans, root, cow, 1);
 			else
 				ret = btrfs_inc_ref(trans, root, cow, 0);
-			BUG_ON(ret); /* -ENOMEM */
+			if (ret)
+				return ret;
 		}
 		if (new_flags != 0) {
 			int level = btrfs_header_level(buf);
@@ -1068,9 +1072,11 @@  static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
 				ret = btrfs_inc_ref(trans, root, cow, 1);
 			else
 				ret = btrfs_inc_ref(trans, root, cow, 0);
-			BUG_ON(ret); /* -ENOMEM */
+			if (ret)
+				return ret;
 			ret = btrfs_dec_ref(trans, root, buf, 1);
-			BUG_ON(ret); /* -ENOMEM */
+			if (ret)
+				return ret;
 		}
 		clean_tree_block(fs_info, buf);
 		*last_ref = 1;