Re: [PATCH] btrfs-progs: property: introduce drop_subtree_threshold property

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/8/10 15:47, Sidong Yang 写道:
> This patch introduces new property drop_subtree_threshold. This property
> could be set/get easily by root dir without find sysfs path.
>
> Fixes: https://github.com/kdave/btrfs-progs/issues/795
>
> Issue: #795
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
>   cmds/property.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>
> diff --git a/cmds/property.c b/cmds/property.c
> index a36b5ab2..44b62af6 100644
> --- a/cmds/property.c
> +++ b/cmds/property.c
> @@ -35,6 +35,7 @@
>   #include "common/utils.h"
>   #include "common/help.h"
>   #include "common/filesystem-utils.h"
> +#include "common/sysfs-utils.h"
>   #include "cmds/commands.h"
>   #include "cmds/props.h"
>
> @@ -236,6 +237,45 @@ out:
>
>   	return ret;
>   }
> +static int prop_drop_subtree_threshold(enum prop_object_type type,
> +				       const char *object,
> +				       const char *name,
> +				       const char *value,
> +				       bool force) {
> +	int ret;
> +	int fd;
> +	int sysfs_fd;
> +	char buf[255];
> +
> +        fd = btrfs_open_path(object, value, false);
> +	if (fd < 0)
> +		return -errno;
> +
> +	sysfs_fd = sysfs_open_fsid_file(fd, "qgroups/drop_subtree_threshold");
> +	if (sysfs_fd < 0) {

Since qgroups/ directory is automatically generated/removed according to
the qgroup status, you have to handle -ENOENT case, other than just
erroring out.

At least you can do a warning if qgroup is not enabled.

> +		close(fd);
> +		return -errno;
> +	}
> +
> +	if (value) {
> +		ret = write(sysfs_fd, value, strlen(value));
> +	} else {
> +		ret = read(sysfs_fd, buf, 255);
> +		if (ret > 0) {
> +			buf[ret] = 0;
> +			pr_verbose(LOG_DEFAULT, "drop_subtree_threshold=%s", buf);
> +		}
> +	}
> +	if (ret < 0) {
> +		ret = -errno;
> +	} else {
> +		ret = 0;
> +	}
> +
> +	close(sysfs_fd);
> +	close(fd);
> +	return ret;
> +}
>
>   const struct prop_handler prop_handlers[] = {
>   	{
> @@ -259,6 +299,13 @@ const struct prop_handler prop_handlers[] = {
>   		.types = prop_object_inode,
>   		.handler = prop_compression
>   	},
> +	{
> +		.name = "drop_subtree_threshold",
> +		.desc = "threshold for dropping reference to subtree",

The feature is qgroup specific, thus it's better to mention it.

Furthermore you didn't explain what threshold it is.

I'd go something like:

"subtree level threshold to mark qgroup inconsistent during snapshot
deletion, can reduce qgroup workload of snapshot deletion".

Thanks,
Qu
> +		.read_only = 0,
> +		.types = prop_object_root,
> +		.handler = prop_drop_subtree_threshold
> +	},
>   	{NULL, NULL, 0, 0, NULL}
>   };
>