[PATCH 0/2] btrfs-progs: convert: follow default v2 cache setting

[PATCH 0/2] btrfs-progs: convert: follow default v2 cache setting

[Qu Wenruo]

Although btrfs-convert has followed the new default setting in v5.15
release, it doesn't work.

The problem is the convert workflow itself (making an temporary btrfs
first, then sync the inodes) doesn't really create the free space tree
nor the needed super block flags.

And during the inodes sync phase, we generate v1 cache, causing the
result btrfs to be v1 cache populated, and cause test case failure for
subpage testing.


This patchset would fix the converted btrfs at the final stage, by
clearing the existing v1 cache first, then create and populate a valid
free space tree, with needed super block flags.


There seems to be a behavior change in mkfs.ext4 (e2fsprogs 1.47),
that would create an orphan inode (with fixed ino number 12), and
btrfs-convert would follow the ext4 to create an orphan in btrfs too,
causing btrfs check to complain.

So for now, I can not do convert testing due to the newly updated
e2fsprogs...

Qu Wenruo (2):
  btrfs-progs: make check/clear-cache.c to be separate from check/main.c
  btrfs-progs: convert: follow the default free space tree setting

 Makefile            |  2 +-
 check/clear-cache.c | 84 ++++++++++++++++++++++++---------------------
 check/clear-cache.h |  8 +++--
 check/main.c        |  6 ++--
 convert/main.c      | 23 +++++++++++++
 5 files changed, 76 insertions(+), 47 deletions(-)

-- 
2.39.2

[PATCH 1/2] btrfs-progs: make check/clear-cache.c to be separate from check/main.c

[Qu Wenruo]

Currently check/clear-cache.c still uses a lot of global variables like
gfs_info and g_task_ctx, which are only implemented in check/main.c.

Since we have separated clear-cache code into its own c and header files,
we should not utilize those global variables.

This provides the basis for later clear cache usage out of check realm.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/clear-cache.c | 84 ++++++++++++++++++++++++---------------------
 check/clear-cache.h |  8 +++--
 check/main.c        |  6 ++--
 3 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/check/clear-cache.c b/check/clear-cache.c
index 0a3001a4a6aa..772d920fd397 100644
--- a/check/clear-cache.c
+++ b/check/clear-cache.c
@@ -35,7 +35,7 @@
  */
 #define NR_BLOCK_GROUP_CLUSTER		(16)
 
