[PATCH 0/2] btrfs-progs: speed up "btrfs subvolume sync"

[PATCH 0/2] btrfs-progs: speed up "btrfs subvolume sync"

[Qu Wenruo]

Unlike the name, "btrfs subvolume sync" doesn't sync the fs, but only
wait for unlinked subvolumes to be fully deleted.

But that wait completely depends on when the cleaner is woken up, and by
default it takes 30s.

That's why a lot of fstests cases are remount with "commit=1" mount
option to speed "btrfs subvolume sync" up.

To speed things up, and reduce the confusion on the command name, do a
proper BTRFS_IOC_SYNC before waiting for unlinked subvolumes.
That's done in the 2nd patch.

The first patch is minor fix to an uninitilized access, exposed by LLM
during the review of the 2nd patch.

Qu Wenruo (2):
  btrfs-progs: subvolume: fix an uninitialized access
  btrfs-progs: trigger a sync before waiting for subvolumes to be
    dropped

 Documentation/btrfs-subvolume.rst | 7 +++++++
 cmds/subvolume.c                  | 6 +++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

--
2.56.0

[PATCH 1/2] btrfs-progs: subvolume: fix an uninitialized access

[Qu Wenruo]

If the fstatvfs() call failed inside wait_for_subvolume_cleaning(), for
the first failure it output a message, and set @statvfs_warned to true.

Then if fstatvfs() failed again by some reason, since @statvfs_warned is
true, the same old warning won't be triggered, so
"(st.f_flag & ST_RDONLY)" is checked instead.

But since the fstatvfs() failed already, the @st is uninitialized, which
can trigger an incorrect warning and return before waiting for
subvolumes.

This is really a minor problem, exposed by LLM during review of patches
touching the same wait_for_subvolume_cleaning() function.

Fix it by only checking @st.f_flag if the fstatvfs() succeeded.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds/subvolume.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index f0da28582931..1c53686596df 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -112,7 +112,7 @@ static int wait_for_subvolume_cleaning(int fd, size_t count, uint64_t *ids,
 		if (ret < 0 && !statvfs_warned) {
 			statvfs_warned = true;
 			warning("cannot check read-only status of the filesystem: %m");
-		} else if (st.f_flag & ST_RDONLY) {
+		} else if (ret == 0 && st.f_flag & ST_RDONLY) {
 			warning("filesystem is now read-only");
 			return 1;
 		}
-- 
2.56.0

[PATCH 2/2] btrfs-progs: trigger a sync before waiting for subvolumes to be dropped

[Qu Wenruo]

The command "btrfs subvolume sync" is utilized to wait for deleted
subvolume(s) to be completely dropped.

The subvolume dropping is done by the cleaner kthread, which is normally
woken up by the transaction kthread.

That cleaner auto-wake up will take up to 30s (the default commit
interval), and that's why a lot of fstests are using "commit=1" to speed
up such subvolume dropping.

Considering the default wait time is pretty long (30s), and the command
itself contains the "sync", it's better to call BTRFS_IOC_SYNC to sync
the fs and wake up the cleaner, to greately reduce the wait time.

Also update the docs to reflect the new change.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Documentation/btrfs-subvolume.rst | 7 +++++++
 cmds/subvolume.c                  | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/btrfs-subvolume.rst b/Documentation/btrfs-subvolume.rst
index 77fe5d005c35..478caecde69c 100644
--- a/Documentation/btrfs-subvolume.rst
+++ b/Documentation/btrfs-subvolume.rst
@@ -276,6 +276,13 @@ sync <path> [subvolid...]
 
         If the filesystem status changes to read-only then the waiting is interrupted.
 
+	.. note ::
+		Before btrfs-progs v7.4, the wait can be very long, as the subvolume
+		cleanup is triggered by cleaner, which is only woken up every 30s by
+		default.
+		Since btrfs-progs v7.4, this command will manually sync the fs to
+		wake up the cleaner, to greatly reduce the wait time.
+
         ``Options``
 
         -s <N>
diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 1c53686596df..9db7e9ba0c23 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -82,6 +82,10 @@ static int wait_for_subvolume_cleaning(int fd, size_t count, uint64_t *ids,
 	size_t done = 0;
 	bool statvfs_warned = false;
 
+	pr_default("Syncing to wake up the cleaner\n");
+	err = btrfs_util_fs_sync_fd(fd);
+	if (err != BTRFS_UTIL_OK)
+		warning("Sync failed: %m, wait for cleaner auto wakeup");
 	pr_default("Waiting for %zu subvolume%s\n", count,
 			(count > 1 ? "s" : ""));
 	while (1) {
-- 
2.56.0