Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] btrfs: qgroup: Try our best to delete qgroup relations
@ 2019-08-06 14:05 Qu Wenruo
  2019-08-07 17:12 ` David Sterba
  0 siblings, 1 reply; 2+ messages in thread
From: Qu Wenruo @ 2019-08-06 14:05 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Andrei Borzenkov

When we try to delete qgroups, we're pretty cautious, we make sure both
qgroups exist and there is a relationship between them, then try to
delete the relation.

This behavior is OK, but the problem is we need to two relation items,
and if we failed the first item deletion, we error out, leaving the
other relation item in qgroup tree.

Sometimes the error from del_qgroup_relation_item() could just be
-ENOENT, thus we can ignore that error and continue without any problem.

Further more, such cautious behavior makes qgroup relation deletion
impossible for orphan relation items.

This patch will enhance __del_qgroup_relation():
- If both qgroups and their relation items exist
  Go the regular deletion routine and update their accounting if needed.

- If any qgroup or relation item doesn't exist
  Then we still try to delete the orphan items anyway, but don't trigger
  the accounting update.

By this, we try our best to remove relation items, and can handle orphan
relation items properly, while still keep the existing behavior for good
qgroup tree.

Reported-by: Andrei Borzenkov <arvidjaar@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v2:
- Fix a condition for checking either @parent or @member is not found
---
 fs/btrfs/qgroup.c | 46 +++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 3e6ffbbd8b0a..d6500ed42e99 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1312,8 +1312,9 @@ static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
 	struct btrfs_qgroup *member;
 	struct btrfs_qgroup_list *list;
 	struct ulist *tmp;
+	bool found = false;
 	int ret = 0;
-	int err;
+	int ret2;
 
 	tmp = ulist_alloc(GFP_KERNEL);
 	if (!tmp)
@@ -1327,28 +1328,39 @@ static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
 
 	member = find_qgroup_rb(fs_info, src);
 	parent = find_qgroup_rb(fs_info, dst);
-	if (!member || !parent) {
-		ret = -EINVAL;
-		goto out;
-	}
+	/*
+	 * The parent/member pair doesn't exist, then try to delete the dead
+	 * relation items only.
+	 */
+	if (!member || !parent)
+		goto delete_item;
 
 	/* check if such qgroup relation exist firstly */
 	list_for_each_entry(list, &member->groups, next_group) {
-		if (list->group == parent)
-			goto exist;
+		if (list->group == parent) {
+			found = true;
+			break;
+		}
 	}
-	ret = -ENOENT;
-	goto out;
-exist:
+
+delete_item:
 	ret = del_qgroup_relation_item(trans, src, dst);
-	err = del_qgroup_relation_item(trans, dst, src);
-	if (err && !ret)
-		ret = err;
+	if (ret < 0 && ret != -ENOENT)
+		goto out;
+	ret2 = del_qgroup_relation_item(trans, dst, src);
+	if (ret2 < 0 && ret2 != -ENOENT)
+		goto out;
 
-	spin_lock(&fs_info->qgroup_lock);
-	del_relation_rb(fs_info, src, dst);
-	ret = quick_update_accounting(fs_info, tmp, src, dst, -1);
-	spin_unlock(&fs_info->qgroup_lock);
+	/* At least one deletion succeeded, return 0 */
+	if (!ret || !ret2)
+		ret = 0;
+
+	if (found) {
+		spin_lock(&fs_info->qgroup_lock);
+		del_relation_rb(fs_info, src, dst);
+		ret = quick_update_accounting(fs_info, tmp, src, dst, -1);
+		spin_unlock(&fs_info->qgroup_lock);
+	}
 out:
 	ulist_free(tmp);
 	return ret;
-- 
2.22.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] btrfs: qgroup: Try our best to delete qgroup relations
  2019-08-06 14:05 [PATCH v2] btrfs: qgroup: Try our best to delete qgroup relations Qu Wenruo
@ 2019-08-07 17:12 ` David Sterba
  0 siblings, 0 replies; 2+ messages in thread
From: David Sterba @ 2019-08-07 17:12 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, Andrei Borzenkov

On Tue, Aug 06, 2019 at 10:05:07PM +0800, Qu Wenruo wrote:
> When we try to delete qgroups, we're pretty cautious, we make sure both
> qgroups exist and there is a relationship between them, then try to
> delete the relation.
> 
> This behavior is OK, but the problem is we need to two relation items,
> and if we failed the first item deletion, we error out, leaving the
> other relation item in qgroup tree.
> 
> Sometimes the error from del_qgroup_relation_item() could just be
> -ENOENT, thus we can ignore that error and continue without any problem.
> 
> Further more, such cautious behavior makes qgroup relation deletion
> impossible for orphan relation items.
> 
> This patch will enhance __del_qgroup_relation():
> - If both qgroups and their relation items exist
>   Go the regular deletion routine and update their accounting if needed.
> 
> - If any qgroup or relation item doesn't exist
>   Then we still try to delete the orphan items anyway, but don't trigger
>   the accounting update.
> 
> By this, we try our best to remove relation items, and can handle orphan
> relation items properly, while still keep the existing behavior for good
> qgroup tree.
> 
> Reported-by: Andrei Borzenkov <arvidjaar@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> changelog:
> v2:
> - Fix a condition for checking either @parent or @member is not found

Patch replaced in misc-next, thanks.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06 14:05 [PATCH v2] btrfs: qgroup: Try our best to delete qgroup relations Qu Wenruo
2019-08-07 17:12 ` David Sterba

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox