* [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property @ 2024-08-11 12:24 Sidong Yang 2024-08-11 22:46 ` Qu Wenruo 2024-08-11 23:17 ` Roman Mamedov 0 siblings, 2 replies; 5+ messages in thread From: Sidong Yang @ 2024-08-11 12:24 UTC (permalink / raw) To: linux-btrfs, Qu Wenruo; +Cc: 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> --- v2: error msg for -ENOENT, fix desc for prop --- cmds/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/cmds/property.c b/cmds/property.c index a36b5ab2..88344001 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,50 @@ 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) { + if (sysfs_fd == -ENOENT) { + error("failed to access qgroups/drop_subtree_threshold for %s," + " quota should be enabled for this property: %m", + object); + } + 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 +304,14 @@ const struct prop_handler prop_handlers[] = { .types = prop_object_inode, .handler = prop_compression }, + { + .name = "drop_subtree_threshold", + .desc = "subtree level threshold to mark qgroup inconsistent during" + "snapshot deletion, can reduce qgroup workload of snapshot deletion", + .read_only = 0, + .types = prop_object_root, + .handler = prop_drop_subtree_threshold + }, {NULL, NULL, 0, 0, NULL} }; -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property 2024-08-11 12:24 [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property Sidong Yang @ 2024-08-11 22:46 ` Qu Wenruo 2024-08-12 0:52 ` Sidong Yang 2024-08-11 23:17 ` Roman Mamedov 1 sibling, 1 reply; 5+ messages in thread From: Qu Wenruo @ 2024-08-11 22:46 UTC (permalink / raw) To: Sidong Yang, linux-btrfs, Qu Wenruo 在 2024/8/11 21:54, 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> > --- > v2: error msg for -ENOENT, fix desc for prop > --- > cmds/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/cmds/property.c b/cmds/property.c > index a36b5ab2..88344001 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,50 @@ 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) { > + if (sysfs_fd == -ENOENT) { > + error("failed to access qgroups/drop_subtree_threshold for %s," > + " quota should be enabled for this property: %m", > + object); I do not think this needs to be error(), warning() should be enough. > + } > + close(fd); > + return -errno; Furthermore for ENOENT case I do not even think we should return error. The point here is, if qgroup is not running (or running in simple quota) mode, there is no need to set threshold at all, thus no need to return any error. > + } > + > + 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 +304,14 @@ const struct prop_handler prop_handlers[] = { > .types = prop_object_inode, > .handler = prop_compression > }, > + { > + .name = "drop_subtree_threshold", > + .desc = "subtree level threshold to mark qgroup inconsistent during" > + "snapshot deletion, can reduce qgroup workload of snapshot deletion", Please do not break the string, even it's very long. Thanks, Qu > + .read_only = 0, > + .types = prop_object_root, > + .handler = prop_drop_subtree_threshold > + }, > {NULL, NULL, 0, 0, NULL} > }; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property 2024-08-11 22:46 ` Qu Wenruo @ 2024-08-12 0:52 ` Sidong Yang 0 siblings, 0 replies; 5+ messages in thread From: Sidong Yang @ 2024-08-12 0:52 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs, Qu Wenruo On Mon, Aug 12, 2024 at 08:16:13AM +0930, Qu Wenruo wrote: > > > 在 2024/8/11 21:54, 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> > > --- > > v2: error msg for -ENOENT, fix desc for prop > > --- > > cmds/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 53 insertions(+) > > > > diff --git a/cmds/property.c b/cmds/property.c > > index a36b5ab2..88344001 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,50 @@ 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) { > > + if (sysfs_fd == -ENOENT) { > > + error("failed to access qgroups/drop_subtree_threshold for %s," > > + " quota should be enabled for this property: %m", > > + object); > > I do not think this needs to be error(), warning() should be enough. > > > + } > > + close(fd); > > + return -errno; > > Furthermore for ENOENT case I do not even think we should return error. > > The point here is, if qgroup is not running (or running in simple quota) > mode, there is no need to set threshold at all, thus no need to return any > error. > Okay, I understood. It's better to return 0 if qgroup is not running in next patch version. > > + } > > + > > + 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 +304,14 @@ const struct prop_handler prop_handlers[] = { > > .types = prop_object_inode, > > .handler = prop_compression > > }, > > + { > > + .name = "drop_subtree_threshold", > > + .desc = "subtree level threshold to mark qgroup inconsistent during" > > + "snapshot deletion, can reduce qgroup workload of snapshot deletion", > > Please do not break the string, even it's very long. Thanks, I was worried about this. Thanks, Sidong > > Thanks, > Qu > > > + .read_only = 0, > > + .types = prop_object_root, > > + .handler = prop_drop_subtree_threshold > > + }, > > {NULL, NULL, 0, 0, NULL} > > }; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property 2024-08-11 12:24 [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property Sidong Yang 2024-08-11 22:46 ` Qu Wenruo @ 2024-08-11 23:17 ` Roman Mamedov 2024-08-12 0:54 ` Sidong Yang 1 sibling, 1 reply; 5+ messages in thread From: Roman Mamedov @ 2024-08-11 23:17 UTC (permalink / raw) To: Sidong Yang; +Cc: linux-btrfs, Qu Wenruo On Sun, 11 Aug 2024 12:24:15 +0000 Sidong Yang <realwakka@gmail.com> wrote: > 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> > --- > v2: error msg for -ENOENT, fix desc for prop > --- > cmds/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/cmds/property.c b/cmds/property.c > index a36b5ab2..88344001 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,50 @@ 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); This line is indented with spaces instead of TABs like the rest and as it should. I did not check if this is the only occurrence. Checkpatch should catch this: https://docs.kernel.org/dev-tools/checkpatch.html > + if (fd < 0) > + return -errno; > + > + sysfs_fd = sysfs_open_fsid_file(fd, "qgroups/drop_subtree_threshold"); > + if (sysfs_fd < 0) { > + if (sysfs_fd == -ENOENT) { > + error("failed to access qgroups/drop_subtree_threshold for %s," > + " quota should be enabled for this property: %m", > + object); > + } > + 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 +304,14 @@ const struct prop_handler prop_handlers[] = { > .types = prop_object_inode, > .handler = prop_compression > }, > + { > + .name = "drop_subtree_threshold", > + .desc = "subtree level threshold to mark qgroup inconsistent during" > + "snapshot deletion, can reduce qgroup workload of snapshot deletion", > + .read_only = 0, > + .types = prop_object_root, > + .handler = prop_drop_subtree_threshold > + }, > {NULL, NULL, 0, 0, NULL} > }; > -- With respect, Roman ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property 2024-08-11 23:17 ` Roman Mamedov @ 2024-08-12 0:54 ` Sidong Yang 0 siblings, 0 replies; 5+ messages in thread From: Sidong Yang @ 2024-08-12 0:54 UTC (permalink / raw) To: Roman Mamedov; +Cc: linux-btrfs, Qu Wenruo On Mon, Aug 12, 2024 at 04:17:20AM +0500, Roman Mamedov wrote: Hi! Roman. Thanks for review. > On Sun, 11 Aug 2024 12:24:15 +0000 > Sidong Yang <realwakka@gmail.com> wrote: > > > 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> > > --- > > v2: error msg for -ENOENT, fix desc for prop > > --- > > cmds/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 53 insertions(+) > > > > diff --git a/cmds/property.c b/cmds/property.c > > index a36b5ab2..88344001 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,50 @@ 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); > > This line is indented with spaces instead of TABs like the rest and as it > should. I did not check if this is the only occurrence. > > Checkpatch should catch this: https://docs.kernel.org/dev-tools/checkpatch.html Oops, I didn't notice about this. thanks for tip! > > > + if (fd < 0) > > + return -errno; > > + > > + sysfs_fd = sysfs_open_fsid_file(fd, "qgroups/drop_subtree_threshold"); > > + if (sysfs_fd < 0) { > > + if (sysfs_fd == -ENOENT) { > > + error("failed to access qgroups/drop_subtree_threshold for %s," > > + " quota should be enabled for this property: %m", > > + object); > > + } > > + 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 +304,14 @@ const struct prop_handler prop_handlers[] = { > > .types = prop_object_inode, > > .handler = prop_compression > > }, > > + { > > + .name = "drop_subtree_threshold", > > + .desc = "subtree level threshold to mark qgroup inconsistent during" > > + "snapshot deletion, can reduce qgroup workload of snapshot deletion", > > + .read_only = 0, > > + .types = prop_object_root, > > + .handler = prop_drop_subtree_threshold > > + }, > > {NULL, NULL, 0, 0, NULL} > > }; > > > > > -- > With respect, > Roman Thanks, Sidong ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-08-12 0:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-11 12:24 [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property Sidong Yang 2024-08-11 22:46 ` Qu Wenruo 2024-08-12 0:52 ` Sidong Yang 2024-08-11 23:17 ` Roman Mamedov 2024-08-12 0:54 ` Sidong Yang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.