[PATCH 1/2] btrfs: add compat flag functions

[PATCH 1/2] btrfs: add compat flag functions

[Mark Harmstone]

struct btrfs_super_block contains a field compat_flags that has never
been used, intended for informational flags that don't prevent a
filesystem from being mounted read-write if the kernel doesn't
understand them.

Add the support functions needed for these, following the pattern of
compat_ro_flags and incompat_flags.

Signed-off-by: Mark Harmstone <mark@harmstone.com>
---
 fs/btrfs/fs.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/fs.h | 13 +++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/fs/btrfs/fs.c b/fs/btrfs/fs.c
index feb0a2faa837..1d7293a575cf 100644
--- a/fs/btrfs/fs.c
+++ b/fs/btrfs/fs.c
@@ -273,3 +273,49 @@ void __btrfs_clear_fs_compat_ro(struct btrfs_fs_info *fs_info, u64 flag,
 		set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags);
 	}
 }
+
+void __btrfs_set_fs_compat(struct btrfs_fs_info *fs_info, u64 flag,
+			   const char *name)
+{
+	struct btrfs_super_block *disk_super;
+	u64 features;
+
+	disk_super = fs_info->super_copy;
+	features = btrfs_super_compat_flags(disk_super);
+	if (!(features & flag)) {
+		spin_lock(&fs_info->super_lock);
+		features = btrfs_super_compat_flags(disk_super);
+		if (!(features & flag)) {
+			features |= flag;
+			btrfs_set_super_compat_flags(disk_super, features);
+			btrfs_info(fs_info,
+				"setting compat feature flag for %s (0x%llx)",
+				name, flag);
+		}
+		spin_unlock(&fs_info->super_lock);
+		set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags);
+	}
+}
+
+void __btrfs_clear_fs_compat(struct btrfs_fs_info *fs_info, u64 flag,
+			     const char *name)
+{
+	struct btrfs_super_block *disk_super;
+	u64 features;
+
+	disk_super = fs_info->super_copy;
+	features = btrfs_super_compat_flags(disk_super);
+	if (features & flag) {
+		spin_lock(&fs_info->super_lock);
+		features = btrfs_super_compat_flags(disk_super);
+		if (features & flag) {
+			features &= ~flag;
+			btrfs_set_super_compat_flags(disk_super, features);
+			btrfs_info(fs_info,
+				"clearing compat feature flag for %s (0x%llx)",
+				name, flag);
+		}
+		spin_unlock(&fs_info->super_lock);
+		set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags);
+	}
+}
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index 814bbc9417d2..3a8b4aba12d2 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -1053,6 +1053,10 @@ void __btrfs_set_fs_compat_ro(struct btrfs_fs_info *fs_info, u64 flag,
 			      const char *name);
 void __btrfs_clear_fs_compat_ro(struct btrfs_fs_info *fs_info, u64 flag,
 				const char *name);
+void __btrfs_set_fs_compat(struct btrfs_fs_info *fs_info, u64 flag,
+			   const char *name);
+void __btrfs_clear_fs_compat(struct btrfs_fs_info *fs_info, u64 flag,
+			     const char *name);
 
 #define __btrfs_fs_incompat(fs_info, flags)				\
 	(!!(btrfs_super_incompat_flags((fs_info)->super_copy) & (flags)))
@@ -1060,6 +1064,9 @@ void __btrfs_clear_fs_compat_ro(struct btrfs_fs_info *fs_info, u64 flag,
 #define __btrfs_fs_compat_ro(fs_info, flags)				\
 	(!!(btrfs_super_compat_ro_flags((fs_info)->super_copy) & (flags)))
 
+#define __btrfs_fs_compat(fs_info, flags)				\
+	(!!(btrfs_super_compat_flags((fs_info)->super_copy) & (flags)))
+
 #define btrfs_set_fs_incompat(__fs_info, opt)				\
 	__btrfs_set_fs_incompat((__fs_info), BTRFS_FEATURE_INCOMPAT_##opt, #opt)
 
@@ -1078,6 +1085,12 @@ void __btrfs_clear_fs_compat_ro(struct btrfs_fs_info *fs_info, u64 flag,
 #define btrfs_fs_compat_ro(fs_info, opt)				\
 	__btrfs_fs_compat_ro((fs_info), BTRFS_FEATURE_COMPAT_RO_##opt)
 
+#define btrfs_fs_compat(fs_info, opt)				\
+	__btrfs_fs_compat((fs_info), BTRFS_FEATURE_COMPAT_##opt)
+
+#define btrfs_set_fs_compat(__fs_info, opt)				\
+	__btrfs_set_fs_compat((__fs_info), BTRFS_FEATURE_COMPAT_##opt, #opt)
+
 #define btrfs_clear_opt(o, opt)		((o) &= ~BTRFS_MOUNT_##opt)
 #define btrfs_set_opt(o, opt)		((o) |= BTRFS_MOUNT_##opt)
 #define btrfs_raw_test_opt(o, opt)	((o) & BTRFS_MOUNT_##opt)
-- 
2.49.1

[PATCH 2/2] btrfs: clear spurious free-space entries

[Mark Harmstone]

Version 6.16.1 of btrfs-progs fixes a broken btrfs check test for
spurious entries in the free-space tree, those that don't belong to any
block group. Unfortunately mkfs.btrfs had been generating these, meaning
that these filesystems will now fail btrfs check.

Add a compat flag BTRFS_FEATURE_COMPAT_NO_SPURIOUS_FREE_SPACE, and if on
mount we find this isn't set, clean any spurious entries from the
beginning of the free-space tree.

Signed-off-by: Mark Harmstone <mark@harmstone.com>
---
 fs/btrfs/disk-io.c         |  10 ++++
 fs/btrfs/free-space-tree.c | 115 +++++++++++++++++++++++++++++++++++++
 fs/btrfs/free-space-tree.h |   1 +
 include/uapi/linux/btrfs.h |   2 +
 4 files changed, 128 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 21c2a19d690f..224369c450e4 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3077,6 +3077,16 @@ int btrfs_start_pre_rw_mount(struct btrfs_fs_info *fs_info)
 		}
 	}
 
+	if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
+	    !btrfs_fs_compat(fs_info, NO_SPURIOUS_FREE_SPACE)) {
+		ret = btrfs_remove_spurious_free_space(fs_info);
+		if (ret) {
+			btrfs_warn(fs_info,
+				   "failed to remove spurious free space: %d",
+				   ret);
+		}
+	}
+
 	/*
 	 * btrfs_find_orphan_roots() is responsible for finding all the dead
 	 * roots (with 0 refs), flag them with BTRFS_ROOT_DEAD_TREE and load
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index dad0b492a663..5980710cf6b5 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1722,3 +1722,118 @@ int btrfs_load_free_space_tree(struct btrfs_caching_control *caching_ctl)
 	else
 		return load_free_space_extents(caching_ctl, path, extent_count);
 }
+
+/*
+ * Earlier versions of mkfs.btrfs created spurious entries at the beginning of
+ * the free-space tree, before the start of any block group.
+ * If the compat flag NO_SPURIOUS_FREE_SPACE is not set, clean these up and
+ * set the flag so we know we don't have to check again.
+ */
+int btrfs_remove_spurious_free_space(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_root *fst;
+	struct btrfs_trans_handle *trans;
+	struct btrfs_key key;
+	struct extent_buffer *leaf;
+	struct btrfs_block_group *bg;
+	u64 bg_start;
+	BTRFS_PATH_AUTO_FREE(path);
+	int ret, ret2;
+	unsigned int entries_to_remove = 0;
+
+	struct btrfs_key root_key = {
+		.objectid = BTRFS_FREE_SPACE_TREE_OBJECTID,
+		.type = BTRFS_ROOT_ITEM_KEY,
+		.offset = 0,
+	};
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	fst = btrfs_grab_root(btrfs_global_root(fs_info, &root_key));
+	if (!fst)
+		return -EINVAL;
+
+	trans = btrfs_start_transaction(fst, 0);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto end;
+	}
+
+	key.objectid = 0;
+	key.type = 0;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(trans, fst, &key, path, 0, 0);
+	if (ret < 0)
+		goto end_trans;
+
+	while (true) {
+		leaf = path->nodes[0];
+		if (path->slots[0] >= btrfs_header_nritems(leaf)) {
+			ret = btrfs_next_leaf(fst, path);
+			if (ret < 0)
+				goto end_trans;
+			if (ret > 0)
+				break;
+			leaf = path->nodes[0];
+		}
+
+		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
+
+		bg = btrfs_lookup_first_block_group(fs_info, key.objectid);
+		if (!bg)
+			break;
+
+		bg_start = bg->start;
+
+		btrfs_put_block_group(bg);
+
+		if (key.objectid >= bg_start)
+			break;
+
+		entries_to_remove++;
+
+		path->slots[0]++;
+	}
+
+	if (entries_to_remove == 0) {
+		ret = 0;
+		goto end_trans;
+	}
+
+	btrfs_release_path(path);
+
+	key.objectid = 0;
+	key.type = 0;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(trans, fst, &key, path, -1, 1);
+	if (ret < 0)
+		goto end_trans;
+
+	ret = btrfs_del_items(trans, fst, path, 0, entries_to_remove);
+	if (ret)
+		btrfs_abort_transaction(trans, ret);
+
+end_trans:
+	btrfs_release_path(path);
+
+	if (!ret)
+		btrfs_set_fs_compat(fs_info, NO_SPURIOUS_FREE_SPACE);
+
+	ret2 = btrfs_commit_transaction(trans);
+	if (!ret)
+		ret = ret2;
+
+	if (!ret && entries_to_remove > 0) {
+		btrfs_info(fs_info, "removed %u spurious free-space entries",
+			   entries_to_remove);
+	}
+
+end:
+	btrfs_put_root(fst);
+
+	return ret;
+}
diff --git a/fs/btrfs/free-space-tree.h b/fs/btrfs/free-space-tree.h
index 3d9a5d4477fc..b501c41acf3b 100644
--- a/fs/btrfs/free-space-tree.h
+++ b/fs/btrfs/free-space-tree.h
@@ -35,6 +35,7 @@ int btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
 				 u64 start, u64 size);
 int btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
 				      u64 start, u64 size);
