Skip to content

Commit 6ef8fbc

Browse files
fdmananakdave
authored andcommitted
btrfs: fix missing error handling when adding delayed ref with qgroups enabled
When adding a delayed ref head, at delayed-ref.c:add_delayed_ref_head(), if we fail to insert the qgroup record we don't error out, we ignore it. In fact we treat it as if there was no error and there was already an existing record - we don't distinguish between the cases where btrfs_qgroup_trace_extent_nolock() returns 1, meaning a record already existed and we can free the given record, and the case where it returns a negative error value, meaning the insertion into the xarray that is used to track records failed. Effectively we end up ignoring that we are lacking qgroup record in the dirty extents xarray, resulting in incorrect qgroup accounting. Fix this by checking for errors and return them to the callers. Fixes: 3cce39a ("btrfs: qgroup: use xarray to track dirty extents in transaction") Reviewed-by: Qu Wenruo <[email protected]> Signed-off-by: Filipe Manana <[email protected]> Reviewed-by: David Sterba <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 6931385 commit 6ef8fbc

File tree

1 file changed

+33
-9
lines changed

1 file changed

+33
-9
lines changed

fs/btrfs/delayed-ref.c

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,8 @@ static void init_delayed_ref_head(struct btrfs_delayed_ref_head *head_ref,
840840
* helper function to actually insert a head node into the rbtree.
841841
* this does all the dirty work in terms of maintaining the correct
842842
* overall modification count.
843+
*
844+
* Returns an error pointer in case of an error.
843845
*/
844846
static noinline struct btrfs_delayed_ref_head *
845847
add_delayed_ref_head(struct btrfs_trans_handle *trans,
@@ -862,6 +864,9 @@ add_delayed_ref_head(struct btrfs_trans_handle *trans,
862864
if (ret) {
863865
/* Clean up if insertion fails or item exists. */
864866
xa_release(&delayed_refs->dirty_extents, qrecord->bytenr);
867+
/* Caller responsible for freeing qrecord on error. */
868+
if (ret < 0)
869+
return ERR_PTR(ret);
865870
kfree(qrecord);
866871
} else {
867872
qrecord_inserted = true;
@@ -1000,27 +1005,35 @@ static int add_delayed_ref(struct btrfs_trans_handle *trans,
10001005
struct btrfs_fs_info *fs_info = trans->fs_info;
10011006
struct btrfs_delayed_ref_node *node;
10021007
struct btrfs_delayed_ref_head *head_ref;
1008+
struct btrfs_delayed_ref_head *new_head_ref;
10031009
struct btrfs_delayed_ref_root *delayed_refs;
10041010
struct btrfs_qgroup_extent_record *record = NULL;
10051011
bool qrecord_inserted;
10061012
int action = generic_ref->action;
10071013
bool merged;
1014+
int ret;
10081015

10091016
node = kmem_cache_alloc(btrfs_delayed_ref_node_cachep, GFP_NOFS);
10101017
if (!node)
10111018
return -ENOMEM;
10121019

10131020
head_ref = kmem_cache_alloc(btrfs_delayed_ref_head_cachep, GFP_NOFS);
1014-
if (!head_ref)
1021+
if (!head_ref) {
1022+
ret = -ENOMEM;
10151023
goto free_node;
1024+
}
10161025

10171026
if (btrfs_qgroup_full_accounting(fs_info) && !generic_ref->skip_qgroup) {
10181027
record = kzalloc(sizeof(*record), GFP_NOFS);
1019-
if (!record)
1028+
if (!record) {
1029+
ret = -ENOMEM;
10201030
goto free_head_ref;
1031+
}
10211032
if (xa_reserve(&trans->transaction->delayed_refs.dirty_extents,
1022-
generic_ref->bytenr, GFP_NOFS))
1033+
generic_ref->bytenr, GFP_NOFS)) {
1034+
ret = -ENOMEM;
10231035
goto free_record;
1036+
}
10241037
}
10251038

10261039
init_delayed_ref_common(fs_info, node, generic_ref);
@@ -1034,8 +1047,14 @@ static int add_delayed_ref(struct btrfs_trans_handle *trans,
10341047
* insert both the head node and the new ref without dropping
10351048
* the spin lock
10361049
*/
1037-
head_ref = add_delayed_ref_head(trans, head_ref, record,
1038-
action, &qrecord_inserted);
1050+
new_head_ref = add_delayed_ref_head(trans, head_ref, record,
1051+
action, &qrecord_inserted);
1052+
if (IS_ERR(new_head_ref)) {
1053+
spin_unlock(&delayed_refs->lock);
1054+
ret = PTR_ERR(new_head_ref);
1055+
goto free_record;
1056+
}
1057+
head_ref = new_head_ref;
10391058

10401059
merged = insert_delayed_ref(trans, head_ref, node);
10411060
spin_unlock(&delayed_refs->lock);
@@ -1063,7 +1082,7 @@ static int add_delayed_ref(struct btrfs_trans_handle *trans,
10631082
kmem_cache_free(btrfs_delayed_ref_head_cachep, head_ref);
10641083
free_node:
10651084
kmem_cache_free(btrfs_delayed_ref_node_cachep, node);
1066-
return -ENOMEM;
1085+
return ret;
10671086
}
10681087

10691088
/*
@@ -1094,6 +1113,7 @@ int btrfs_add_delayed_extent_op(struct btrfs_trans_handle *trans,
10941113
struct btrfs_delayed_extent_op *extent_op)
10951114
{
10961115
struct btrfs_delayed_ref_head *head_ref;
1116+
struct btrfs_delayed_ref_head *head_ref_ret;
10971117
struct btrfs_delayed_ref_root *delayed_refs;
10981118
struct btrfs_ref generic_ref = {
10991119
.type = BTRFS_REF_METADATA,
@@ -1113,11 +1133,15 @@ int btrfs_add_delayed_extent_op(struct btrfs_trans_handle *trans,
11131133
delayed_refs = &trans->transaction->delayed_refs;
11141134
spin_lock(&delayed_refs->lock);
11151135

1116-
add_delayed_ref_head(trans, head_ref, NULL, BTRFS_UPDATE_DELAYED_HEAD,
1117-
NULL);
1118-
1136+
head_ref_ret = add_delayed_ref_head(trans, head_ref, NULL,
1137+
BTRFS_UPDATE_DELAYED_HEAD, NULL);
11191138
spin_unlock(&delayed_refs->lock);
11201139

1140+
if (IS_ERR(head_ref_ret)) {
1141+
kmem_cache_free(btrfs_delayed_ref_head_cachep, head_ref);
1142+
return PTR_ERR(head_ref_ret);
1143+
}
1144+
11211145
/*
11221146
* Need to update the delayed_refs_rsv with any changes we may have
11231147
* made.

0 commit comments

Comments
 (0)