* [PATCH 1/4] btrfs-progs: cmds/qgroup: sync the fs before doing qgroup search
2024-05-09 9:12 [PATCH 0/4] btrfs-progs: cmds/qgroup: enhance stale qgroups handling Qu Wenruo
@ 2024-05-09 9:12 ` Qu Wenruo
2024-05-09 9:12 ` [PATCH 2/4] btrfs-progs: cmds/qgroup: add qgroup_lookup::flags member Qu Wenruo
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2024-05-09 9:12 UTC (permalink / raw)
To: linux-btrfs
Since qgroup numbers are only updated at transaction commit time, it's
better to do the a sync before reading the quota tree, to reduce the
chance of uncommitted qgroup changes.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
cmds/qgroup.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/cmds/qgroup.c b/cmds/qgroup.c
index 6e88db5803a5..09eac0d2b36e 100644
--- a/cmds/qgroup.c
+++ b/cmds/qgroup.c
@@ -2135,6 +2135,7 @@ static const char * const cmd_qgroup_clear_stale_usage[] = {
static int cmd_qgroup_clear_stale(const struct cmd_struct *cmd, int argc, char **argv)
{
+ enum btrfs_util_error err;
int ret = 0;
int fd;
char *path = NULL;
@@ -2151,6 +2152,11 @@ static int cmd_qgroup_clear_stale(const struct cmd_struct *cmd, int argc, char *
if (fd < 0)
return 1;
+ /* Sync the fs so that the qgroup numbers are uptodate. */
+ err = btrfs_util_sync_fd(fd);
+ if (err)
+ warning("sync ioctl failed on '%s': %m", path);
+
ret = qgroups_search_all(fd, &qgroup_lookup);
if (ret == -ENOTTY) {
error("can't list qgroups: quotas not enabled");
--
2.45.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] btrfs-progs: cmds/qgroup: add qgroup_lookup::flags member
2024-05-09 9:12 [PATCH 0/4] btrfs-progs: cmds/qgroup: enhance stale qgroups handling Qu Wenruo
2024-05-09 9:12 ` [PATCH 1/4] btrfs-progs: cmds/qgroup: sync the fs before doing qgroup search Qu Wenruo
@ 2024-05-09 9:12 ` Qu Wenruo
2024-05-09 9:12 ` [PATCH 3/4] btrfs-progs: cmds/qgroup: handle stale qgroup deleteion more accurately Qu Wenruo
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2024-05-09 9:12 UTC (permalink / raw)
To: linux-btrfs
This allows the users to identify if the running qgroup mode and whether
the numebrs are already inconsistent.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
cmds/qgroup.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/cmds/qgroup.c b/cmds/qgroup.c
index 09eac0d2b36e..a023f0948180 100644
--- a/cmds/qgroup.c
+++ b/cmds/qgroup.c
@@ -65,6 +65,8 @@ struct btrfs_qgroup_list {
};
struct qgroup_lookup {
+ /* This matches btrfs_qgroup_status_item::flags. */
+ u64 flags;
struct rb_root root;
};
@@ -1313,6 +1315,7 @@ static int __qgroups_search(int fd, struct btrfs_tree_search_args *args,
case BTRFS_QGROUP_STATUS_KEY:
si = btrfs_tree_search_data(args, off);
flags = btrfs_stack_qgroup_status_flags(si);
+ qgroup_lookup->flags = flags;
print_status_flag_warning(flags);
break;
--
2.45.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] btrfs-progs: cmds/qgroup: handle stale qgroup deleteion more accurately
2024-05-09 9:12 [PATCH 0/4] btrfs-progs: cmds/qgroup: enhance stale qgroups handling Qu Wenruo
2024-05-09 9:12 ` [PATCH 1/4] btrfs-progs: cmds/qgroup: sync the fs before doing qgroup search Qu Wenruo
2024-05-09 9:12 ` [PATCH 2/4] btrfs-progs: cmds/qgroup: add qgroup_lookup::flags member Qu Wenruo
@ 2024-05-09 9:12 ` Qu Wenruo
2024-05-09 9:12 ` [PATCH 4/4] btrfs-progs: cmds/qgroup: add more special status for qgroups Qu Wenruo
2024-05-17 16:15 ` [PATCH 0/4] btrfs-progs: cmds/qgroup: enhance stale qgroups handling David Sterba
4 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2024-05-09 9:12 UTC (permalink / raw)
To: linux-btrfs
The current stale qgroup delete is a mess, it doesn't handle the
following cases at all:
- It doesn't detect stale qgroups correctly
The current check is using the root backref, which means unlinked but
not yet fully dropped subvolumes would mark its corresponding qgroups
stale.
This is incorrect. The real stale check should be based on the root
item, not root backref.
- Squota non-empty but stale qgroups
Such qgroups can not and should not be deleted, as future accounting
still require them.
- Full accounting mode, stale qgroups but not empty
Since qgroup numbers are inconsistent already, it's common to have
such stale qgroups with non-zero numbers.
Now it's dependent on the kernel to determine whether such qgroup can
be deleted.
The patch would address the above problems by:
- Do root_item based detection
So that btrfs_qgroup::stale would properly indicate if there is a
subvolume root item for the qgroup.
- Do not attempt to delete squota stale but non-empty qgroups
- Attempt to delete stale but non-empty qgroups for full accounting mode
And deletion failure would not count as an error.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
cmds/qgroup.c | 119 +++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 103 insertions(+), 16 deletions(-)
diff --git a/cmds/qgroup.c b/cmds/qgroup.c
index a023f0948180..70a306117ebd 100644
--- a/cmds/qgroup.c
+++ b/cmds/qgroup.c
@@ -80,9 +80,24 @@ struct btrfs_qgroup {
struct rb_node all_parent_node;
u64 qgroupid;
- /* NULL for qgroups with level > 0 */
+ /*
+ * NULL for qgroups with level > 0 or the subvolume is *unlinked*.
+ *
+ * Note that, an unlinked subvolume doesn't mean it has been fully
+ * dropped, so callers should not rely on this to determine if
+ * a qgroup is stale.
+ *
+ * This member is only to help locating the path of the corresponding
+ * subvolume.
+ */
const char *path;
+ /*
+ * This is only true if the qgroup is level 0 qgroup, and there is
+ * no subvolume tree for the qgroup at all.
+ */
+ bool stale;
+
/*
* info_item
*/
@@ -229,6 +244,12 @@ static struct {
static btrfs_qgroup_filter_func all_filter_funcs[];
static btrfs_qgroup_comp_func all_comp_funcs[];
+static bool is_qgroup_empty(struct btrfs_qgroup *qg)
+{
+ return !(qg->info.referenced || qg->info.exclusive ||
+ qg->info.referenced_compressed ||
+ qg->info.exclusive_compressed);
+}
static void qgroup_setup_print_column(enum btrfs_qgroup_column_enum column)
{
int i;
@@ -795,7 +816,6 @@ static struct btrfs_qgroup *get_or_add_qgroup(int fd,
uret = btrfs_util_subvolume_path_fd(fd, qgroupid, &path);
if (uret == BTRFS_UTIL_OK)
bq->path = path;
- /* Ignore stale qgroup items */
else if (uret != BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND) {
error("%s", btrfs_util_strerror(uret));
if (uret == BTRFS_UTIL_ERROR_NO_MEMORY)
@@ -803,6 +823,24 @@ static struct btrfs_qgroup *get_or_add_qgroup(int fd,
else
return ERR_PTR(-EIO);
}
+ /*
+ * Do a correct stale detection by searching for the ROOT_ITEM of
+ * the subvolume.
+ *
+ * Passing @subvol as NULL will force the search to only search
+ * for the ROOT_ITEM.
+ */
+ uret = btrfs_util_subvolume_info_fd(fd, qgroupid, NULL);
+ if (uret == BTRFS_UTIL_OK) {
+ bq->stale = false;
+ } else if (uret == BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND) {
+ bq->stale = true;
+ } else {
+ warning("failed to search root item for qgroup %u/%llu, assuming it not stale",
+ btrfs_qgroup_level(qgroupid),
+ btrfs_qgroup_subvolid(qgroupid));
+ bq->stale = false;
+ }
}
ret = qgroup_tree_insert(qgroup_lookup, bq);
@@ -2136,6 +2174,65 @@ static const char * const cmd_qgroup_clear_stale_usage[] = {
NULL
};
+/*
+ * Return >0 if the qgroup should or can not be deleted.
+ * Return 0 if the qgroup is deleted.
+ * Return <0 for critical error.
+ */
+static int delete_one_stale_qgroup(struct qgroup_lookup *lookup,
+ struct btrfs_qgroup *qg, int fd)
+{
+ u16 level = btrfs_qgroup_level(qg->qgroupid);
+ struct btrfs_ioctl_qgroup_create_args args = { .create = false };
+ const bool inconsistent = lookup->flags & BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ const bool squota = lookup->flags & BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE;
+ const bool empty = is_qgroup_empty(qg);
+ bool attempt = false;
+ int ret;
+
+ if (level || !qg->stale)
+ return 1;
+
+ /*
+ * By design, squota can have a subvolume fully dropped but its qgroup numbers
+ * are not zero.
+ * Such qgroup is still needed for future accounting, thus can not be deleted.
+ */
+ if (squota && !empty)
+ return 1;
+
+ /*
+ * We can have inconsistent qgroup numbers, in that case a really stale
+ * qgroup can exist while its numbers are not zero.
+ *
+ * In this case we only attempt to delete the qgroup, depending on the
+ * kernel implementation, we may or may not be able to delete it.
+ */
+ if (inconsistent && !empty)
+ attempt = true;
+
+ if (attempt)
+ pr_verbose(LOG_DEFAULT,
+ "Attempt to delete stale but non-empty qgroup %u/%llu\n",
+ level, btrfs_qgroup_subvolid(qg->qgroupid));
+ else
+ pr_verbose(LOG_DEFAULT, "Delete stale qgroup %u/%llu\n",
+ level, btrfs_qgroup_subvolid(qg->qgroupid));
+ args.qgroupid = qg->qgroupid;
+ ret = ioctl(fd, BTRFS_IOC_QGROUP_CREATE, &args);
+ if (ret < 0) {
+ if (attempt) {
+ warning("not supported to delete non-empty stale qgroup %u/%llu",
+ level, btrfs_qgroup_subvolid(qg->qgroupid));
+ ret = 1;
+ } else {
+ error("failed to delete qgroup %u/%llu",
+ level, btrfs_qgroup_subvolid(qg->qgroupid));
+ }
+ }
+ return ret;
+}
+
static int cmd_qgroup_clear_stale(const struct cmd_struct *cmd, int argc, char **argv)
{
enum btrfs_util_error err;
@@ -2172,22 +2269,12 @@ static int cmd_qgroup_clear_stale(const struct cmd_struct *cmd, int argc, char *
node = rb_first(&qgroup_lookup.root);
while (node) {
- u64 level;
- struct btrfs_ioctl_qgroup_create_args args = { .create = false };
+ int delete_ret;
entry = rb_entry(node, struct btrfs_qgroup, rb_node);
- level = btrfs_qgroup_level(entry->qgroupid);
- if (!entry->path && level == 0) {
- pr_verbose(LOG_DEFAULT, "Delete stale qgroup %llu/%llu\n",
- level, btrfs_qgroup_subvolid(entry->qgroupid));
- args.qgroupid = entry->qgroupid;
- ret = ioctl(fd, BTRFS_IOC_QGROUP_CREATE, &args);
- if (ret < 0) {
- error("cannot delete qgroup %llu/%llu: %m",
- level,
- btrfs_qgroup_subvolid(entry->qgroupid));
- }
- }
+ delete_ret = delete_one_stale_qgroup(&qgroup_lookup, entry, fd);
+ if (delete_ret < 0 && !ret)
+ ret = delete_ret;
node = rb_next(node);
}
--
2.45.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] btrfs-progs: cmds/qgroup: add more special status for qgroups
2024-05-09 9:12 [PATCH 0/4] btrfs-progs: cmds/qgroup: enhance stale qgroups handling Qu Wenruo
` (2 preceding siblings ...)
2024-05-09 9:12 ` [PATCH 3/4] btrfs-progs: cmds/qgroup: handle stale qgroup deleteion more accurately Qu Wenruo
@ 2024-05-09 9:12 ` Qu Wenruo
2024-05-17 16:15 ` [PATCH 0/4] btrfs-progs: cmds/qgroup: enhance stale qgroups handling David Sterba
4 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2024-05-09 9:12 UTC (permalink / raw)
To: linux-btrfs
Currently `btrfs qgroup show` command shows any 0 level qgroup withou a
root backref as `<stale>`, which is not correct.
There are several more cases:
- Under deletion
The subvolume is not yet full dropped, but unlinked.
In that case we would not have a root backref item, but the qgroup is
not stale.
- Squota space holder
This is for squota mode, that a fully dropped subvolume still have
extents accounting on the already-gone subvolume.
In this case it's not stale either, and future accounting relies on
it.
This patch would add above special cases, and add an extra `SPECIAL
PATHS` section to explain all the cases, including `<stale>`.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Documentation/btrfs-qgroup.rst | 24 ++++++++++++++++++++++++
cmds/qgroup.c | 32 ++++++++++++++++++++++++++------
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/Documentation/btrfs-qgroup.rst b/Documentation/btrfs-qgroup.rst
index 343626767eb5..0e47c336619f 100644
--- a/Documentation/btrfs-qgroup.rst
+++ b/Documentation/btrfs-qgroup.rst
@@ -153,6 +153,30 @@ show [options] <path>
To retrieve information after updating the state of qgroups,
force sync of the filesystem identified by *path* before getting information.
+SPECIAL PATHS
+-------------
+For `btrfs qgroup show` subcommand, the ``path`` column may has some special
+strings:
+
+`<toplevel>`
+ The toplevel subvolume
+
+`<under deletion>`
+ The subvolume is unlinked, but not yet fully deleted.
+
+`<squota space holder>`
+ For simple quota mode only.
+ By its design, a fully deleted subvolume may still have accounting on
+ it, so even the subvolume is gone, the numbers are still here for future
+ accounting.
+
+`<stale>`
+ The qgroup has no corresponding subvolume anymore, and the qgroup
+ can be cleaned up under most cases.
+ The only exception is that, if the qgroup numbers are inconsistent and
+ the qgroup numbers are not all zeros, some older kernels may refuse to
+ delete such qgroups until a full rescan.
+
QUOTA RESCAN
------------
diff --git a/cmds/qgroup.c b/cmds/qgroup.c
index 70a306117ebd..928c432ca432 100644
--- a/cmds/qgroup.c
+++ b/cmds/qgroup.c
@@ -71,6 +71,8 @@ struct qgroup_lookup {
};
struct btrfs_qgroup {
+ struct qgroup_lookup *lookup;
+
struct rb_node rb_node;
struct rb_node sort_node;
/*
@@ -321,6 +323,26 @@ static void print_qgroup_column_add_blank(enum btrfs_qgroup_column_enum column,
printf(" ");
}
+static const char *get_qgroup_path(struct btrfs_qgroup *qgroup)
+{
+ if (qgroup->path)
+ return qgroup->path;
+
+ /* No path but not stale either, the qgroup is being deleted. */
+ if (!qgroup->stale)
+ return "<under deletion>";
+ /*
+ * Squota mode stale qgroup, but not empty.
+ * This is fully deleted but still necessary.
+ */
+ if (qgroup->stale &&
+ (qgroup->lookup->flags & BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE) &&
+ !is_qgroup_empty(qgroup))
+ return "<squota space holder>";
+
+ return "<stale>";
+}
+
static void print_path_column(struct btrfs_qgroup *qgroup)
{
struct btrfs_qgroup_list *list = NULL;
@@ -338,11 +360,8 @@ static void print_path_column(struct btrfs_qgroup *qgroup)
if (count)
pr_verbose(LOG_DEFAULT, " ");
if (level == 0) {
- const char *path = member->path;
-
- if (!path)
- path = "<stale>";
- pr_verbose(LOG_DEFAULT, "%s", path);
+ pr_verbose(LOG_DEFAULT, "%s",
+ get_qgroup_path(qgroup));
} else {
pr_verbose(LOG_DEFAULT, "%llu/%llu", level, sid);
}
@@ -353,7 +372,7 @@ static void print_path_column(struct btrfs_qgroup *qgroup)
} else if (qgroup->path) {
pr_verbose(LOG_DEFAULT, "%s%s", (*qgroup->path ? "" : "<toplevel>"), qgroup->path);
} else {
- pr_verbose(LOG_DEFAULT, "<stale>");
+ pr_verbose(LOG_DEFAULT, "%s", get_qgroup_path(qgroup));
}
}
@@ -805,6 +824,7 @@ static struct btrfs_qgroup *get_or_add_qgroup(int fd,
return ERR_PTR(-ENOMEM);
}
+ bq->lookup = qgroup_lookup;
bq->qgroupid = qgroupid;
INIT_LIST_HEAD(&bq->qgroups);
INIT_LIST_HEAD(&bq->members);
--
2.45.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] btrfs-progs: cmds/qgroup: enhance stale qgroups handling
2024-05-09 9:12 [PATCH 0/4] btrfs-progs: cmds/qgroup: enhance stale qgroups handling Qu Wenruo
` (3 preceding siblings ...)
2024-05-09 9:12 ` [PATCH 4/4] btrfs-progs: cmds/qgroup: add more special status for qgroups Qu Wenruo
@ 2024-05-17 16:15 ` David Sterba
4 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2024-05-17 16:15 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Thu, May 09, 2024 at 06:42:29PM +0930, Qu Wenruo wrote:
> Currently the `<stale>` label is give incorrectly for a lot of qgroups
> that can not and should not be deleted.
>
> The patchset would address the problem for `btrfs qgroup show` and
> `btrfs qgroup clear-stale` commands, so that:
>
> - `clear-stale` would not try to delete incorrect qgroups
> Thus it should not return false errors.
>
> - `show` would show extra types of qgroups
> Including `<under deletion>` and `<squota space holder>`
>
> Qu Wenruo (4):
> btrfs-progs: cmds/qgroup: sync the fs before doing qgroup search
> btrfs-progs: cmds/qgroup: add qgroup_lookup::flags member
> btrfs-progs: cmds/qgroup: handle stale qgroup deleteion more
> accurately
> btrfs-progs: cmds/qgroup: add more special status for qgroups
Added to devel, thanks. Please use the subjects that match the style
used in progs, ie. no 'cmds/' prefix.
^ permalink raw reply [flat|nested] 6+ messages in thread