+int btrfs_remove_spurious_free_space(struct btrfs_fs_info *fs_info);
 
 #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
 struct btrfs_free_space_info *
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 8e710bbb688e..6219e2b8e334 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -337,6 +337,8 @@ struct btrfs_ioctl_fs_info_args {
 #define BTRFS_FEATURE_INCOMPAT_RAID_STRIPE_TREE	(1ULL << 14)
 #define BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA	(1ULL << 16)
 
+#define BTRFS_FEATURE_COMPAT_NO_SPURIOUS_FREE_SPACE	(1ULL << 0)
+
 struct btrfs_ioctl_feature_flags {
 	__u64 compat_flags;
 	__u64 compat_ro_flags;
-- 
2.49.1

Re: [PATCH 2/2] btrfs: clear spurious free-space entries

[Filipe Manana]

On Tue, Sep 23, 2025 at 4:55 PM Mark Harmstone <mark@harmstone.com> wrote:
>
> Version 6.16.1 of btrfs-progs fixes a broken btrfs check test for
> spurious entries in the free-space tree, those that don't belong to any
> block group. Unfortunately mkfs.btrfs had been generating these, meaning
> that these filesystems will now fail btrfs check.
>
> Add a compat flag BTRFS_FEATURE_COMPAT_NO_SPURIOUS_FREE_SPACE, and if on
> mount we find this isn't set, clean any spurious entries from the
> beginning of the free-space tree.
>
> Signed-off-by: Mark Harmstone <mark@harmstone.com>
> ---
>  fs/btrfs/disk-io.c         |  10 ++++
>  fs/btrfs/free-space-tree.c | 115 +++++++++++++++++++++++++++++++++++++
>  fs/btrfs/free-space-tree.h |   1 +
>  include/uapi/linux/btrfs.h |   2 +
>  4 files changed, 128 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 21c2a19d690f..224369c450e4 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3077,6 +3077,16 @@ int btrfs_start_pre_rw_mount(struct btrfs_fs_info *fs_info)
>                 }
>         }
>
> +       if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
> +           !btrfs_fs_compat(fs_info, NO_SPURIOUS_FREE_SPACE)) {
> +               ret = btrfs_remove_spurious_free_space(fs_info);
> +               if (ret) {
> +                       btrfs_warn(fs_info,
> +                                  "failed to remove spurious free space: %d",
> +                                  ret);

There's no need to split the line, we tolerate more than 80 characters
nowadays up to a reasonable limit, say close to 90.

> +               }
> +       }
> +
>         /*
>          * btrfs_find_orphan_roots() is responsible for finding all the dead
>          * roots (with 0 refs), flag them with BTRFS_ROOT_DEAD_TREE and load
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index dad0b492a663..5980710cf6b5 100644
> --- a/fs/btrfs/free-space-tree.c
> +++ b/fs/btrfs/free-space-tree.c
> @@ -1722,3 +1722,118 @@ int btrfs_load_free_space_tree(struct btrfs_caching_control *caching_ctl)
>         else
>                 return load_free_space_extents(caching_ctl, path, extent_count);
>  }
> +
> +/*
> + * Earlier versions of mkfs.btrfs created spurious entries at the beginning of
> + * the free-space tree, before the start of any block group.
> + * If the compat flag NO_SPURIOUS_FREE_SPACE is not set, clean these up and
> + * set the flag so we know we don't have to check again.
> + */
> +int btrfs_remove_spurious_free_space(struct btrfs_fs_info *fs_info)
> +{
> +       struct btrfs_root *fst;
> +       struct btrfs_trans_handle *trans;
> +       struct btrfs_key key;
> +       struct extent_buffer *leaf;
> +       struct btrfs_block_group *bg;
> +       u64 bg_start;
> +       BTRFS_PATH_AUTO_FREE(path);
> +       int ret, ret2;
> +       unsigned int entries_to_remove = 0;
> +
> +       struct btrfs_key root_key = {

Please don't leave a new line between variable declarations.
Also some of these variables are only used inside the loop, like
'leaf' and 'bg_start', so they should be declared inside that block.

> +               .objectid = BTRFS_FREE_SPACE_TREE_OBJECTID,
> +               .type = BTRFS_ROOT_ITEM_KEY,
> +               .offset = 0,
> +       };
> +
> +       path = btrfs_alloc_path();
> +       if (!path)
> +               return -ENOMEM;
> +
> +       fst = btrfs_grab_root(btrfs_global_root(fs_info, &root_key));
> +       if (!fst)
> +               return -EINVAL;

Not sure if -EINVAL is the best error here, or even if we should error out.

> +
> +       trans = btrfs_start_transaction(fst, 0);
> +       if (IS_ERR(trans)) {
> +               ret = PTR_ERR(trans);
> +               goto end;
> +       }
> +
> +       key.objectid = 0;
> +       key.type = 0;
> +       key.offset = 0;
> +
> +       ret = btrfs_search_slot(trans, fst, &key, path, 0, 0);
> +       if (ret < 0)
> +               goto end_trans;
> +
> +       while (true) {
> +               leaf = path->nodes[0];
> +               if (path->slots[0] >= btrfs_header_nritems(leaf)) {
> +                       ret = btrfs_next_leaf(fst, path);
> +                       if (ret < 0)
> +                               goto end_trans;
> +                       if (ret > 0)
> +                               break;
> +                       leaf = path->nodes[0];
> +               }
> +
> +               btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
> +
> +               bg = btrfs_lookup_first_block_group(fs_info, key.objectid);
> +               if (!bg)
> +                       break;
> +
> +               bg_start = bg->start;
> +
> +               btrfs_put_block_group(bg);
> +
> +               if (key.objectid >= bg_start)
> +                       break;
> +
> +               entries_to_remove++;
> +
> +               path->slots[0]++;
> +       }
> +
> +       if (entries_to_remove == 0) {
> +               ret = 0;
> +               goto end_trans;
> +       }
> +
> +       btrfs_release_path(path);
> +
> +       key.objectid = 0;
> +       key.type = 0;
> +       key.offset = 0;
> +
> +       ret = btrfs_search_slot(trans, fst, &key, path, -1, 1);
> +       if (ret < 0)
> +               goto end_trans;
> +
> +       ret = btrfs_del_items(trans, fst, path, 0, entries_to_remove);

So this all works fine if all items to delete are in a single leaf.
If they aren't, what happens?

btrfs_del_items() is prepared to process a single leaf, and if the
number of items to delete is greater than the number of items in the
leaf, we get some underflows in some operations there, like when we do
"nritems - nr", etc.

> +       if (ret)
> +               btrfs_abort_transaction(trans, ret);
> +
> +end_trans:
> +       btrfs_release_path(path);
> +
> +       if (!ret)
> +               btrfs_set_fs_compat(fs_info, NO_SPURIOUS_FREE_SPACE);
> +
> +       ret2 = btrfs_commit_transaction(trans);
> +       if (!ret)
> +               ret = ret2;
> +
> +       if (!ret && entries_to_remove > 0) {
> +               btrfs_info(fs_info, "removed %u spurious free-space entries",
> +                          entries_to_remove);

This can also stay in a single line.

> +       }
> +
> +end:
> +       btrfs_put_root(fst);
> +
> +       return ret;
> +}
> diff --git a/fs/btrfs/free-space-tree.h b/fs/btrfs/free-space-tree.h
> index 3d9a5d4477fc..b501c41acf3b 100644
> --- a/fs/btrfs/free-space-tree.h
> +++ b/fs/btrfs/free-space-tree.h
> @@ -35,6 +35,7 @@ int btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
>                                  u64 start, u64 size);
>  int btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
>                                       u64 start, u64 size);
> +int btrfs_remove_spurious_free_space(struct btrfs_fs_info *fs_info);
>
>  #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>  struct btrfs_free_space_info *
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 8e710bbb688e..6219e2b8e334 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -337,6 +337,8 @@ struct btrfs_ioctl_fs_info_args {
>  #define BTRFS_FEATURE_INCOMPAT_RAID_STRIPE_TREE        (1ULL << 14)
>  #define BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA    (1ULL << 16)
>
> +#define BTRFS_FEATURE_COMPAT_NO_SPURIOUS_FREE_SPACE    (1ULL << 0)
> +
>  struct btrfs_ioctl_feature_flags {
>         __u64 compat_flags;
>         __u64 compat_ro_flags;
> --
> 2.49.1
>

Re: [PATCH 2/2] btrfs: clear spurious free-space entries

[Qu Wenruo]

在 2025/9/24 01:24, Mark Harmstone 写道:
> Version 6.16.1 of btrfs-progs fixes a broken btrfs check test for
> spurious entries in the free-space tree, those that don't belong to any
> block group. Unfortunately mkfs.btrfs had been generating these, meaning
> that these filesystems will now fail btrfs check.
> 
> Add a compat flag BTRFS_FEATURE_COMPAT_NO_SPURIOUS_FREE_SPACE, and if on
> mount we find this isn't set, clean any spurious entries from the
> beginning of the free-space tree.

I found this compat flag a little overkilled.

Are we really going to introduce a new compat flag every time there is 
something wrong with the free space tree?

> 
> Signed-off-by: Mark Harmstone <mark@harmstone.com>
> ---
>   fs/btrfs/disk-io.c         |  10 ++++
>   fs/btrfs/free-space-tree.c | 115 +++++++++++++++++++++++++++++++++++++
>   fs/btrfs/free-space-tree.h |   1 +
>   include/uapi/linux/btrfs.h |   2 +
>   4 files changed, 128 insertions(+)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 21c2a19d690f..224369c450e4 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3077,6 +3077,16 @@ int btrfs_start_pre_rw_mount(struct btrfs_fs_info *fs_info)
>   		}
>   	}
>   
> +	if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
> +	    !btrfs_fs_compat(fs_info, NO_SPURIOUS_FREE_SPACE)) {
> +		ret = btrfs_remove_spurious_free_space(fs_info);
> +		if (ret) {
> +			btrfs_warn(fs_info,
> +				   "failed to remove spurious free space: %d",
> +				   ret);
> +		}
> +	}
> +
>   	/*
>   	 * btrfs_find_orphan_roots() is responsible for finding all the dead
>   	 * roots (with 0 refs), flag them with BTRFS_ROOT_DEAD_TREE and load
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index dad0b492a663..5980710cf6b5 100644
> --- a/fs/btrfs/free-space-tree.c
> +++ b/fs/btrfs/free-space-tree.c
> @@ -1722,3 +1722,118 @@ int btrfs_load_free_space_tree(struct btrfs_caching_control *caching_ctl)
>   	else
>   		return load_free_space_extents(caching_ctl, path, extent_count);
>   }
> +
> +/*
> + * Earlier versions of mkfs.btrfs created spurious entries at the beginning of
> + * the free-space tree, before the start of any block group.
> + * If the compat flag NO_SPURIOUS_FREE_SPACE is not set, clean these up and
> + * set the flag so we know we don't have to check again.
> + */
> +int btrfs_remove_spurious_free_space(struct btrfs_fs_info *fs_info)
> +{
> +	struct btrfs_root *fst;
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_key key;
> +	struct extent_buffer *leaf;
> +	struct btrfs_block_group *bg;
> +	u64 bg_start;
> +	BTRFS_PATH_AUTO_FREE(path);
> +	int ret, ret2;
> +	unsigned int entries_to_remove = 0;
> +
> +	struct btrfs_key root_key = {
> +		.objectid = BTRFS_FREE_SPACE_TREE_OBJECTID,
> +		.type = BTRFS_ROOT_ITEM_KEY,
> +		.offset = 0,
> +	};
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +
> +	fst = btrfs_grab_root(btrfs_global_root(fs_info, &root_key));
> +	if (!fst)
> +		return -EINVAL;
> +
> +	trans = btrfs_start_transaction(fst, 0);
> +	if (IS_ERR(trans)) {
> +		ret = PTR_ERR(trans);
> +		goto end;
> +	}
> +
> +	key.objectid = 0;
> +	key.type = 0;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(trans, fst, &key, path, 0, 0);
> +	if (ret < 0)
> +		goto end_trans;
> +
> +	while (true) {
> +		leaf = path->nodes[0];
> +		if (path->slots[0] >= btrfs_header_nritems(leaf)) {
> +			ret = btrfs_next_leaf(fst, path);
> +			if (ret < 0)
> +				goto end_trans;
> +			if (ret > 0)
> +				break;
> +			leaf = path->nodes[0];
> +		}
> +
> +		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
> +
> +		bg = btrfs_lookup_first_block_group(fs_info, key.objectid);

We can do the lookup out of the loop. With a parameter 0 for @bytenr.

As we just need to delete any entry before the first bg, there is no 
need to lookup the bg every time.

Furthermore, since the stale entries are just from the temporary chunks, 
they should be pretty small, thus we are able to afford the check at 
every mount.

> +		if (!bg)
> +			break;
> +
> +		bg_start = bg->start;
> +
> +		btrfs_put_block_group(bg);
> +
> +		if (key.objectid >= bg_start)
> +			break;
> +
> +		entries_to_remove++;
> +
> +		path->slots[0]++;
> +	}
> +
> +	if (entries_to_remove == 0) {
> +		ret = 0;
> +		goto end_trans;
> +	}
> +
> +	btrfs_release_path(path);
> +
> +	key.objectid = 0;
> +	key.type = 0;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(trans, fst, &key, path, -1, 1);
> +	if (ret < 0)
> +		goto end_trans;
> +
> +	ret = btrfs_del_items(trans, fst, path, 0, entries_to_remove);

What if @entries_to_remove is larger than the items in the leaf?
I know there should not be that many entries, but we should still take 
extra cares just in case.

Thanks,
Qu
> +	if (ret)
> +		btrfs_abort_transaction(trans, ret);
> +
> +end_trans:
> +	btrfs_release_path(path);
> +
> +	if (!ret)
> +		btrfs_set_fs_compat(fs_info, NO_SPURIOUS_FREE_SPACE);
> +
> +	ret2 = btrfs_commit_transaction(trans);
> +	if (!ret)
> +		ret = ret2;
> +
> +	if (!ret && entries_to_remove > 0) {
> +		btrfs_info(fs_info, "removed %u spurious free-space entries",
> +			   entries_to_remove);
> +	}
> +
> +end:
> +	btrfs_put_root(fst);
> +
> +	return ret;
> +}
> diff --git a/fs/btrfs/free-space-tree.h b/fs/btrfs/free-space-tree.h
> index 3d9a5d4477fc..b501c41acf3b 100644
> --- a/fs/btrfs/free-space-tree.h
> +++ b/fs/btrfs/free-space-tree.h
> @@ -35,6 +35,7 @@ int btrfs_add_to_free_space_tree(struct btrfs_trans_handle *trans,
>   				 u64 start, u64 size);
>   int btrfs_remove_from_free_space_tree(struct btrfs_trans_handle *trans,
>   				      u64 start, u64 size);
> +int btrfs_remove_spurious_free_space(struct btrfs_fs_info *fs_info);
>   
>   #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>   struct btrfs_free_space_info *
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 8e710bbb688e..6219e2b8e334 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -337,6 +337,8 @@ struct btrfs_ioctl_fs_info_args {
>   #define BTRFS_FEATURE_INCOMPAT_RAID_STRIPE_TREE	(1ULL << 14)
>   #define BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA	(1ULL << 16)
>   
> +#define BTRFS_FEATURE_COMPAT_NO_SPURIOUS_FREE_SPACE	(1ULL << 0)
> +
>   struct btrfs_ioctl_feature_flags {
>   	__u64 compat_flags;
>   	__u64 compat_ro_flags;

Re: [PATCH 2/2] btrfs: clear spurious free-space entries

[David Sterba]

On Wed, Sep 24, 2025 at 06:35:43AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2025/9/24 01:24, Mark Harmstone 写道:
> > Version 6.16.1 of btrfs-progs fixes a broken btrfs check test for
> > spurious entries in the free-space tree, those that don't belong to any
> > block group. Unfortunately mkfs.btrfs had been generating these, meaning
> > that these filesystems will now fail btrfs check.
> > 
> > Add a compat flag BTRFS_FEATURE_COMPAT_NO_SPURIOUS_FREE_SPACE, and if on
> > mount we find this isn't set, clean any spurious entries from the
> > beginning of the free-space tree.
> 
> I found this compat flag a little overkilled.
> 
> Are we really going to introduce a new compat flag every time there is 
> something wrong with the free space tree?

Agreed, the compat flag does not make sense to me in this case.

> > +	ret = btrfs_search_slot(trans, fst, &key, path, 0, 0);
> > +	if (ret < 0)
> > +		goto end_trans;
> > +
> > +	while (true) {
> > +		leaf = path->nodes[0];
> > +		if (path->slots[0] >= btrfs_header_nritems(leaf)) {
> > +			ret = btrfs_next_leaf(fst, path);
> > +			if (ret < 0)
> > +				goto end_trans;
> > +			if (ret > 0)
> > +				break;
> > +			leaf = path->nodes[0];
> > +		}
> > +
> > +		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
> > +
> > +		bg = btrfs_lookup_first_block_group(fs_info, key.objectid);
> 
> We can do the lookup out of the loop. With a parameter 0 for @bytenr.
> 
> As we just need to delete any entry before the first bg, there is no 
> need to lookup the bg every time.
> 
> Furthermore, since the stale entries are just from the temporary chunks, 
> they should be pretty small, thus we are able to afford the check at 
> every mount.

A one time check per mount sounds like the best solution, it has
negligible cost, compared to compat bits.