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

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

[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) {
+		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",
+		.read_only = 0,
+		.types = prop_object_root,
+		.handler = prop_drop_subtree_threshold
+	},
 	{NULL, NULL, 0, 0, NULL}
 };
 
-- 
2.43.0

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

[Qu Wenruo]

在 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}
>   };
>

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

[Sidong Yang]

On Sun, Aug 11, 2024 at 03:25:13PM +0930, Qu Wenruo wrote:

Hi Qu. Thanks for review.
> 
> 
> 在 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.

Okay, I didn't care about it. It should handle -ENOENT and print warning
in next version.
> 
> > +		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 for detailed example. It will be used for next patch. Honestly, I
had no idea about this option. I'll look at the kernel implementation.

Thanks,
Sidong

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