| Message ID | acdae2db72282b45ff6d2e11df788a9b146ffe92.1713508989.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: qgroup: stale qgroups related impromvents | expand |
在 2024/4/19 16:20, Qu Wenruo 写道: > [BUG] > Currently if one is utilizing "qgroups/drop_subtree_threshold" sysfs, > and a snapshot with level higher than that value is dropped, btrfs will > not be able to delete the qgroup until next qgroup rescan: > > uuid=ffffffff-eeee-dddd-cccc-000000000000 > > wipefs -fa $dev > mkfs.btrfs -f $dev -O quota -s 4k -n 4k -U $uuid > mount $dev $mnt > > btrfs subvolume create $mnt/subv1/ > for (( i = 0; i < 1024; i++ )); do > xfs_io -f -c "pwrite 0 2k" $mnt/subv1/file_$i > /dev/null > done > sync > btrfs subv snapshot $mnt/subv1 $mnt/snapshot > btrfs quota enable $mnt > btrfs quota rescan -w $mnt > sync > echo 1 > /sys/fs/btrfs/$uuid/qgroups/drop_subtree_threshold > btrfs subvolume delete $mnt/snapshot > btrfs subv sync $mnt > btrfs qgroup show -prce --sync $mnt > btrfs qgroup destroy 0/256 $mnt My bad, the target qgroup show be 0/257. Would update the commit message. Thanks, Qu > umount $mnt > > The final qgroup removal would fail with the following error: > > ERROR: unable to destroy quota group: Device or resource busy > > [CAUSE] > The above script would generate a subvolume of level 2, then snapshot > it, enable qgroup, set the drop_subtree_threshold, then drop the > snapshot. > > Since the subvolume drop would meet the threshold, qgroup would be > marked inconsistent and skip accounting to avoid hanging the system at > transaction commit. > > But currently we do not allow a qgroup with any rfer/excl numbers to be > dropped, and this is not really compatible with the new > drop_subtree_threshold behavior. > > [FIX] > Instead of a strong requirement for zero rfer/excl numbers, just check > if there is any child for higher level qgroup, and for subvolume qgroups > check if there is a corresponding subvolume for it. > > For rsv values, just do a WARN_ON() in case. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/qgroup.c | 48 ++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 41 insertions(+), 7 deletions(-) > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index 9a9f84c4d3b8..e6fcce4372a4 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1736,13 +1736,38 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid) > return ret; > } > > -static bool qgroup_has_usage(struct btrfs_qgroup *qgroup) > +static bool can_delete_qgroup(struct btrfs_fs_info *fs_info, > + struct btrfs_qgroup *qgroup) > { > - return (qgroup->rfer > 0 || qgroup->rfer_cmpr > 0 || > - qgroup->excl > 0 || qgroup->excl_cmpr > 0 || > - qgroup->rsv.values[BTRFS_QGROUP_RSV_DATA] > 0 || > - qgroup->rsv.values[BTRFS_QGROUP_RSV_META_PREALLOC] > 0 || > - qgroup->rsv.values[BTRFS_QGROUP_RSV_META_PERTRANS] > 0); > + struct btrfs_key key; > + struct btrfs_path *path; > + int ret; > + > + /* For higher level qgroup, we can only delete it if it has no child. */ > + if (btrfs_qgroup_level(qgroup->qgroupid)) { > + if (!list_empty(&qgroup->members)) > + return false; > + return true; > + } > + > + /* > + * For level-0 qgroups, we can only delete it if it has no subvolume > + * for it. > + * This means even a subvolume is unlinked but not yet fully dropped, > + * we can not delete the qgroup. > + */ > + key.objectid = qgroup->qgroupid; > + key.type = BTRFS_ROOT_ITEM_KEY; > + key.offset = -1ULL; > + path = btrfs_alloc_path(); > + if (!path) > + return false; > + > + ret = btrfs_find_root(fs_info->tree_root, &key, path, NULL, NULL); > + btrfs_free_path(path); > + if (ret > 0) > + return true; > + return false; > } > > int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid) > @@ -1764,7 +1789,7 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid) > goto out; > } > > - if (is_fstree(qgroupid) && qgroup_has_usage(qgroup)) { > + if (!can_delete_qgroup(fs_info, qgroup)) { > ret = -EBUSY; > goto out; > } > @@ -1775,6 +1800,15 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid) > goto out; > } > > + /* > + * Warn on reserved space. The subvolume should has no child nor > + * corresponding subvolume. > + * Thus its reserved space should all be zero. > + */ > + WARN_ON(qgroup->rsv.values[BTRFS_QGROUP_RSV_DATA] || > + qgroup->rsv.values[BTRFS_QGROUP_RSV_META_PREALLOC] || > + qgroup->rsv.values[BTRFS_QGROUP_RSV_META_PERTRANS]); > + > ret = del_qgroup_item(trans, qgroupid); > if (ret && ret != -ENOENT) > goto out;
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index 9a9f84c4d3b8..e6fcce4372a4 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1736,13 +1736,38 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid) return ret; } -static bool qgroup_has_usage(struct btrfs_qgroup *qgroup) +static bool can_delete_qgroup(struct btrfs_fs_info *fs_info, + struct btrfs_qgroup *qgroup) { - return (qgroup->rfer > 0 || qgroup->rfer_cmpr > 0 || - qgroup->excl > 0 || qgroup->excl_cmpr > 0 || - qgroup->rsv.values[BTRFS_QGROUP_RSV_DATA] > 0 || - qgroup->rsv.values[BTRFS_QGROUP_RSV_META_PREALLOC] > 0 || - qgroup->rsv.values[BTRFS_QGROUP_RSV_META_PERTRANS] > 0); + struct btrfs_key key; + struct btrfs_path *path; + int ret; + + /* For higher level qgroup, we can only delete it if it has no child. */ + if (btrfs_qgroup_level(qgroup->qgroupid)) { + if (!list_empty(&qgroup->members)) + return false; + return true; + } + + /* + * For level-0 qgroups, we can only delete it if it has no subvolume + * for it. + * This means even a subvolume is unlinked but not yet fully dropped, + * we can not delete the qgroup. + */ + key.objectid = qgroup->qgroupid; + key.type = BTRFS_ROOT_ITEM_KEY; + key.offset = -1ULL; + path = btrfs_alloc_path(); + if (!path) + return false; + + ret = btrfs_find_root(fs_info->tree_root, &key, path, NULL, NULL); + btrfs_free_path(path); + if (ret > 0) + return true; + return false; } int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid) @@ -1764,7 +1789,7 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid) goto out; } - if (is_fstree(qgroupid) && qgroup_has_usage(qgroup)) { + if (!can_delete_qgroup(fs_info, qgroup)) { ret = -EBUSY; goto out; } @@ -1775,6 +1800,15 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid) goto out; } + /* + * Warn on reserved space. The subvolume should has no child nor + * corresponding subvolume. + * Thus its reserved space should all be zero. + */ + WARN_ON(qgroup->rsv.values[BTRFS_QGROUP_RSV_DATA] || + qgroup->rsv.values[BTRFS_QGROUP_RSV_META_PREALLOC] || + qgroup->rsv.values[BTRFS_QGROUP_RSV_META_PERTRANS]); + ret = del_qgroup_item(trans, qgroupid); if (ret && ret != -ENOENT) goto out;
[BUG] Currently if one is utilizing "qgroups/drop_subtree_threshold" sysfs, and a snapshot with level higher than that value is dropped, btrfs will not be able to delete the qgroup until next qgroup rescan: uuid=ffffffff-eeee-dddd-cccc-000000000000 wipefs -fa $dev mkfs.btrfs -f $dev -O quota -s 4k -n 4k -U $uuid mount $dev $mnt btrfs subvolume create $mnt/subv1/ for (( i = 0; i < 1024; i++ )); do xfs_io -f -c "pwrite 0 2k" $mnt/subv1/file_$i > /dev/null done sync btrfs subv snapshot $mnt/subv1 $mnt/snapshot btrfs quota enable $mnt btrfs quota rescan -w $mnt sync echo 1 > /sys/fs/btrfs/$uuid/qgroups/drop_subtree_threshold btrfs subvolume delete $mnt/snapshot btrfs subv sync $mnt btrfs qgroup show -prce --sync $mnt btrfs qgroup destroy 0/256 $mnt umount $mnt The final qgroup removal would fail with the following error: ERROR: unable to destroy quota group: Device or resource busy [CAUSE] The above script would generate a subvolume of level 2, then snapshot it, enable qgroup, set the drop_subtree_threshold, then drop the snapshot. Since the subvolume drop would meet the threshold, qgroup would be marked inconsistent and skip accounting to avoid hanging the system at transaction commit. But currently we do not allow a qgroup with any rfer/excl numbers to be dropped, and this is not really compatible with the new drop_subtree_threshold behavior. [FIX] Instead of a strong requirement for zero rfer/excl numbers, just check if there is any child for higher level qgroup, and for subvolume qgroups check if there is a corresponding subvolume for it. For rsv values, just do a WARN_ON() in case. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/qgroup.c | 48 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 7 deletions(-)