[PATCH v2] btrfs-progs: convert: follow the default free space tree setting

[PATCH v2] btrfs-progs: convert: follow the default free space tree setting

[Qu Wenruo]

[BUG]
We got some test failures related to btrfs-convert with subpage, e.g.
btrfs/012, the failure would cause the following dmesg:

 BTRFS warning (device nvme0n1p7): v1 space cache is not supported for page size 16384 with sectorsize 4096
 BTRFS error (device nvme0n1p7): open_ctree failed

[CAUSE]
v1 space cache has tons of hardcoded PAGE_SIZE usage, and considering v2
space cache is going to replace it (which is already the new default
since v5.15 btrfs-progs), thus for btrfs subpage support, we just simply
reject the v1 space cache, and utilize v2 space cache when possible.

But there is special catch in btrfs-convert, although we're specifying
v2 space cache as the new default for btrfs-convert, it doesn't really
follow the specification at all.

Thus the converted btrfs will still go v1 space cache.

[FIX]
It can be a huge change to btrfs-convert to make the initial btrfs image
to support v2 cache.

Thus this patch would change the fs at the final stage, just before we
finalize the btrfs.

This patch would drop all the v1 cache created, then call
btrfs_create_free_space_tree() to populate the free space tree and
commit the superblock with needed compat_ro flags.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changelog:
v2:
- Rebased to latest devel branch
  Which already has the orphan file support for ext4.
---
 Makefile       |  2 +-
 convert/main.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e2497c18b5e9..86c73590d118 100644
--- a/Makefile
+++ b/Makefile
@@ -253,7 +253,7 @@ libbtrfsutil_objects = libbtrfsutil/errors.o libbtrfsutil/filesystem.o \
 		       libbtrfsutil/stubs.o
 convert_objects = convert/main.o convert/common.o convert/source-fs.o \
 		  convert/source-ext2.o convert/source-reiserfs.o \
-		  mkfs/common.o
+		  mkfs/common.o check/clear-cache.o
 mkfs_objects = mkfs/main.o mkfs/common.o mkfs/rootdir.o
 image_objects = image/main.o image/sanitize.o
 tune_objects = tune/main.o tune/seeding.o tune/change-uuid.o tune/change-metadata-uuid.o \
diff --git a/convert/main.c b/convert/main.c
index 0a62101d7e48..27eece089cd4 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -99,6 +99,7 @@
 #include "kernel-shared/disk-io.h"
 #include "kernel-shared/volumes.h"
 #include "kernel-shared/transaction.h"
+#include "kernel-shared/free-space-tree.h"
 #include "kernel-shared/file-item.h"
 #include "crypto/hash.h"
 #include "common/defs.h"
@@ -116,6 +117,7 @@
 #include "common/open-utils.h"
 #include "cmds/commands.h"
 #include "check/repair.h"
+#include "check/clear-cache.h"
 #include "mkfs/common.h"
 #include "convert/common.h"
 #include "convert/source-fs.h"
@@ -1348,6 +1350,28 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 		error("unable to open ctree for finalization");
 		goto fail;
 	}
+
+	/*
+	 * Setup free space tree.
+	 *
+	 * - Clear any v1 cache first
+	 * - Create v2 free space tree
+	 */
+	if (mkfs_cfg.features.compat_ro_flags &
+	    BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE) {
+		ret = do_clear_free_space_cache(root->fs_info, 1);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to clear v1 space cache: %m");
+			goto fail;
+		}
+		ret = btrfs_create_free_space_tree(root->fs_info);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to create v2 space cache: %m");
+			goto fail;
+		}
+	}
 	root->fs_info->finalize_on_close = 1;
 	close_ctree(root);
 	close(fd);
-- 
2.40.1

Re: [PATCH v2] btrfs-progs: convert: follow the default free space tree setting

[Anand Jain]

On 29/05/2023 15:10, Qu Wenruo wrote:
> [BUG]
> We got some test failures related to btrfs-convert with subpage, e.g.
> btrfs/012, the failure would cause the following dmesg:
> 
>   BTRFS warning (device nvme0n1p7): v1 space cache is not supported for page size 16384 with sectorsize 4096
>   BTRFS error (device nvme0n1p7): open_ctree failed
> 
> [CAUSE]
> v1 space cache has tons of hardcoded PAGE_SIZE usage, and considering v2
> space cache is going to replace it (which is already the new default
> since v5.15 btrfs-progs), thus for btrfs subpage support, we just simply
> reject the v1 space cache, and utilize v2 space cache when possible.
> 
> But there is special catch in btrfs-convert, although we're specifying
> v2 space cache as the new default for btrfs-convert, it doesn't really
> follow the specification at all.
> 
> Thus the converted btrfs will still go v1 space cache.
> 
> [FIX]
> It can be a huge change to btrfs-convert to make the initial btrfs image
> to support v2 cache.
> 
> Thus this patch would change the fs at the final stage, just before we
> finalize the btrfs.
> 
> This patch would drop all the v1 cache created, then call
> btrfs_create_free_space_tree() to populate the free space tree and
> commit the superblock with needed compat_ro flags.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH v2] btrfs-progs: convert: follow the default free space tree setting

[David Sterba]

On Mon, May 29, 2023 at 03:10:10PM +0800, Qu Wenruo wrote:
> [BUG]
> We got some test failures related to btrfs-convert with subpage, e.g.
> btrfs/012, the failure would cause the following dmesg:
> 
>  BTRFS warning (device nvme0n1p7): v1 space cache is not supported for page size 16384 with sectorsize 4096
>  BTRFS error (device nvme0n1p7): open_ctree failed
> 
> [CAUSE]
> v1 space cache has tons of hardcoded PAGE_SIZE usage, and considering v2
> space cache is going to replace it (which is already the new default
> since v5.15 btrfs-progs), thus for btrfs subpage support, we just simply
> reject the v1 space cache, and utilize v2 space cache when possible.
> 
> But there is special catch in btrfs-convert, although we're specifying
> v2 space cache as the new default for btrfs-convert, it doesn't really
> follow the specification at all.
> 
> Thus the converted btrfs will still go v1 space cache.
> 
> [FIX]
> It can be a huge change to btrfs-convert to make the initial btrfs image
> to support v2 cache.
> 
> Thus this patch would change the fs at the final stage, just before we
> finalize the btrfs.

This sounds like the safest approach, though a bit wasteful.

> This patch would drop all the v1 cache created, then call
> btrfs_create_free_space_tree() to populate the free space tree and
> commit the superblock with needed compat_ro flags.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to devel, thanks.