Wang Shilong
2013-Apr-17 14:00 UTC
[PATCH V2] Btrfs: fix missing check about ulist_add() in qgroup.c
From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
ulist_add() may return -ENOMEM, fix missing check about
return value.
Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
---
Changelog v1->v2:
ulist_add() may return 1, and this is ok. For this case,
btrfs_qgroup_reserve() should return 0, otherwise, i get
a regression when testing btrfs quota with btrfs-next.
---
fs/btrfs/qgroup.c | 62 +++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 44 insertions(+), 18 deletions(-)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index e089fc1..5d2743d 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1261,7 +1261,10 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle
*trans,
ulist_reinit(tmp);
/* XXX id not needed */
- ulist_add(tmp, qg->qgroupid, (u64)(uintptr_t)qg, GFP_ATOMIC);
+ ret = ulist_add(tmp, qg->qgroupid,
+ (u64)(uintptr_t)qg, GFP_ATOMIC);
+ if (ret < 0)
+ goto unlock;
ULIST_ITER_INIT(&tmp_uiter);
while ((tmp_unode = ulist_next(tmp, &tmp_uiter))) {
struct btrfs_qgroup_list *glist;
@@ -1273,9 +1276,11 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle
*trans,
++qg->refcnt;
list_for_each_entry(glist, &qg->groups, next_group) {
- ulist_add(tmp, glist->group->qgroupid,
- (u64)(uintptr_t)glist->group,
- GFP_ATOMIC);
+ ret = ulist_add(tmp, glist->group->qgroupid,
+ (u64)(uintptr_t)glist->group,
+ GFP_ATOMIC);
+ if (ret < 0)
+ goto unlock;
}
}
}
@@ -1284,7 +1289,10 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle
*trans,
* step 2: walk from the new root
*/
ulist_reinit(tmp);
- ulist_add(tmp, qgroup->qgroupid, (uintptr_t)qgroup, GFP_ATOMIC);
+ ret = ulist_add(tmp, qgroup->qgroupid,
+ (uintptr_t)qgroup, GFP_ATOMIC);
+ if (ret < 0)
+ goto unlock;
ULIST_ITER_INIT(&uiter);
while ((unode = ulist_next(tmp, &uiter))) {
struct btrfs_qgroup *qg;
@@ -1305,8 +1313,10 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle
*trans,
qg->tag = seq;
list_for_each_entry(glist, &qg->groups, next_group) {
- ulist_add(tmp, glist->group->qgroupid,
- (uintptr_t)glist->group, GFP_ATOMIC);
+ ret = ulist_add(tmp, glist->group->qgroupid,
+ (uintptr_t)glist->group, GFP_ATOMIC);
+ if (ret < 0)
+ goto unlock;
}
}
@@ -1324,7 +1334,10 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle
*trans,
continue;
ulist_reinit(tmp);
- ulist_add(tmp, qg->qgroupid, (uintptr_t)qg, GFP_ATOMIC);
+ ret = ulist_add(tmp, qg->qgroupid,
+ (uintptr_t)qg, GFP_ATOMIC);
+ if (ret < 0)
+ goto unlock;
ULIST_ITER_INIT(&tmp_uiter);
while ((tmp_unode = ulist_next(tmp, &tmp_uiter))) {
struct btrfs_qgroup_list *glist;
@@ -1340,9 +1353,11 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle
*trans,
}
list_for_each_entry(glist, &qg->groups, next_group) {
- ulist_add(tmp, glist->group->qgroupid,
- (uintptr_t)glist->group,
- GFP_ATOMIC);
+ ret = ulist_add(tmp, glist->group->qgroupid,
+ (uintptr_t)glist->group,
+ GFP_ATOMIC);
+ if (ret < 0)
+ goto unlock;
}
}
}
@@ -1607,7 +1622,10 @@ int btrfs_qgroup_reserve(struct btrfs_root *root, u64
num_bytes)
ret = -ENOMEM;
goto out;
}
- ulist_add(ulist, qgroup->qgroupid, (uintptr_t)qgroup, GFP_ATOMIC);
+ ret = ulist_add(ulist, qgroup->qgroupid,
+ (uintptr_t)qgroup, GFP_ATOMIC);
+ if (ret < 0)
+ goto out;
ULIST_ITER_INIT(&uiter);
while ((unode = ulist_next(ulist, &uiter))) {
struct btrfs_qgroup *qg;
@@ -1630,11 +1648,13 @@ int btrfs_qgroup_reserve(struct btrfs_root *root, u64
num_bytes)
}
list_for_each_entry(glist, &qg->groups, next_group) {
- ulist_add(ulist, glist->group->qgroupid,
- (uintptr_t)glist->group, GFP_ATOMIC);
+ ret = ulist_add(ulist, glist->group->qgroupid,
+ (uintptr_t)glist->group, GFP_ATOMIC);
+ if (ret < 0)
+ goto out;
}
}
-
+ ret = 0;
/*
* no limits exceeded, now record the reservation into all qgroups
*/
@@ -1663,6 +1683,7 @@ void btrfs_qgroup_free(struct btrfs_root *root, u64
num_bytes)
struct ulist_node *unode;
struct ulist_iterator uiter;
u64 ref_root = root->root_key.objectid;
+ int ret = 0;
if (!is_fstree(ref_root))
return;
@@ -1685,7 +1706,10 @@ void btrfs_qgroup_free(struct btrfs_root *root, u64
num_bytes)
btrfs_std_error(fs_info, -ENOMEM);
goto out;
}
- ulist_add(ulist, qgroup->qgroupid, (uintptr_t)qgroup, GFP_ATOMIC);
+ ret = ulist_add(ulist, qgroup->qgroupid,
+ (uintptr_t)qgroup, GFP_ATOMIC);
+ if (ret < 0)
+ goto out;
ULIST_ITER_INIT(&uiter);
while ((unode = ulist_next(ulist, &uiter))) {
struct btrfs_qgroup *qg;
@@ -1696,8 +1720,10 @@ void btrfs_qgroup_free(struct btrfs_root *root, u64
num_bytes)
qg->reserved -= num_bytes;
list_for_each_entry(glist, &qg->groups, next_group) {
- ulist_add(ulist, glist->group->qgroupid,
- (uintptr_t)glist->group, GFP_ATOMIC);
+ ret = ulist_add(ulist, glist->group->qgroupid,
+ (uintptr_t)glist->group, GFP_ATOMIC);
+ if (ret < 0)
+ goto out;
}
}
--
1.7.11.7
--
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
Wang Shilong
2013-Apr-17 14:18 UTC
Re: [PATCH V2] Btrfs: fix missing check about ulist_add() in qgroup.c
Hello Josef, It really takes me the whole day to tack such strange regression down! In fact, i should test every patch even for a cleanup patch carefully…. Sorry for inconvenience to you. Wang> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com> > > ulist_add() may return -ENOMEM, fix missing check about > return value. > > Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com> > --- > Changelog v1->v2: > ulist_add() may return 1, and this is ok. For this case, > btrfs_qgroup_reserve() should return 0, otherwise, i get > a regression when testing btrfs quota with btrfs-next. > --- > fs/btrfs/qgroup.c | 62 +++++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 44 insertions(+), 18 deletions(-) > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index e089fc1..5d2743d 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1261,7 +1261,10 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans, > > ulist_reinit(tmp); > /* XXX id not needed */ > - ulist_add(tmp, qg->qgroupid, (u64)(uintptr_t)qg, GFP_ATOMIC); > + ret = ulist_add(tmp, qg->qgroupid, > + (u64)(uintptr_t)qg, GFP_ATOMIC); > + if (ret < 0) > + goto unlock; > ULIST_ITER_INIT(&tmp_uiter); > while ((tmp_unode = ulist_next(tmp, &tmp_uiter))) { > struct btrfs_qgroup_list *glist; > @@ -1273,9 +1276,11 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans, > ++qg->refcnt; > > list_for_each_entry(glist, &qg->groups, next_group) { > - ulist_add(tmp, glist->group->qgroupid, > - (u64)(uintptr_t)glist->group, > - GFP_ATOMIC); > + ret = ulist_add(tmp, glist->group->qgroupid, > + (u64)(uintptr_t)glist->group, > + GFP_ATOMIC); > + if (ret < 0) > + goto unlock; > } > } > } > @@ -1284,7 +1289,10 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans, > * step 2: walk from the new root > */ > ulist_reinit(tmp); > - ulist_add(tmp, qgroup->qgroupid, (uintptr_t)qgroup, GFP_ATOMIC); > + ret = ulist_add(tmp, qgroup->qgroupid, > + (uintptr_t)qgroup, GFP_ATOMIC); > + if (ret < 0) > + goto unlock; > ULIST_ITER_INIT(&uiter); > while ((unode = ulist_next(tmp, &uiter))) { > struct btrfs_qgroup *qg; > @@ -1305,8 +1313,10 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans, > qg->tag = seq; > > list_for_each_entry(glist, &qg->groups, next_group) { > - ulist_add(tmp, glist->group->qgroupid, > - (uintptr_t)glist->group, GFP_ATOMIC); > + ret = ulist_add(tmp, glist->group->qgroupid, > + (uintptr_t)glist->group, GFP_ATOMIC); > + if (ret < 0) > + goto unlock; > } > } > > @@ -1324,7 +1334,10 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans, > continue; > > ulist_reinit(tmp); > - ulist_add(tmp, qg->qgroupid, (uintptr_t)qg, GFP_ATOMIC); > + ret = ulist_add(tmp, qg->qgroupid, > + (uintptr_t)qg, GFP_ATOMIC); > + if (ret < 0) > + goto unlock; > ULIST_ITER_INIT(&tmp_uiter); > while ((tmp_unode = ulist_next(tmp, &tmp_uiter))) { > struct btrfs_qgroup_list *glist; > @@ -1340,9 +1353,11 @@ int btrfs_qgroup_account_ref(struct btrfs_trans_handle *trans, > } > > list_for_each_entry(glist, &qg->groups, next_group) { > - ulist_add(tmp, glist->group->qgroupid, > - (uintptr_t)glist->group, > - GFP_ATOMIC); > + ret = ulist_add(tmp, glist->group->qgroupid, > + (uintptr_t)glist->group, > + GFP_ATOMIC); > + if (ret < 0) > + goto unlock; > } > } > } > @@ -1607,7 +1622,10 @@ int btrfs_qgroup_reserve(struct btrfs_root *root, u64 num_bytes) > ret = -ENOMEM; > goto out; > } > - ulist_add(ulist, qgroup->qgroupid, (uintptr_t)qgroup, GFP_ATOMIC); > + ret = ulist_add(ulist, qgroup->qgroupid, > + (uintptr_t)qgroup, GFP_ATOMIC); > + if (ret < 0) > + goto out; > ULIST_ITER_INIT(&uiter); > while ((unode = ulist_next(ulist, &uiter))) { > struct btrfs_qgroup *qg; > @@ -1630,11 +1648,13 @@ int btrfs_qgroup_reserve(struct btrfs_root *root, u64 num_bytes) > } > > list_for_each_entry(glist, &qg->groups, next_group) { > - ulist_add(ulist, glist->group->qgroupid, > - (uintptr_t)glist->group, GFP_ATOMIC); > + ret = ulist_add(ulist, glist->group->qgroupid, > + (uintptr_t)glist->group, GFP_ATOMIC); > + if (ret < 0) > + goto out; > } > } > - > + ret = 0; > /* > * no limits exceeded, now record the reservation into all qgroups > */ > @@ -1663,6 +1683,7 @@ void btrfs_qgroup_free(struct btrfs_root *root, u64 num_bytes) > struct ulist_node *unode; > struct ulist_iterator uiter; > u64 ref_root = root->root_key.objectid; > + int ret = 0; > > if (!is_fstree(ref_root)) > return; > @@ -1685,7 +1706,10 @@ void btrfs_qgroup_free(struct btrfs_root *root, u64 num_bytes) > btrfs_std_error(fs_info, -ENOMEM); > goto out; > } > - ulist_add(ulist, qgroup->qgroupid, (uintptr_t)qgroup, GFP_ATOMIC); > + ret = ulist_add(ulist, qgroup->qgroupid, > + (uintptr_t)qgroup, GFP_ATOMIC); > + if (ret < 0) > + goto out; > ULIST_ITER_INIT(&uiter); > while ((unode = ulist_next(ulist, &uiter))) { > struct btrfs_qgroup *qg; > @@ -1696,8 +1720,10 @@ void btrfs_qgroup_free(struct btrfs_root *root, u64 num_bytes) > qg->reserved -= num_bytes; > > list_for_each_entry(glist, &qg->groups, next_group) { > - ulist_add(ulist, glist->group->qgroupid, > - (uintptr_t)glist->group, GFP_ATOMIC); > + ret = ulist_add(ulist, glist->group->qgroupid, > + (uintptr_t)glist->group, GFP_ATOMIC); > + if (ret < 0) > + goto out; > } > } > > -- > 1.7.11.7 >-- 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