-static int clear_free_space_cache(void)
+static int clear_free_space_cache(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_block_group *bg_cache;
@@ -43,7 +43,7 @@ static int clear_free_space_cache(void)
 	u64 current = 0;
 	int ret = 0;
 
-	trans = btrfs_start_transaction(gfs_info->tree_root, 0);
+	trans = btrfs_start_transaction(fs_info->tree_root, 0);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
 		errno = -ret;
@@ -53,7 +53,7 @@ static int clear_free_space_cache(void)
 
 	/* Clear all free space cache inodes and its extent data */
 	while (1) {
-		bg_cache = btrfs_lookup_first_block_group(gfs_info, current);
+		bg_cache = btrfs_lookup_first_block_group(fs_info, current);
 		if (!bg_cache)
 			break;
 		ret = btrfs_clear_free_space_cache(trans, bg_cache);
@@ -64,13 +64,13 @@ static int clear_free_space_cache(void)
 		nr_handled++;
 
 		if (nr_handled == NR_BLOCK_GROUP_CLUSTER) {
-			ret = btrfs_commit_transaction(trans, gfs_info->tree_root);
+			ret = btrfs_commit_transaction(trans, fs_info->tree_root);
 			if (ret < 0) {
 				errno = -ret;
 				error_msg(ERROR_MSG_START_TRANS, "%m");
 				return ret;
 			}
-			trans = btrfs_start_transaction(gfs_info->tree_root, 0);
+			trans = btrfs_start_transaction(fs_info->tree_root, 0);
 			if (IS_ERR(trans)) {
 				ret = PTR_ERR(trans);
 				errno = -ret;
@@ -81,8 +81,8 @@ static int clear_free_space_cache(void)
 		current = bg_cache->start + bg_cache->length;
 	}
 
-	btrfs_set_super_cache_generation(gfs_info->super_copy, (u64)-1);
-	ret = btrfs_commit_transaction(trans, gfs_info->tree_root);
+	btrfs_set_super_cache_generation(fs_info->super_copy, (u64)-1);
+	ret = btrfs_commit_transaction(trans, fs_info->tree_root);
 	if (ret < 0) {
 		errno = -ret;
 		error_msg(ERROR_MSG_START_TRANS, "%m");
@@ -90,16 +90,17 @@ static int clear_free_space_cache(void)
 	return ret;
 }
 
-int do_clear_free_space_cache(int clear_version)
+int do_clear_free_space_cache(struct btrfs_fs_info *fs_info,
+			      int clear_version)
 {
 	int ret = 0;
 
 	if (clear_version == 1) {
-		if (btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE))
+		if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE))
 			warning(
 "free space cache v2 detected, use --clear-space-cache v2, proceeding with clearing v1");
 
-		ret = clear_free_space_cache();
+		ret = clear_free_space_cache(fs_info);
 		if (ret) {
 			error("failed to clear free space cache");
 			ret = 1;
@@ -107,13 +108,13 @@ int do_clear_free_space_cache(int clear_version)
 			printf("Free space cache cleared\n");
 		}
 	} else if (clear_version == 2) {
-		if (!btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE)) {
+		if (!btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) {
 			printf("no free space cache v2 to clear\n");
 			ret = 0;
 			goto close_out;
 		}
 		printf("Clear free space cache v2\n");
-		ret = btrfs_clear_free_space_tree(gfs_info);
+		ret = btrfs_clear_free_space_tree(fs_info);
 		if (ret) {
 			error("failed to clear free space cache v2: %d", ret);
 			ret = 1;
@@ -127,6 +128,7 @@ close_out:
 
 static int check_free_space_tree(struct btrfs_root *root)
 {
+	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_key key = { 0 };
 	struct btrfs_path path;
 	int ret = 0;
@@ -158,7 +160,7 @@ static int check_free_space_tree(struct btrfs_root *root)
 			goto out;
 		}
 
-		bg = btrfs_lookup_first_block_group(gfs_info, key.objectid);
+		bg = btrfs_lookup_first_block_group(fs_info, key.objectid);
 		if (!bg) {
 			fprintf(stderr,
 		"We have a space info key for a block group that doesn't exist\n");
@@ -187,7 +189,7 @@ static int check_free_space_trees(struct btrfs_root *root)
 	};
 	int ret = 0;
 
-	free_space_root = btrfs_global_root(gfs_info, &key);
+	free_space_root = btrfs_global_root(root->fs_info, &key);
 	while (1) {
 		ret = check_free_space_tree(free_space_root);
 		if (ret)
@@ -214,7 +216,7 @@ static int check_cache_range(struct btrfs_root *root,
 
 	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
-		ret = btrfs_rmap_block(gfs_info,
+		ret = btrfs_rmap_block(root->fs_info,
 				       cache->start, bytenr,
 				       &logical, &nr, &stripe_len);
 		if (ret)
@@ -340,8 +342,9 @@ static int verify_space_cache(struct btrfs_root *root,
 	return ret;
 }
 
-static int check_space_cache(struct btrfs_root *root)
+static int check_space_cache(struct btrfs_root *root, struct task_ctx *task_ctx)
 {
+	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct extent_io_tree used;
 	struct btrfs_block_group *cache;
 	u64 start = BTRFS_SUPER_INFO_OFFSET + BTRFS_SUPER_INFO_SIZE;
@@ -349,20 +352,20 @@ static int check_space_cache(struct btrfs_root *root)
 	int error = 0;
 
 	extent_io_tree_init(&used);
-	ret = btrfs_mark_used_blocks(gfs_info, &used);
+	ret = btrfs_mark_used_blocks(fs_info, &used);
 	if (ret)
 		return ret;
 
 	while (1) {
-		g_task_ctx.item_count++;
-		cache = btrfs_lookup_first_block_group(gfs_info, start);
+		task_ctx->item_count++;
+		cache = btrfs_lookup_first_block_group(fs_info, start);
 		if (!cache)
 			break;
 
 		start = cache->start + cache->length;
 		if (!cache->free_space_ctl) {
 			if (btrfs_init_free_space_ctl(cache,
-						gfs_info->sectorsize)) {
+						fs_info->sectorsize)) {
 				ret = -ENOMEM;
 				break;
 			}
@@ -370,8 +373,8 @@ static int check_space_cache(struct btrfs_root *root)
 			btrfs_remove_free_space_cache(cache);
 		}
 
-		if (btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE)) {
-			ret = exclude_super_stripes(gfs_info, cache);
+		if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) {
+			ret = exclude_super_stripes(fs_info, cache);
 			if (ret) {
 				errno = -ret;
 				fprintf(stderr,
@@ -379,8 +382,8 @@ static int check_space_cache(struct btrfs_root *root)
 				error++;
 				continue;
 			}
-			ret = load_free_space_tree(gfs_info, cache);
-			free_excluded_extents(gfs_info, cache);
+			ret = load_free_space_tree(fs_info, cache);
+			free_excluded_extents(fs_info, cache);
 			if (ret < 0) {
 				errno = -ret;
 				fprintf(stderr,
@@ -390,7 +393,7 @@ static int check_space_cache(struct btrfs_root *root)
 			}
 			error += ret;
 		} else {
-			ret = load_free_space_cache(gfs_info, cache);
+			ret = load_free_space_cache(fs_info, cache);
 			if (ret < 0)
 				error++;
 			if (ret <= 0)
@@ -409,33 +412,34 @@ static int check_space_cache(struct btrfs_root *root)
 }
 
 
-int validate_free_space_cache(struct btrfs_root *root)
+int validate_free_space_cache(struct btrfs_root *root, struct task_ctx *task_ctx)
 {
+	struct btrfs_fs_info *fs_info = root->fs_info;
 	int ret;
 
 	/*
 	 * If cache generation is between 0 and -1ULL, sb generation must be
 	 * equal to sb cache generation or the v1 space caches are outdated.
 	 */
-	if (btrfs_super_cache_generation(gfs_info->super_copy) != -1ULL &&
-	    btrfs_super_cache_generation(gfs_info->super_copy) != 0 &&
-	    btrfs_super_generation(gfs_info->super_copy) !=
-	    btrfs_super_cache_generation(gfs_info->super_copy)) {
+	if (btrfs_super_cache_generation(fs_info->super_copy) != -1ULL &&
+	    btrfs_super_cache_generation(fs_info->super_copy) != 0 &&
+	    btrfs_super_generation(fs_info->super_copy) !=
+	    btrfs_super_cache_generation(fs_info->super_copy)) {
 		printf(
 "cache and super generation don't match, space cache will be invalidated\n");
 		return 0;
 	}
 
-	ret = check_space_cache(root);
-	if (!ret && btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE))
+	ret = check_space_cache(root, task_ctx);
+	if (!ret && btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE))
 		ret = check_free_space_trees(root);
-	if (ret && btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE) &&
+	if (ret && btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
 	    opt_check_repair) {
-		ret = do_clear_free_space_cache(2);
+		ret = do_clear_free_space_cache(fs_info, 2);
 		if (ret)
 			goto out;
 
-		ret = btrfs_create_free_space_tree(gfs_info);
+		ret = btrfs_create_free_space_tree(fs_info);
 		if (ret)
 			error("couldn't repair freespace tree");
 	}
@@ -539,7 +543,7 @@ out:
 	return ret;
 }
 
-int clear_ino_cache_items(void)
+int clear_ino_cache_items(struct btrfs_fs_info *fs_info)
 {
 	int ret;
 	struct btrfs_path path;
@@ -550,7 +554,7 @@ int clear_ino_cache_items(void)
 	key.offset = 0;
 
 	btrfs_init_path(&path);
-	ret = btrfs_search_slot(NULL, gfs_info->tree_root, &key, &path,	0, 0);
+	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, &path,	0, 0);
 	if (ret < 0)
 		return ret;
 
@@ -563,7 +567,7 @@ int clear_ino_cache_items(void)
 			struct btrfs_root *root;
 
 			found_key.offset = (u64)-1;
-			root = btrfs_read_fs_root(gfs_info, &found_key);
+			root = btrfs_read_fs_root(fs_info, &found_key);
 			if (IS_ERR(root))
 				goto next;
 			ret = truncate_free_ino_items(root);
@@ -586,12 +590,12 @@ next:
 		if (key.objectid == BTRFS_FS_TREE_OBJECTID) {
 			key.objectid = BTRFS_FIRST_FREE_OBJECTID;
 			btrfs_release_path(&path);
-			ret = btrfs_search_slot(NULL, gfs_info->tree_root, &key,
+			ret = btrfs_search_slot(NULL, fs_info->tree_root, &key,
 						&path,	0, 0);
 			if (ret < 0)
 				return ret;
 		} else {
-			ret = btrfs_next_item(gfs_info->tree_root, &path);
+			ret = btrfs_next_item(fs_info->tree_root, &path);
 			if (ret < 0) {
 				goto out;
 			} else if (ret > 0) {
diff --git a/check/clear-cache.h b/check/clear-cache.h
index b8b71a89df93..78845e8d9557 100644
--- a/check/clear-cache.h
+++ b/check/clear-cache.h
@@ -17,11 +17,13 @@
 #ifndef __BTRFS_CHECK_CLEAR_CACHE_H__
 #define __BTRFS_CHECK_CLEAR_CACHE_H__
 
+struct btrfs_fs_info;
 struct btrfs_root;
+struct task_ctx;
 
-int do_clear_free_space_cache(int clear_version);
-int validate_free_space_cache(struct btrfs_root *root);
+int do_clear_free_space_cache(struct btrfs_fs_info *fs_info, int clear_version);
+int validate_free_space_cache(struct btrfs_root *root, struct task_ctx *task_ctx);
 int truncate_free_ino_items(struct btrfs_root *root);
-int clear_ino_cache_items(void);
+int clear_ino_cache_items(struct btrfs_fs_info *fs_info);
 
 #endif
diff --git a/check/main.c b/check/main.c
index 9a7f40e7d7fe..74ebd3b240d4 100644
--- a/check/main.c
+++ b/check/main.c
@@ -10234,13 +10234,13 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 	}
 
 	if (clear_space_cache) {
-		ret = do_clear_free_space_cache(clear_space_cache);
+		ret = do_clear_free_space_cache(gfs_info, clear_space_cache);
 		err |= !!ret;
 		goto close_out;
 	}
 
 	if (clear_ino_cache) {
-		ret = clear_ino_cache_items();
+		ret = clear_ino_cache_items(gfs_info);
 		err = ret;
 		goto close_out;
 	}
@@ -10406,7 +10406,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 		task_start(g_task_ctx.info, &g_task_ctx.start_time, &g_task_ctx.item_count);
 	}
 
-	ret = validate_free_space_cache(root);
+	ret = validate_free_space_cache(root, &g_task_ctx);
 	task_stop(g_task_ctx.info);
 	err |= !!ret;
 
-- 
2.39.2

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

[Qu Wenruo]

[BUG]
We got some test failures related to btrfs-convert, like 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>
---
It looks like since e2fsprogs 1.47, there would be a new special inode
with ino number 12 after mkfs.ext4.

Such special inode has no parent directory entry pointing to it, thus it
would be always orphan, and cause btrfs check to complain.
As btrfs-convert strictly follows the ext inode, and created an orphan
inode in btrfs too.

Thus with newer e2fsprogs, this would cause convert tests to fail.

Still confirming with ext4 community to get a way to work around this.
---
 Makefile       |  2 +-
 convert/main.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4b0a869b6ca5..1e61bb31eb79 100644
--- a/Makefile
+++ b/Makefile
@@ -246,7 +246,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 3f54ea304e6f..b4df5a984238 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 "crypto/hash.h"
 #include "common/defs.h"
 #include "common/extent-cache.h"
@@ -115,6 +116,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"
@@ -1343,6 +1345,27 @@ 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.39.2

Re: [PATCH 0/2] btrfs-progs: convert: follow default v2 cache setting

[Neal Gompa]

On Wed, Mar 22, 2023 at 6:29 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Although btrfs-convert has followed the new default setting in v5.15
> release, it doesn't work.
>
> The problem is the convert workflow itself (making an temporary btrfs
> first, then sync the inodes) doesn't really create the free space tree
> nor the needed super block flags.
>
> And during the inodes sync phase, we generate v1 cache, causing the
> result btrfs to be v1 cache populated, and cause test case failure for
> subpage testing.
>
>
> This patchset would fix the converted btrfs at the final stage, by
> clearing the existing v1 cache first, then create and populate a valid
> free space tree, with needed super block flags.
>
>
> There seems to be a behavior change in mkfs.ext4 (e2fsprogs 1.47),
> that would create an orphan inode (with fixed ino number 12), and
> btrfs-convert would follow the ext4 to create an orphan in btrfs too,
> causing btrfs check to complain.
>
> So for now, I can not do convert testing due to the newly updated
> e2fsprogs...
>
> Qu Wenruo (2):
>   btrfs-progs: make check/clear-cache.c to be separate from check/main.c
>   btrfs-progs: convert: follow the default free space tree setting
>
>  Makefile            |  2 +-
>  check/clear-cache.c | 84 ++++++++++++++++++++++++---------------------
>  check/clear-cache.h |  8 +++--
>  check/main.c        |  6 ++--
>  convert/main.c      | 23 +++++++++++++
>  5 files changed, 76 insertions(+), 47 deletions(-)
>
> --
> 2.39.2
>

LGTM.

Reviewed-by: Neal Gompa <neal@gompa.dev>




--
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 1/2] btrfs-progs: make check/clear-cache.c to be separate from check/main.c

[David Sterba]

On Wed, Mar 22, 2023 at 06:27:04PM +0800, Qu Wenruo wrote:
> Currently check/clear-cache.c still uses a lot of global variables like
> gfs_info and g_task_ctx, which are only implemented in check/main.c.
> 
> Since we have separated clear-cache code into its own c and header files,
> we should not utilize those global variables.

Why? The global fs_info for the whole check is declared in
check/common.h and is supposed to be used for all internal check
functions. This is intentional.

Re: [PATCH 1/2] btrfs-progs: make check/clear-cache.c to be separate from check/main.c

[Qu Wenruo]

On 2023/3/24 03:03, David Sterba wrote:
> On Wed, Mar 22, 2023 at 06:27:04PM +0800, Qu Wenruo wrote:
>> Currently check/clear-cache.c still uses a lot of global variables like
>> gfs_info and g_task_ctx, which are only implemented in check/main.c.
>>
>> Since we have separated clear-cache code into its own c and header files,
>> we should not utilize those global variables.
> 
> Why? The global fs_info for the whole check is declared in
> check/common.h and is supposed to be used for all internal check
> functions. This is intentional.

Because I want to use the function do_clear_space_cache().

If we still have any gfs_info usage, it would cause compile error:

/usr/bin/ld: check/clear-cache.o: in function `clear_free_space_cache':
/home/adam/btrfs/btrfs-progs/check/clear-cache.c:46: undefined reference 
to `gfs_info'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:56: 
undefined reference to `gfs_info'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:67: 
undefined reference to `gfs_info'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:73: 
undefined reference to `gfs_info'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:84: 
undefined reference to `gfs_info'
/usr/bin/ld: 
check/clear-cache.o:/home/adam/btrfs/btrfs-progs/check/clear-cache.c:85: 
more undefined references to `gfs_info' follow
/usr/bin/ld: check/clear-cache.o: in function `check_space_cache':
/home/adam/btrfs/btrfs-progs/check/clear-cache.c:357: undefined 
reference to `g_task_ctx'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:357: 
undefined reference to `g_task_ctx'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:358: 
undefined reference to `gfs_info'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:365: 
undefined reference to `gfs_info'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:373: 
undefined reference to `gfs_info'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:374: 
undefined reference to `gfs_info'
/usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:382: 
undefined reference to `gfs_info'
/usr/bin/ld: 
check/clear-cache.o:/home/adam/btrfs/btrfs-progs/check/clear-cache.c:383: 
more undefined references to `gfs_info' follow
collect2: error: ld returned 1 exit status
make: *** [Makefile:693: btrfs-convert] Error 1

Thanks,
Qu

Re: [PATCH 1/2] btrfs-progs: make check/clear-cache.c to be separate from check/main.c

[Qu Wenruo]

On 2023/3/24 08:39, Qu Wenruo wrote:
> 
> 
> On 2023/3/24 03:03, David Sterba wrote:
>> On Wed, Mar 22, 2023 at 06:27:04PM +0800, Qu Wenruo wrote:
>>> Currently check/clear-cache.c still uses a lot of global variables like
>>> gfs_info and g_task_ctx, which are only implemented in check/main.c.
>>>
>>> Since we have separated clear-cache code into its own c and header 
>>> files,
>>> we should not utilize those global variables.
>>
>> Why? The global fs_info for the whole check is declared in
>> check/common.h and is supposed to be used for all internal check
>> functions. This is intentional.
> 
> Because I want to use the function do_clear_space_cache().
> 
> If we still have any gfs_info usage, it would cause compile error:
> 
> /usr/bin/ld: check/clear-cache.o: in function `clear_free_space_cache':
> /home/adam/btrfs/btrfs-progs/check/clear-cache.c:46: undefined reference 
> to `gfs_info'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:56: 
> undefined reference to `gfs_info'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:67: 
> undefined reference to `gfs_info'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:73: 
> undefined reference to `gfs_info'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:84: 
> undefined reference to `gfs_info'
> /usr/bin/ld: 
> check/clear-cache.o:/home/adam/btrfs/btrfs-progs/check/clear-cache.c:85: 
> more undefined references to `gfs_info' follow
> /usr/bin/ld: check/clear-cache.o: in function `check_space_cache':
> /home/adam/btrfs/btrfs-progs/check/clear-cache.c:357: undefined 
> reference to `g_task_ctx'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:357: 
> undefined reference to `g_task_ctx'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:358: 
> undefined reference to `gfs_info'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:365: 
> undefined reference to `gfs_info'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:373: 
> undefined reference to `gfs_info'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:374: 
> undefined reference to `gfs_info'
> /usr/bin/ld: /home/adam/btrfs/btrfs-progs/check/clear-cache.c:382: 
> undefined reference to `gfs_info'
> /usr/bin/ld: 
> check/clear-cache.o:/home/adam/btrfs/btrfs-progs/check/clear-cache.c:383: more undefined references to `gfs_info' follow
> collect2: error: ld returned 1 exit status
> make: *** [Makefile:693: btrfs-convert] Error 1

Any comments on this? I didn't see this patch merged.

I have some other patchsets requiring a similar work, and without this 
patch, we can not solve the convert failure on subpage case either.

Thanks,
Qu
> 
> Thanks,
> Qu