From patchwork Tue Aug 6 14:05:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Qu Wenruo X-Patchwork-Id: 11078991 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2F6FF746 for ; Tue, 6 Aug 2019 14:05:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 20BBD28A1B for ; Tue, 6 Aug 2019 14:05:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1F22428A22; Tue, 6 Aug 2019 14:05:17 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9AD8F28A1B for ; Tue, 6 Aug 2019 14:05:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731749AbfHFOFP (ORCPT ); Tue, 6 Aug 2019 10:05:15 -0400 Received: from mx2.suse.de ([195.135.220.15]:50892 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728836AbfHFOFP (ORCPT ); Tue, 6 Aug 2019 10:05:15 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 53D89AF0D; Tue, 6 Aug 2019 14:05:14 +0000 (UTC) From: Qu Wenruo To: linux-btrfs@vger.kernel.org Cc: Andrei Borzenkov Subject: [PATCH v2] btrfs: qgroup: Try our best to delete qgroup relations Date: Tue, 6 Aug 2019 22:05:07 +0800 Message-Id: <20190806140507.5204-1-wqu@suse.com> X-Mailer: git-send-email 2.22.0 MIME-Version: 1.0 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Signed-off-by: Qu Wenruo --- 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;