[PATCH 0/9] btrfs-progs: mkfs fixes and enhancements for extent tree v2

[PATCH 0/9] btrfs-progs: mkfs fixes and enhancements for extent tree v2

[Josef Bacik]

Hello,

In order to reduce the amount of pain the reviewers have to endure I'm going to
be sending any prepatory patches separately from the actual feature work.

To that end this is the first batch of preparatory patches.  These are to make
working with mkfs a lot easier for the changes I'm making.  These are all fixes
or enhancements that can apply currently.  The only thing that is extent tree v2
specific is the last patch, which adds the incompat flag.

I've added the patch for the incompat flag because I will have other preparatory
patches that add helpers that essentially do

if (!btrfs_fs_incompat(fs_info, EXTENT_TREE_V2))
	/* Do the old thing. */

and then have patches after that add the extent tree v2 magic.  I think this
will make it easier to break up the work, but if we're not comfortable reserving
the bit then I'm fine with dropping that last patch.  It will just mean future
prep work will have to come along with the feature enablement patches.

I've tested this with make test to validate I didn't break anything.  Thanks,

Josef

Josef Bacik (9):
  btrfs-progs: use an associative array for init mkfs blocks
  btrfs-progs: use blocks_nr to determine the super used bytes
  btrfs-progs: allocate blocks from the start of the temp system chunk
  btrfs-progs: set nritems based on root items written
  btrfs-progs: add helper for writing empty tree nodes
  btrfs-progs: add the block group item in make_btrfs()
  btrfs-progs: add add_block_group_free_space helper
  btrfs-progs: generate free space tree at make_btrfs() time
  btrfs-progs: add the incompat flag for extent tree v2

 kernel-shared/ctree.h           |   1 +
 kernel-shared/extent-tree.c     |   2 +
 kernel-shared/free-space-tree.c |  26 ++++
 kernel-shared/free-space-tree.h |   2 +
 mkfs/common.c                   | 209 +++++++++++++++++++++++++-------
 mkfs/common.h                   |  19 +++
 mkfs/main.c                     |  18 +--
 7 files changed, 218 insertions(+), 59 deletions(-)

-- 
2.26.3

[PATCH 1/9] btrfs-progs: use an associative array for init mkfs blocks

[Josef Bacik]

Extent tree v2 will not create an extent tree or csum tree initially,
and it will create a block group tree.  To handle this we want to rework
the initial mkfs step to take an array of the blocks we want to create
and use the array to keep track of which blocks we need to create.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 mkfs/common.c | 56 ++++++++++++++++++++++++++++++++-------------------
 mkfs/common.h | 10 +++++++++
 2 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index 2c041224..e9ff529a 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -31,16 +31,18 @@
 #include "mkfs/common.h"
 
 static u64 reference_root_table[] = {
-	[1] =	BTRFS_ROOT_TREE_OBJECTID,
-	[2] =	BTRFS_EXTENT_TREE_OBJECTID,
-	[3] =	BTRFS_CHUNK_TREE_OBJECTID,
-	[4] =	BTRFS_DEV_TREE_OBJECTID,
-	[5] =	BTRFS_FS_TREE_OBJECTID,
-	[6] =	BTRFS_CSUM_TREE_OBJECTID,
+	[MKFS_ROOT_TREE]	=	BTRFS_ROOT_TREE_OBJECTID,
+	[MKFS_EXTENT_TREE]	=	BTRFS_EXTENT_TREE_OBJECTID,
+	[MKFS_CHUNK_TREE]	=	BTRFS_CHUNK_TREE_OBJECTID,
+	[MKFS_DEV_TREE]		=	BTRFS_DEV_TREE_OBJECTID,
+	[MKFS_FS_TREE]		=	BTRFS_FS_TREE_OBJECTID,
+	[MKFS_CSUM_TREE]	=	BTRFS_CSUM_TREE_OBJECTID,
 };
 
 static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
-			struct extent_buffer *buf)
+				  struct extent_buffer *buf,
+				  const enum btrfs_mkfs_block *blocks,
+				  int blocks_nr)
 {
 	struct btrfs_root_item root_item;
 	struct btrfs_inode_item *inode_item;
@@ -49,6 +51,7 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 	u32 itemoff;
 	int ret = 0;
 	int blk;
+	int i;
 	u8 uuid[BTRFS_UUID_SIZE];
 
 	memset(buf->data + sizeof(struct btrfs_header), 0,
@@ -71,7 +74,8 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 	btrfs_set_disk_key_offset(&disk_key, 0);
 	itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize) - sizeof(root_item);
 
-	for (blk = 0; blk < MKFS_BLOCK_COUNT; blk++) {
+	for (i = 0; i < blocks_nr; i++) {
+		blk = blocks[i];
 		if (blk == MKFS_SUPER_BLOCK || blk == MKFS_ROOT_TREE
 		    || blk == MKFS_CHUNK_TREE)
 			continue;
@@ -145,10 +149,13 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	struct btrfs_chunk *chunk;
 	struct btrfs_dev_item *dev_item;
 	struct btrfs_dev_extent *dev_extent;
+	const enum btrfs_mkfs_block *blocks = extent_tree_v1_blocks;
 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
 	u8 *ptr;
 	int i;
 	int ret;
+	int blocks_nr = ARRAY_SIZE(extent_tree_v1_blocks);
+	int blk;
 	u32 itemoff;
 	u32 nritems = 0;
 	u64 first_free;
@@ -195,8 +202,11 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	uuid_generate(chunk_tree_uuid);
 
 	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
-	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
-		cfg->blocks[i] = system_group_offset + cfg->nodesize * (i - 1);
+	for (i = 0; i < blocks_nr; i++) {
+		blk = blocks[i];
+		if (blk == MKFS_SUPER_BLOCK)
+			continue;
+		cfg->blocks[blk] = system_group_offset + cfg->nodesize * i;
 	}
 
 	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
@@ -236,7 +246,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 			    btrfs_header_chunk_tree_uuid(buf),
 			    BTRFS_UUID_SIZE);
 
-	ret = btrfs_create_tree_root(fd, cfg, buf);
+	ret = btrfs_create_tree_root(fd, cfg, buf, blocks, blocks_nr);
 	if (ret < 0)
 		goto out;
 
@@ -245,30 +255,34 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 		cfg->nodesize - sizeof(struct btrfs_header));
 	nritems = 0;
 	itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize);
-	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
+	for (i = 0; i < blocks_nr; i++) {
+		blk = blocks[i];
+		if (blk == MKFS_SUPER_BLOCK)
+			continue;
+
 		item_size = sizeof(struct btrfs_extent_item);
 		if (!skinny_metadata)
 			item_size += sizeof(struct btrfs_tree_block_info);
 
-		if (cfg->blocks[i] < first_free) {
+		if (cfg->blocks[blk] < first_free) {
 			error("block[%d] below first free: %llu < %llu",
-					i, (unsigned long long)cfg->blocks[i],
+					i, (unsigned long long)cfg->blocks[blk],
 					(unsigned long long)first_free);
 			ret = -EINVAL;
 			goto out;
 		}
-		if (cfg->blocks[i] < cfg->blocks[i - 1]) {
+		if (cfg->blocks[blk] < cfg->blocks[blocks[i - 1]]) {
 			error("blocks %d and %d in reverse order: %llu < %llu",
-				i, i - 1,
-				(unsigned long long)cfg->blocks[i],
-				(unsigned long long)cfg->blocks[i - 1]);
+				blk, blocks[i - 1],
+				(unsigned long long)cfg->blocks[blk],
+				(unsigned long long)cfg->blocks[blocks[i - 1]]);
 			ret = -EINVAL;
 			goto out;
 		}
 
 		/* create extent item */
 		itemoff -= item_size;
-		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[i]);
+		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[blk]);
 		if (skinny_metadata) {
 			btrfs_set_disk_key_type(&disk_key,
 						BTRFS_METADATA_ITEM_KEY);
@@ -292,8 +306,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 		nritems++;
 
 		/* create extent ref */
-		ref_root = reference_root_table[i];
-		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[i]);
+		ref_root = reference_root_table[blk];
+		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[blk]);
 		btrfs_set_disk_key_offset(&disk_key, ref_root);
 		btrfs_set_disk_key_type(&disk_key, BTRFS_TREE_BLOCK_REF_KEY);
 		btrfs_set_item_key(buf, &disk_key, nritems);
diff --git a/mkfs/common.h b/mkfs/common.h
index ea87c3ca..378da6bd 100644
--- a/mkfs/common.h
+++ b/mkfs/common.h
@@ -55,6 +55,16 @@ enum btrfs_mkfs_block {
 	MKFS_BLOCK_COUNT
 };
 
+static const enum btrfs_mkfs_block extent_tree_v1_blocks[] = {
+	MKFS_SUPER_BLOCK,
+	MKFS_ROOT_TREE,
+	MKFS_EXTENT_TREE,
+	MKFS_CHUNK_TREE,
+	MKFS_DEV_TREE,
+	MKFS_FS_TREE,
+	MKFS_CSUM_TREE,
+};
+
 struct btrfs_mkfs_config {
 	/* Label of the new filesystem */
 	const char *label;
-- 
2.26.3

Re: [PATCH 1/9] btrfs-progs: use an associative array for init mkfs blocks

[Qu Wenruo]

On 2021/8/21 上午3:11, Josef Bacik wrote:
> Extent tree v2 will not create an extent tree or csum tree initially,
> and it will create a block group tree.  To handle this we want to rework
> the initial mkfs step to take an array of the blocks we want to create
> and use the array to keep track of which blocks we need to create.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   mkfs/common.c | 56 ++++++++++++++++++++++++++++++++-------------------
>   mkfs/common.h | 10 +++++++++
>   2 files changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/mkfs/common.c b/mkfs/common.c
> index 2c041224..e9ff529a 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -31,16 +31,18 @@
>   #include "mkfs/common.h"
>
>   static u64 reference_root_table[] = {
> -	[1] =	BTRFS_ROOT_TREE_OBJECTID,
> -	[2] =	BTRFS_EXTENT_TREE_OBJECTID,
> -	[3] =	BTRFS_CHUNK_TREE_OBJECTID,
> -	[4] =	BTRFS_DEV_TREE_OBJECTID,
> -	[5] =	BTRFS_FS_TREE_OBJECTID,
> -	[6] =	BTRFS_CSUM_TREE_OBJECTID,
> +	[MKFS_ROOT_TREE]	=	BTRFS_ROOT_TREE_OBJECTID,
> +	[MKFS_EXTENT_TREE]	=	BTRFS_EXTENT_TREE_OBJECTID,
> +	[MKFS_CHUNK_TREE]	=	BTRFS_CHUNK_TREE_OBJECTID,
> +	[MKFS_DEV_TREE]		=	BTRFS_DEV_TREE_OBJECTID,
> +	[MKFS_FS_TREE]		=	BTRFS_FS_TREE_OBJECTID,
> +	[MKFS_CSUM_TREE]	=	BTRFS_CSUM_TREE_OBJECTID,
>   };

Can we use a bitmap for these combinations?

So that we just need one parameter to indicate what trees we want.

>
>   static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
> -			struct extent_buffer *buf)
> +				  struct extent_buffer *buf,
> +				  const enum btrfs_mkfs_block *blocks,
> +				  int blocks_nr)
>   {
>   	struct btrfs_root_item root_item;
>   	struct btrfs_inode_item *inode_item;
> @@ -49,6 +51,7 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>   	u32 itemoff;
>   	int ret = 0;
>   	int blk;
> +	int i;
>   	u8 uuid[BTRFS_UUID_SIZE];
>
>   	memset(buf->data + sizeof(struct btrfs_header), 0,
> @@ -71,7 +74,8 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>   	btrfs_set_disk_key_offset(&disk_key, 0);
>   	itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize) - sizeof(root_item);
>
> -	for (blk = 0; blk < MKFS_BLOCK_COUNT; blk++) {
> +	for (i = 0; i < blocks_nr; i++) {
> +		blk = blocks[i];
>   		if (blk == MKFS_SUPER_BLOCK || blk == MKFS_ROOT_TREE
>   		    || blk == MKFS_CHUNK_TREE)
>   			continue;
> @@ -145,10 +149,13 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	struct btrfs_chunk *chunk;
>   	struct btrfs_dev_item *dev_item;
>   	struct btrfs_dev_extent *dev_extent;
> +	const enum btrfs_mkfs_block *blocks = extent_tree_v1_blocks;
>   	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
>   	u8 *ptr;
>   	int i;
>   	int ret;
> +	int blocks_nr = ARRAY_SIZE(extent_tree_v1_blocks);
> +	int blk;
>   	u32 itemoff;
>   	u32 nritems = 0;
>   	u64 first_free;
> @@ -195,8 +202,11 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	uuid_generate(chunk_tree_uuid);
>
>   	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
> -	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
> -		cfg->blocks[i] = system_group_offset + cfg->nodesize * (i - 1);
> +	for (i = 0; i < blocks_nr; i++) {
> +		blk = blocks[i];
> +		if (blk == MKFS_SUPER_BLOCK)
> +			continue;
> +		cfg->blocks[blk] = system_group_offset + cfg->nodesize * i;
>   	}
>
>   	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
> @@ -236,7 +246,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   			    btrfs_header_chunk_tree_uuid(buf),
>   			    BTRFS_UUID_SIZE);
>
> -	ret = btrfs_create_tree_root(fd, cfg, buf);
> +	ret = btrfs_create_tree_root(fd, cfg, buf, blocks, blocks_nr);
>   	if (ret < 0)
>   		goto out;
>
> @@ -245,30 +255,34 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   		cfg->nodesize - sizeof(struct btrfs_header));
>   	nritems = 0;
>   	itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize);
> -	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
> +	for (i = 0; i < blocks_nr; i++) {
> +		blk = blocks[i];
> +		if (blk == MKFS_SUPER_BLOCK)
> +			continue;
> +
>   		item_size = sizeof(struct btrfs_extent_item);
>   		if (!skinny_metadata)
>   			item_size += sizeof(struct btrfs_tree_block_info);
>
> -		if (cfg->blocks[i] < first_free) {
> +		if (cfg->blocks[blk] < first_free) {
>   			error("block[%d] below first free: %llu < %llu",
> -					i, (unsigned long long)cfg->blocks[i],
> +					i, (unsigned long long)cfg->blocks[blk],
>   					(unsigned long long)first_free);
>   			ret = -EINVAL;
>   			goto out;
>   		}
> -		if (cfg->blocks[i] < cfg->blocks[i - 1]) {
> +		if (cfg->blocks[blk] < cfg->blocks[blocks[i - 1]]) {
>   			error("blocks %d and %d in reverse order: %llu < %llu",
> -				i, i - 1,
> -				(unsigned long long)cfg->blocks[i],
> -				(unsigned long long)cfg->blocks[i - 1]);
> +				blk, blocks[i - 1],
> +				(unsigned long long)cfg->blocks[blk],
> +				(unsigned long long)cfg->blocks[blocks[i - 1]]);
>   			ret = -EINVAL;
>   			goto out;
>   		}
>
>   		/* create extent item */
>   		itemoff -= item_size;
> -		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[i]);
> +		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[blk]);
>   		if (skinny_metadata) {
>   			btrfs_set_disk_key_type(&disk_key,
>   						BTRFS_METADATA_ITEM_KEY);
> @@ -292,8 +306,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   		nritems++;
>
>   		/* create extent ref */
> -		ref_root = reference_root_table[i];
> -		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[i]);
> +		ref_root = reference_root_table[blk];
> +		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[blk]);
>   		btrfs_set_disk_key_offset(&disk_key, ref_root);
>   		btrfs_set_disk_key_type(&disk_key, BTRFS_TREE_BLOCK_REF_KEY);
>   		btrfs_set_item_key(buf, &disk_key, nritems);
> diff --git a/mkfs/common.h b/mkfs/common.h
> index ea87c3ca..378da6bd 100644
> --- a/mkfs/common.h
> +++ b/mkfs/common.h
> @@ -55,6 +55,16 @@ enum btrfs_mkfs_block {
>   	MKFS_BLOCK_COUNT
>   };
>
> +static const enum btrfs_mkfs_block extent_tree_v1_blocks[] = {
> +	MKFS_SUPER_BLOCK,

Do we really need this special enum for superblock?

It's always in the fixed offset on-disk.

Despite it looks like a good cleanup.

Thanks,
Qu
> +	MKFS_ROOT_TREE,
> +	MKFS_EXTENT_TREE,
> +	MKFS_CHUNK_TREE,
> +	MKFS_DEV_TREE,
> +	MKFS_FS_TREE,
> +	MKFS_CSUM_TREE,
> +};
> +
>   struct btrfs_mkfs_config {
>   	/* Label of the new filesystem */
>   	const char *label;
>

Re: [PATCH 1/9] btrfs-progs: use an associative array for init mkfs blocks

[David Sterba]

On Mon, Aug 23, 2021 at 04:42:24PM +0800, Qu Wenruo wrote:
> On 2021/8/21 上午3:11, Josef Bacik wrote:
> > Extent tree v2 will not create an extent tree or csum tree initially,
> > and it will create a block group tree.  To handle this we want to rework
> > the initial mkfs step to take an array of the blocks we want to create
> > and use the array to keep track of which blocks we need to create.
> >
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> >   mkfs/common.c | 56 ++++++++++++++++++++++++++++++++-------------------
> >   mkfs/common.h | 10 +++++++++
> >   2 files changed, 45 insertions(+), 21 deletions(-)
> >
> > diff --git a/mkfs/common.c b/mkfs/common.c
> > index 2c041224..e9ff529a 100644
> > --- a/mkfs/common.c
> > +++ b/mkfs/common.c
> > @@ -31,16 +31,18 @@
> >   #include "mkfs/common.h"
> >
> >   static u64 reference_root_table[] = {
> > -	[1] =	BTRFS_ROOT_TREE_OBJECTID,
> > -	[2] =	BTRFS_EXTENT_TREE_OBJECTID,
> > -	[3] =	BTRFS_CHUNK_TREE_OBJECTID,
> > -	[4] =	BTRFS_DEV_TREE_OBJECTID,
> > -	[5] =	BTRFS_FS_TREE_OBJECTID,
> > -	[6] =	BTRFS_CSUM_TREE_OBJECTID,
> > +	[MKFS_ROOT_TREE]	=	BTRFS_ROOT_TREE_OBJECTID,
> > +	[MKFS_EXTENT_TREE]	=	BTRFS_EXTENT_TREE_OBJECTID,
> > +	[MKFS_CHUNK_TREE]	=	BTRFS_CHUNK_TREE_OBJECTID,
> > +	[MKFS_DEV_TREE]		=	BTRFS_DEV_TREE_OBJECTID,
> > +	[MKFS_FS_TREE]		=	BTRFS_FS_TREE_OBJECTID,
> > +	[MKFS_CSUM_TREE]	=	BTRFS_CSUM_TREE_OBJECTID,
> >   };
> 
> Can we use a bitmap for these combinations?
> 
> So that we just need one parameter to indicate what trees we want.

The array can also define order, in followup patches there the free
space tree added to the end.

Re: [PATCH 1/9] btrfs-progs: use an associative array for init mkfs blocks

[Wang Yugui]

Hi, Josef Bacik, Qu Wenruo

This patchset will make a patch from Qu Wenruo will no longer work as
expected.

 From:    Qu Wenruo <wqu@suse.com>
 To:      linux-btrfs@vger.kernel.org
 Cc:      Chris Murphy <lists@colorremedies.com>
 Date:    Sat, 31 Jul 2021 15:42:40 +0800
 Subject: [PATCH] btrfs-progs: mkfs: set super_cache_generation to 0 if we're using free space tree

  # mkfs.btrfs -R free-space-tree test.img
  # btrfs inspect-internal dump-super test.img | grep cache_generation

0 is expected, but -1 is returned since this patchset.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2021/10/09

> Extent tree v2 will not create an extent tree or csum tree initially,
> and it will create a block group tree.  To handle this we want to rework
> the initial mkfs step to take an array of the blocks we want to create
> and use the array to keep track of which blocks we need to create.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  mkfs/common.c | 56 ++++++++++++++++++++++++++++++++-------------------
>  mkfs/common.h | 10 +++++++++
>  2 files changed, 45 insertions(+), 21 deletions(-)
> 
> diff --git a/mkfs/common.c b/mkfs/common.c
> index 2c041224..e9ff529a 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -31,16 +31,18 @@
>  #include "mkfs/common.h"
>  
>  static u64 reference_root_table[] = {
> -	[1] =	BTRFS_ROOT_TREE_OBJECTID,
> -	[2] =	BTRFS_EXTENT_TREE_OBJECTID,
> -	[3] =	BTRFS_CHUNK_TREE_OBJECTID,
> -	[4] =	BTRFS_DEV_TREE_OBJECTID,
> -	[5] =	BTRFS_FS_TREE_OBJECTID,
> -	[6] =	BTRFS_CSUM_TREE_OBJECTID,
> +	[MKFS_ROOT_TREE]	=	BTRFS_ROOT_TREE_OBJECTID,
> +	[MKFS_EXTENT_TREE]	=	BTRFS_EXTENT_TREE_OBJECTID,
> +	[MKFS_CHUNK_TREE]	=	BTRFS_CHUNK_TREE_OBJECTID,
> +	[MKFS_DEV_TREE]		=	BTRFS_DEV_TREE_OBJECTID,
> +	[MKFS_FS_TREE]		=	BTRFS_FS_TREE_OBJECTID,
> +	[MKFS_CSUM_TREE]	=	BTRFS_CSUM_TREE_OBJECTID,
>  };
>  
>  static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
> -			struct extent_buffer *buf)
> +				  struct extent_buffer *buf,
> +				  const enum btrfs_mkfs_block *blocks,
> +				  int blocks_nr)
>  {
>  	struct btrfs_root_item root_item;
>  	struct btrfs_inode_item *inode_item;
> @@ -49,6 +51,7 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>  	u32 itemoff;
>  	int ret = 0;
>  	int blk;
> +	int i;
>  	u8 uuid[BTRFS_UUID_SIZE];
>  
>  	memset(buf->data + sizeof(struct btrfs_header), 0,
> @@ -71,7 +74,8 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>  	btrfs_set_disk_key_offset(&disk_key, 0);
>  	itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize) - sizeof(root_item);
>  
> -	for (blk = 0; blk < MKFS_BLOCK_COUNT; blk++) {
> +	for (i = 0; i < blocks_nr; i++) {
> +		blk = blocks[i];
>  		if (blk == MKFS_SUPER_BLOCK || blk == MKFS_ROOT_TREE
>  		    || blk == MKFS_CHUNK_TREE)
>  			continue;
> @@ -145,10 +149,13 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  	struct btrfs_chunk *chunk;
>  	struct btrfs_dev_item *dev_item;
>  	struct btrfs_dev_extent *dev_extent;
> +	const enum btrfs_mkfs_block *blocks = extent_tree_v1_blocks;
>  	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
>  	u8 *ptr;
>  	int i;
>  	int ret;
> +	int blocks_nr = ARRAY_SIZE(extent_tree_v1_blocks);
> +	int blk;
>  	u32 itemoff;
>  	u32 nritems = 0;
>  	u64 first_free;
> @@ -195,8 +202,11 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  	uuid_generate(chunk_tree_uuid);
>  
>  	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
> -	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
> -		cfg->blocks[i] = system_group_offset + cfg->nodesize * (i - 1);
> +	for (i = 0; i < blocks_nr; i++) {
> +		blk = blocks[i];
> +		if (blk == MKFS_SUPER_BLOCK)
> +			continue;
> +		cfg->blocks[blk] = system_group_offset + cfg->nodesize * i;
>  	}
>  
>  	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
> @@ -236,7 +246,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  			    btrfs_header_chunk_tree_uuid(buf),
>  			    BTRFS_UUID_SIZE);
>  
> -	ret = btrfs_create_tree_root(fd, cfg, buf);
> +	ret = btrfs_create_tree_root(fd, cfg, buf, blocks, blocks_nr);
>  	if (ret < 0)
>  		goto out;
>  
> @@ -245,30 +255,34 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  		cfg->nodesize - sizeof(struct btrfs_header));
>  	nritems = 0;
>  	itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize);
> -	for (i = 1; i < MKFS_BLOCK_COUNT; i++) {
> +	for (i = 0; i < blocks_nr; i++) {
> +		blk = blocks[i];
> +		if (blk == MKFS_SUPER_BLOCK)
> +			continue;
> +
>  		item_size = sizeof(struct btrfs_extent_item);
>  		if (!skinny_metadata)
>  			item_size += sizeof(struct btrfs_tree_block_info);
>  
> -		if (cfg->blocks[i] < first_free) {
> +		if (cfg->blocks[blk] < first_free) {
>  			error("block[%d] below first free: %llu < %llu",
> -					i, (unsigned long long)cfg->blocks[i],
> +					i, (unsigned long long)cfg->blocks[blk],
>  					(unsigned long long)first_free);
>  			ret = -EINVAL;
>  			goto out;
>  		}
> -		if (cfg->blocks[i] < cfg->blocks[i - 1]) {
> +		if (cfg->blocks[blk] < cfg->blocks[blocks[i - 1]]) {
>  			error("blocks %d and %d in reverse order: %llu < %llu",
> -				i, i - 1,
> -				(unsigned long long)cfg->blocks[i],
> -				(unsigned long long)cfg->blocks[i - 1]);
> +				blk, blocks[i - 1],
> +				(unsigned long long)cfg->blocks[blk],
> +				(unsigned long long)cfg->blocks[blocks[i - 1]]);
>  			ret = -EINVAL;
>  			goto out;
>  		}
>  
>  		/* create extent item */
>  		itemoff -= item_size;
> -		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[i]);
> +		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[blk]);
>  		if (skinny_metadata) {
>  			btrfs_set_disk_key_type(&disk_key,
>  						BTRFS_METADATA_ITEM_KEY);
> @@ -292,8 +306,8 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>  		nritems++;
>  
>  		/* create extent ref */
> -		ref_root = reference_root_table[i];
> -		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[i]);
> +		ref_root = reference_root_table[blk];
> +		btrfs_set_disk_key_objectid(&disk_key, cfg->blocks[blk]);
>  		btrfs_set_disk_key_offset(&disk_key, ref_root);
>  		btrfs_set_disk_key_type(&disk_key, BTRFS_TREE_BLOCK_REF_KEY);
>  		btrfs_set_item_key(buf, &disk_key, nritems);
> diff --git a/mkfs/common.h b/mkfs/common.h
> index ea87c3ca..378da6bd 100644
> --- a/mkfs/common.h
> +++ b/mkfs/common.h
> @@ -55,6 +55,16 @@ enum btrfs_mkfs_block {
>  	MKFS_BLOCK_COUNT
>  };
>  
> +static const enum btrfs_mkfs_block extent_tree_v1_blocks[] = {
> +	MKFS_SUPER_BLOCK,
> +	MKFS_ROOT_TREE,
> +	MKFS_EXTENT_TREE,
> +	MKFS_CHUNK_TREE,
> +	MKFS_DEV_TREE,
> +	MKFS_FS_TREE,
> +	MKFS_CSUM_TREE,
> +};
> +
>  struct btrfs_mkfs_config {
>  	/* Label of the new filesystem */
>  	const char *label;
> -- 
> 2.26.3

Re: [PATCH 1/9] btrfs-progs: use an associative array for init mkfs blocks

[David Sterba]

On Sat, Oct 09, 2021 at 02:34:03PM +0800, Wang Yugui wrote:
> Hi, Josef Bacik, Qu Wenruo
> 
> This patchset will make a patch from Qu Wenruo will no longer work as
> expected.
> 
>  From:    Qu Wenruo <wqu@suse.com>
>  To:      linux-btrfs@vger.kernel.org
>  Cc:      Chris Murphy <lists@colorremedies.com>
>  Date:    Sat, 31 Jul 2021 15:42:40 +0800
>  Subject: [PATCH] btrfs-progs: mkfs: set super_cache_generation to 0 if we're using free space tree
> 
>   # mkfs.btrfs -R free-space-tree test.img
>   # btrfs inspect-internal dump-super test.img | grep cache_generation
> 
> 0 is expected, but -1 is returned since this patchset.

Thanks for the report, now tracked as
https://github.com/kdave/btrfs-progs/issues/414 .

[PATCH 2/9] btrfs-progs: use blocks_nr to determine the super used bytes

[Josef Bacik]

We were setting the superblock's used bytes to a static number.  However
the number of blocks we have to write has the correct used size, so just
add up the total number of blocks we're allocating as we determine their
offsets.  This value will be used later which is why I'm calculating it
this way instead of doing the math to set the bytes_super specifically.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 mkfs/common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index e9ff529a..8902d39e 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -162,6 +162,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	u64 ref_root;
 	u32 array_size;
 	u32 item_size;
+	u64 total_used = 0;
 	int skinny_metadata = !!(cfg->features &
 				 BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA);
 	u64 num_bytes;
@@ -207,6 +208,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 		if (blk == MKFS_SUPER_BLOCK)
 			continue;
 		cfg->blocks[blk] = system_group_offset + cfg->nodesize * i;
+		total_used += cfg->nodesize;
 	}
 
 	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
@@ -216,7 +218,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	btrfs_set_super_root(&super, cfg->blocks[MKFS_ROOT_TREE]);
 	btrfs_set_super_chunk_root(&super, cfg->blocks[MKFS_CHUNK_TREE]);
 	btrfs_set_super_total_bytes(&super, num_bytes);
-	btrfs_set_super_bytes_used(&super, 6 * cfg->nodesize);
+	btrfs_set_super_bytes_used(&super, total_used);
 	btrfs_set_super_sectorsize(&super, cfg->sectorsize);
 	super.__unused_leafsize = cpu_to_le32(cfg->nodesize);
 	btrfs_set_super_nodesize(&super, cfg->nodesize);
-- 
2.26.3

Re: [PATCH 2/9] btrfs-progs: use blocks_nr to determine the super used bytes

[Qu Wenruo]

On 2021/8/21 上午3:11, Josef Bacik wrote:
> We were setting the superblock's used bytes to a static number.  However
> the number of blocks we have to write has the correct used size, so just
> add up the total number of blocks we're allocating as we determine their
> offsets.  This value will be used later which is why I'm calculating it
> this way instead of doing the math to set the bytes_super specifically.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Getting rid of an hardcoded immediate number is always a good thing.

Thanks,
Qu
> ---
>   mkfs/common.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mkfs/common.c b/mkfs/common.c
> index e9ff529a..8902d39e 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -162,6 +162,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	u64 ref_root;
>   	u32 array_size;
>   	u32 item_size;
> +	u64 total_used = 0;
>   	int skinny_metadata = !!(cfg->features &
>   				 BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA);
>   	u64 num_bytes;
> @@ -207,6 +208,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   		if (blk == MKFS_SUPER_BLOCK)
>   			continue;
>   		cfg->blocks[blk] = system_group_offset + cfg->nodesize * i;
> +		total_used += cfg->nodesize;
>   	}
>
>   	btrfs_set_super_bytenr(&super, cfg->blocks[MKFS_SUPER_BLOCK]);
> @@ -216,7 +218,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	btrfs_set_super_root(&super, cfg->blocks[MKFS_ROOT_TREE]);
>   	btrfs_set_super_chunk_root(&super, cfg->blocks[MKFS_CHUNK_TREE]);
>   	btrfs_set_super_total_bytes(&super, num_bytes);
> -	btrfs_set_super_bytes_used(&super, 6 * cfg->nodesize);
> +	btrfs_set_super_bytes_used(&super, total_used);
>   	btrfs_set_super_sectorsize(&super, cfg->sectorsize);
>   	super.__unused_leafsize = cpu_to_le32(cfg->nodesize);
>   	btrfs_set_super_nodesize(&super, cfg->nodesize);
>

[PATCH 3/9] btrfs-progs: allocate blocks from the start of the temp system chunk

[Josef Bacik]

During mkfs we skip allocating a block for the super block, however
because we're using the blocks array iterator to determine the offset of
our block we're leaving a hole at the beginning of the temporary chunk.
This isn't a problem per-se, but I'm going to start generating the free
space tree at make_btrfs() time and having this hole makes the free
space tree creation slightly more complicated.

Instead keep track of which block we're on so that we start from the
actual offset of the system chunk.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 mkfs/common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index 8902d39e..0e747301 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -153,6 +153,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
 	u8 *ptr;
 	int i;
+	int cnt;
 	int ret;
 	int blocks_nr = ARRAY_SIZE(extent_tree_v1_blocks);
 	int blk;
@@ -203,11 +204,11 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	uuid_generate(chunk_tree_uuid);
 
 	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
-	for (i = 0; i < blocks_nr; i++) {
+	for (cnt = 0, i = 0; i < blocks_nr; i++) {
 		blk = blocks[i];
 		if (blk == MKFS_SUPER_BLOCK)
 			continue;
-		cfg->blocks[blk] = system_group_offset + cfg->nodesize * i;
+		cfg->blocks[blk] = system_group_offset + cfg->nodesize * cnt++;
 		total_used += cfg->nodesize;
 	}
 
-- 
2.26.3

Re: [PATCH 3/9] btrfs-progs: allocate blocks from the start of the temp system chunk

[Qu Wenruo]

On 2021/8/21 上午3:11, Josef Bacik wrote:
> During mkfs we skip allocating a block for the super block, however
> because we're using the blocks array iterator to determine the offset of
> our block we're leaving a hole at the beginning of the temporary chunk.
> This isn't a problem per-se, but I'm going to start generating the free
> space tree at make_btrfs() time and having this hole makes the free
> space tree creation slightly more complicated.
>
> Instead keep track of which block we're on so that we start from the
> actual offset of the system chunk.

Can't we just get rid the extra block for superblock?

As the superblock is always located at a fixed location, we don't really
need to put it into the blocks[] array.

Thanks,
Qu
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   mkfs/common.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mkfs/common.c b/mkfs/common.c
> index 8902d39e..0e747301 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -153,6 +153,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
>   	u8 *ptr;
>   	int i;
> +	int cnt;
>   	int ret;
>   	int blocks_nr = ARRAY_SIZE(extent_tree_v1_blocks);
>   	int blk;
> @@ -203,11 +204,11 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	uuid_generate(chunk_tree_uuid);
>
>   	cfg->blocks[MKFS_SUPER_BLOCK] = BTRFS_SUPER_INFO_OFFSET;
> -	for (i = 0; i < blocks_nr; i++) {
> +	for (cnt = 0, i = 0; i < blocks_nr; i++) {
>   		blk = blocks[i];
>   		if (blk == MKFS_SUPER_BLOCK)
>   			continue;
> -		cfg->blocks[blk] = system_group_offset + cfg->nodesize * i;
> +		cfg->blocks[blk] = system_group_offset + cfg->nodesize * cnt++;
>   		total_used += cfg->nodesize;
>   	}
>
>

[PATCH 4/9] btrfs-progs: set nritems based on root items written

[Josef Bacik]

For the root tree we were just hard setting the nritems to 4, which will
change when we move to extent tree v2.  Instead set the nritems after
we've added all the root items we need to the root tree.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 mkfs/common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index 0e747301..29fc8f12 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -107,6 +107,8 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 		itemoff -= sizeof(root_item);
 	}
 
+	btrfs_set_header_nritems(buf, nritems);
+
 	/* generate checksum */
 	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
 			     cfg->csum_type);
@@ -238,7 +240,6 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	memset(buf->data, 0, cfg->nodesize);
 	buf->len = cfg->nodesize;
 	btrfs_set_header_bytenr(buf, cfg->blocks[MKFS_ROOT_TREE]);
-	btrfs_set_header_nritems(buf, 4);
 	btrfs_set_header_generation(buf, 1);
 	btrfs_set_header_backref_rev(buf, BTRFS_MIXED_BACKREF_REV);
 	btrfs_set_header_owner(buf, BTRFS_ROOT_TREE_OBJECTID);
-- 
2.26.3

Re: [PATCH 4/9] btrfs-progs: set nritems based on root items written

[Qu Wenruo]

On 2021/8/21 上午3:11, Josef Bacik wrote:
> For the root tree we were just hard setting the nritems to 4, which will
> change when we move to extent tree v2.  Instead set the nritems after
> we've added all the root items we need to the root tree.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   mkfs/common.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mkfs/common.c b/mkfs/common.c
> index 0e747301..29fc8f12 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -107,6 +107,8 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>   		itemoff -= sizeof(root_item);
>   	}
>
> +	btrfs_set_header_nritems(buf, nritems);
> +
>   	/* generate checksum */
>   	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
>   			     cfg->csum_type);
> @@ -238,7 +240,6 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	memset(buf->data, 0, cfg->nodesize);
>   	buf->len = cfg->nodesize;
>   	btrfs_set_header_bytenr(buf, cfg->blocks[MKFS_ROOT_TREE]);
> -	btrfs_set_header_nritems(buf, 4);
>   	btrfs_set_header_generation(buf, 1);
>   	btrfs_set_header_backref_rev(buf, BTRFS_MIXED_BACKREF_REV);
>   	btrfs_set_header_owner(buf, BTRFS_ROOT_TREE_OBJECTID);
>

[PATCH 5/9] btrfs-progs: add helper for writing empty tree nodes

[Josef Bacik]

With extent tree v2 we're going to be writing some more empty trees for
the initial mkfs step, so take this common code and make it a helper.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 mkfs/common.c | 47 +++++++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index 29fc8f12..9263965e 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -39,6 +39,25 @@ static u64 reference_root_table[] = {
 	[MKFS_CSUM_TREE]	=	BTRFS_CSUM_TREE_OBJECTID,
 };
 
+static int btrfs_write_empty_tree(int fd, struct btrfs_mkfs_config *cfg,
+				  struct extent_buffer *buf, u64 objectid,
+				  u64 block)
+{
+	int ret;
+
+	memset(buf->data + sizeof(struct btrfs_header), 0,
+		cfg->nodesize - sizeof(struct btrfs_header));
+	btrfs_set_header_bytenr(buf, block);
+	btrfs_set_header_owner(buf, objectid);
+	btrfs_set_header_nritems(buf, 0);
+	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
+			     cfg->csum_type);
+	ret = pwrite(fd, buf->data, cfg->nodesize, block);
+	if (ret != cfg->nodesize)
+		return ret < 0 ? -errno : -EIO;
+	return 0;
+}
+
 static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 				  struct extent_buffer *buf,
 				  const enum btrfs_mkfs_block *blocks,
@@ -460,31 +479,15 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	}
 
 	/* create the FS root */
-	memset(buf->data + sizeof(struct btrfs_header), 0,
-		cfg->nodesize - sizeof(struct btrfs_header));
-	btrfs_set_header_bytenr(buf, cfg->blocks[MKFS_FS_TREE]);
-	btrfs_set_header_owner(buf, BTRFS_FS_TREE_OBJECTID);
-	btrfs_set_header_nritems(buf, 0);
-	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
-			     cfg->csum_type);
-	ret = pwrite(fd, buf->data, cfg->nodesize, cfg->blocks[MKFS_FS_TREE]);
-	if (ret != cfg->nodesize) {
-		ret = (ret < 0 ? -errno : -EIO);
+	ret = btrfs_write_empty_tree(fd, cfg, buf, BTRFS_FS_TREE_OBJECTID,
+				     cfg->blocks[MKFS_FS_TREE]);
+	if (ret)
 		goto out;
-	}
 	/* finally create the csum root */
-	memset(buf->data + sizeof(struct btrfs_header), 0,
-		cfg->nodesize - sizeof(struct btrfs_header));
-	btrfs_set_header_bytenr(buf, cfg->blocks[MKFS_CSUM_TREE]);
-	btrfs_set_header_owner(buf, BTRFS_CSUM_TREE_OBJECTID);
-	btrfs_set_header_nritems(buf, 0);
-	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
-			     cfg->csum_type);
-	ret = pwrite(fd, buf->data, cfg->nodesize, cfg->blocks[MKFS_CSUM_TREE]);
-	if (ret != cfg->nodesize) {
-		ret = (ret < 0 ? -errno : -EIO);
+	ret = btrfs_write_empty_tree(fd, cfg, buf, BTRFS_CSUM_TREE_OBJECTID,
+				     cfg->blocks[MKFS_CSUM_TREE]);
+	if (ret)
 		goto out;
-	}
 
 	/* and write out the super block */
 	memset(buf->data, 0, BTRFS_SUPER_INFO_SIZE);
-- 
2.26.3

Re: [PATCH 5/9] btrfs-progs: add helper for writing empty tree nodes

[Qu Wenruo]

On 2021/8/21 上午3:11, Josef Bacik wrote:
> With extent tree v2 we're going to be writing some more empty trees for
> the initial mkfs step, so take this common code and make it a helper.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Just a small nitpick on the function naming inliend below.

> ---
>   mkfs/common.c | 47 +++++++++++++++++++++++++----------------------
>   1 file changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/mkfs/common.c b/mkfs/common.c
> index 29fc8f12..9263965e 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -39,6 +39,25 @@ static u64 reference_root_table[] = {
>   	[MKFS_CSUM_TREE]	=	BTRFS_CSUM_TREE_OBJECTID,
>   };
>
> +static int btrfs_write_empty_tree(int fd, struct btrfs_mkfs_config *cfg,
> +				  struct extent_buffer *buf, u64 objectid,
> +				  u64 block)

Can't we replace the btrfs prefix?

This function is specific to mkfs, and it only works for the initial
btrfs creation.

Thanks,
Qu

> +{
> +	int ret;
> +
> +	memset(buf->data + sizeof(struct btrfs_header), 0,
> +		cfg->nodesize - sizeof(struct btrfs_header));
> +	btrfs_set_header_bytenr(buf, block);
> +	btrfs_set_header_owner(buf, objectid);
> +	btrfs_set_header_nritems(buf, 0);
> +	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
> +			     cfg->csum_type);
> +	ret = pwrite(fd, buf->data, cfg->nodesize, block);
> +	if (ret != cfg->nodesize)
> +		return ret < 0 ? -errno : -EIO;
> +	return 0;
> +}
> +
>   static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>   				  struct extent_buffer *buf,
>   				  const enum btrfs_mkfs_block *blocks,
> @@ -460,31 +479,15 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	}
>
>   	/* create the FS root */
> -	memset(buf->data + sizeof(struct btrfs_header), 0,
> -		cfg->nodesize - sizeof(struct btrfs_header));
> -	btrfs_set_header_bytenr(buf, cfg->blocks[MKFS_FS_TREE]);
> -	btrfs_set_header_owner(buf, BTRFS_FS_TREE_OBJECTID);
> -	btrfs_set_header_nritems(buf, 0);
> -	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
> -			     cfg->csum_type);
> -	ret = pwrite(fd, buf->data, cfg->nodesize, cfg->blocks[MKFS_FS_TREE]);
> -	if (ret != cfg->nodesize) {
> -		ret = (ret < 0 ? -errno : -EIO);
> +	ret = btrfs_write_empty_tree(fd, cfg, buf, BTRFS_FS_TREE_OBJECTID,
> +				     cfg->blocks[MKFS_FS_TREE]);
> +	if (ret)
>   		goto out;
> -	}
>   	/* finally create the csum root */
> -	memset(buf->data + sizeof(struct btrfs_header), 0,
> -		cfg->nodesize - sizeof(struct btrfs_header));
> -	btrfs_set_header_bytenr(buf, cfg->blocks[MKFS_CSUM_TREE]);
> -	btrfs_set_header_owner(buf, BTRFS_CSUM_TREE_OBJECTID);
> -	btrfs_set_header_nritems(buf, 0);
> -	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
> -			     cfg->csum_type);
> -	ret = pwrite(fd, buf->data, cfg->nodesize, cfg->blocks[MKFS_CSUM_TREE]);
> -	if (ret != cfg->nodesize) {
> -		ret = (ret < 0 ? -errno : -EIO);
> +	ret = btrfs_write_empty_tree(fd, cfg, buf, BTRFS_CSUM_TREE_OBJECTID,
> +				     cfg->blocks[MKFS_CSUM_TREE]);
> +	if (ret)
>   		goto out;
> -	}
>
>   	/* and write out the super block */
>   	memset(buf->data, 0, BTRFS_SUPER_INFO_SIZE);
>

[PATCH 6/9] btrfs-progs: add the block group item in make_btrfs()

[Josef Bacik]

Currently we build a bare-bones file system in make_btrfs(), and then we
load it up and fill in the rest of the file system after the fact.  One
thing we omit in make_btrfs() is the block group item for the temporary
system chunk we allocate, because we just add it after we've opened the
file system.

However I want to be able to generate the free space tree at
make_btrfs() time, because extent tree v2 will not have an extent tree
that has every block allocated in the system.  In order to do this I
need to make sure that the free space tree entries are added on block
group creation, which is annoying if we have to add this chunk after
I've created a free space tree.

So make future work simpler by simply adding our block group item at
make_btrfs() time, this way I can do the right things with the free
space tree in the generic make block group code without needing a
special case for our temporary system chunk.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 mkfs/common.c | 31 +++++++++++++++++++++++++++++++
 mkfs/main.c   |  9 ++-------
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index 9263965e..cba97687 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -190,6 +190,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	u64 num_bytes;
 	u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
 	u64 system_group_size = BTRFS_MKFS_SYSTEM_GROUP_SIZE;
+	bool add_block_group = true;
 
 	if ((cfg->features & BTRFS_FEATURE_INCOMPAT_ZONED)) {
 		system_group_offset = cfg->zone_size * BTRFS_NR_SB_LOG_ZONES;
@@ -283,6 +284,36 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 		if (blk == MKFS_SUPER_BLOCK)
 			continue;
 
+		/* Add the block group item for our temporary chunk. */
+		if (cfg->blocks[blk] > system_group_offset &&
+		    add_block_group) {
+			struct btrfs_block_group_item *bg_item;
+
+			add_block_group = false;
+
+			itemoff -= sizeof(*bg_item);
+			btrfs_set_disk_key_objectid(&disk_key,
+						    system_group_offset);
+			btrfs_set_disk_key_offset(&disk_key,
+						  system_group_size);
+			btrfs_set_disk_key_type(&disk_key,
+						BTRFS_BLOCK_GROUP_ITEM_KEY);
+			btrfs_set_item_key(buf, &disk_key, nritems);
+			btrfs_set_item_offset(buf, btrfs_item_nr(nritems),
+					      itemoff);
+			btrfs_set_item_size(buf, btrfs_item_nr(nritems),
+					    sizeof(*bg_item));
+
+			bg_item = btrfs_item_ptr(buf, nritems,
+						 struct btrfs_block_group_item);
+			btrfs_set_block_group_used(buf, bg_item, total_used);
+			btrfs_set_block_group_flags(buf, bg_item,
+						    BTRFS_BLOCK_GROUP_SYSTEM);
+			btrfs_set_block_group_chunk_objectid(buf, bg_item,
+					BTRFS_FIRST_CHUNK_TREE_OBJECTID);
+			nritems++;
+		}
+
 		item_size = sizeof(struct btrfs_extent_item);
 		if (!skinny_metadata)
 			item_size += sizeof(struct btrfs_tree_block_info);
diff --git a/mkfs/main.c b/mkfs/main.c
index eab93eb3..ea53e9c7 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -67,7 +67,6 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 	struct btrfs_trans_handle *trans;
 	struct btrfs_space_info *sinfo;
 	u64 flags = BTRFS_BLOCK_GROUP_METADATA;
-	u64 bytes_used;
 	u64 chunk_start = 0;
 	u64 chunk_size = 0;
 	u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
@@ -90,16 +89,12 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 
 	trans = btrfs_start_transaction(root, 1);
 	BUG_ON(IS_ERR(trans));
-	bytes_used = btrfs_super_bytes_used(fs_info->super_copy);
 
 	root->fs_info->system_allocs = 1;
 	/*
-	 * First temporary system chunk must match the chunk layout
-	 * created in make_btrfs().
+	 * We already created the block group item for our temporary system
+	 * chunk in make_btrfs(), so account for the size here.
 	 */
-	ret = btrfs_make_block_group(trans, fs_info, bytes_used,
-				     BTRFS_BLOCK_GROUP_SYSTEM,
-				     system_group_offset, system_group_size);
 	allocation->system += system_group_size;
 	if (ret)
 		return ret;
-- 
2.26.3

Re: [PATCH 6/9] btrfs-progs: add the block group item in make_btrfs()

[Qu Wenruo]

On 2021/8/21 上午3:11, Josef Bacik wrote:
> Currently we build a bare-bones file system in make_btrfs(), and then we
> load it up and fill in the rest of the file system after the fact.  One
> thing we omit in make_btrfs() is the block group item for the temporary
> system chunk we allocate, because we just add it after we've opened the
> file system.
>
> However I want to be able to generate the free space tree at
> make_btrfs() time, because extent tree v2 will not have an extent tree
> that has every block allocated in the system.  In order to do this I
> need to make sure that the free space tree entries are added on block
> group creation, which is annoying if we have to add this chunk after
> I've created a free space tree.
>
> So make future work simpler by simply adding our block group item at
> make_btrfs() time, this way I can do the right things with the free
> space tree in the generic make block group code without needing a
> special case for our temporary system chunk.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   mkfs/common.c | 31 +++++++++++++++++++++++++++++++
>   mkfs/main.c   |  9 ++-------
>   2 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/mkfs/common.c b/mkfs/common.c
> index 9263965e..cba97687 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -190,6 +190,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   	u64 num_bytes;
>   	u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
>   	u64 system_group_size = BTRFS_MKFS_SYSTEM_GROUP_SIZE;
> +	bool add_block_group = true;
>
>   	if ((cfg->features & BTRFS_FEATURE_INCOMPAT_ZONED)) {
>   		system_group_offset = cfg->zone_size * BTRFS_NR_SB_LOG_ZONES;
> @@ -283,6 +284,36 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>   		if (blk == MKFS_SUPER_BLOCK)
>   			continue;
>
> +		/* Add the block group item for our temporary chunk. */
> +		if (cfg->blocks[blk] > system_group_offset &&
> +		    add_block_group) {

This makes the block group item always be the first item.

But for skinny metadata, METADATA_ITEM is smaller than BLOCK_GROUP_ITEM,
meaning it should be before BLOCK_GROUP_ITEM.

Won't this cause the extent tree has a bad key order?


BTW, if we get rid of the superblock in cfg->blocks[], we don't need to
check against system_group_offset.

Thanks,
Qu
> +			struct btrfs_block_group_item *bg_item;
> +
> +			add_block_group = false;
> +
> +			itemoff -= sizeof(*bg_item);
> +			btrfs_set_disk_key_objectid(&disk_key,
> +						    system_group_offset);
> +			btrfs_set_disk_key_offset(&disk_key,
> +						  system_group_size);
> +			btrfs_set_disk_key_type(&disk_key,
> +						BTRFS_BLOCK_GROUP_ITEM_KEY);
> +			btrfs_set_item_key(buf, &disk_key, nritems);
> +			btrfs_set_item_offset(buf, btrfs_item_nr(nritems),
> +					      itemoff);
> +			btrfs_set_item_size(buf, btrfs_item_nr(nritems),
> +					    sizeof(*bg_item));
> +
> +			bg_item = btrfs_item_ptr(buf, nritems,
> +						 struct btrfs_block_group_item);
> +			btrfs_set_block_group_used(buf, bg_item, total_used);
> +			btrfs_set_block_group_flags(buf, bg_item,
> +						    BTRFS_BLOCK_GROUP_SYSTEM);
> +			btrfs_set_block_group_chunk_objectid(buf, bg_item,
> +					BTRFS_FIRST_CHUNK_TREE_OBJECTID);
> +			nritems++;
> +		}
> +
>   		item_size = sizeof(struct btrfs_extent_item);
>   		if (!skinny_metadata)
>   			item_size += sizeof(struct btrfs_tree_block_info);
> diff --git a/mkfs/main.c b/mkfs/main.c
> index eab93eb3..ea53e9c7 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -67,7 +67,6 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
>   	struct btrfs_trans_handle *trans;
>   	struct btrfs_space_info *sinfo;
>   	u64 flags = BTRFS_BLOCK_GROUP_METADATA;
> -	u64 bytes_used;
>   	u64 chunk_start = 0;
>   	u64 chunk_size = 0;
>   	u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
> @@ -90,16 +89,12 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
>
>   	trans = btrfs_start_transaction(root, 1);
>   	BUG_ON(IS_ERR(trans));
> -	bytes_used = btrfs_super_bytes_used(fs_info->super_copy);
>
>   	root->fs_info->system_allocs = 1;
>   	/*
> -	 * First temporary system chunk must match the chunk layout
> -	 * created in make_btrfs().
> +	 * We already created the block group item for our temporary system
> +	 * chunk in make_btrfs(), so account for the size here.
>   	 */
> -	ret = btrfs_make_block_group(trans, fs_info, bytes_used,
> -				     BTRFS_BLOCK_GROUP_SYSTEM,
> -				     system_group_offset, system_group_size);
>   	allocation->system += system_group_size;
>   	if (ret)
>   		return ret;
>

Re: [PATCH 6/9] btrfs-progs: add the block group item in make_btrfs()

[Josef Bacik]

On 8/23/21 5:00 AM, Qu Wenruo wrote:
> 
> 
> On 2021/8/21 上午3:11, Josef Bacik wrote:
>> Currently we build a bare-bones file system in make_btrfs(), and then we
>> load it up and fill in the rest of the file system after the fact.  One
>> thing we omit in make_btrfs() is the block group item for the temporary
>> system chunk we allocate, because we just add it after we've opened the
>> file system.
>>
>> However I want to be able to generate the free space tree at
>> make_btrfs() time, because extent tree v2 will not have an extent tree
>> that has every block allocated in the system.  In order to do this I
>> need to make sure that the free space tree entries are added on block
>> group creation, which is annoying if we have to add this chunk after
>> I've created a free space tree.
>>
>> So make future work simpler by simply adding our block group item at
>> make_btrfs() time, this way I can do the right things with the free
>> space tree in the generic make block group code without needing a
>> special case for our temporary system chunk.
>>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> ---
>>   mkfs/common.c | 31 +++++++++++++++++++++++++++++++
>>   mkfs/main.c   |  9 ++-------
>>   2 files changed, 33 insertions(+), 7 deletions(-)
>>
>> diff --git a/mkfs/common.c b/mkfs/common.c
>> index 9263965e..cba97687 100644
>> --- a/mkfs/common.c
>> +++ b/mkfs/common.c
>> @@ -190,6 +190,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
>>       u64 num_bytes;
>>       u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
>>       u64 system_group_size = BTRFS_MKFS_SYSTEM_GROUP_SIZE;
>> +    bool add_block_group = true;
>>
>>       if ((cfg->features & BTRFS_FEATURE_INCOMPAT_ZONED)) {
>>           system_group_offset = cfg->zone_size * BTRFS_NR_SB_LOG_ZONES;
>> @@ -283,6 +284,36 @@ int make_btrfs(int fd, struct btrfs_mkfs_config 
>> *cfg)
>>           if (blk == MKFS_SUPER_BLOCK)
>>               continue;
>>
>> +        /* Add the block group item for our temporary chunk. */
>> +        if (cfg->blocks[blk] > system_group_offset &&
>> +            add_block_group) {
> 
> This makes the block group item always be the first item.
> 
> But for skinny metadata, METADATA_ITEM is smaller than BLOCK_GROUP_ITEM,
> meaning it should be before BLOCK_GROUP_ITEM.
> 
> Won't this cause the extent tree has a bad key order?
> 

No because it's based on the actual bytenr.  We'll insert the extent 
item entry first, and then move to the next block and if it's past the 
first block we add the block group item, and then the actual extent 
item.  So it goes

first block X gets extent item inserted
X+1 > X, insert block group item
insert X+1 extent item.

Thanks,

Josef

Re: [PATCH 6/9] btrfs-progs: add the block group item in make_btrfs()

[Qu Wenruo]

On 2021/8/24 上午4:04, Josef Bacik wrote:
> On 8/23/21 5:00 AM, Qu Wenruo wrote:
>>
>>
>> On 2021/8/21 上午3:11, Josef Bacik wrote:
>>> Currently we build a bare-bones file system in make_btrfs(), and then we
>>> load it up and fill in the rest of the file system after the fact.  One
>>> thing we omit in make_btrfs() is the block group item for the temporary
>>> system chunk we allocate, because we just add it after we've opened the
>>> file system.
>>>
>>> However I want to be able to generate the free space tree at
>>> make_btrfs() time, because extent tree v2 will not have an extent tree
>>> that has every block allocated in the system.  In order to do this I
>>> need to make sure that the free space tree entries are added on block
>>> group creation, which is annoying if we have to add this chunk after
>>> I've created a free space tree.
>>>
>>> So make future work simpler by simply adding our block group item at
>>> make_btrfs() time, this way I can do the right things with the free
>>> space tree in the generic make block group code without needing a
>>> special case for our temporary system chunk.
>>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> ---
>>>   mkfs/common.c | 31 +++++++++++++++++++++++++++++++
>>>   mkfs/main.c   |  9 ++-------
>>>   2 files changed, 33 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/mkfs/common.c b/mkfs/common.c
>>> index 9263965e..cba97687 100644
>>> --- a/mkfs/common.c
>>> +++ b/mkfs/common.c
>>> @@ -190,6 +190,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config
>>> *cfg)
>>>       u64 num_bytes;
>>>       u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
>>>       u64 system_group_size = BTRFS_MKFS_SYSTEM_GROUP_SIZE;
>>> +    bool add_block_group = true;
>>>
>>>       if ((cfg->features & BTRFS_FEATURE_INCOMPAT_ZONED)) {
>>>           system_group_offset = cfg->zone_size * BTRFS_NR_SB_LOG_ZONES;
>>> @@ -283,6 +284,36 @@ int make_btrfs(int fd, struct btrfs_mkfs_config
>>> *cfg)
>>>           if (blk == MKFS_SUPER_BLOCK)
>>>               continue;
>>>
>>> +        /* Add the block group item for our temporary chunk. */
>>> +        if (cfg->blocks[blk] > system_group_offset &&
>>> +            add_block_group) {
>>
>> This makes the block group item always be the first item.
>>
>> But for skinny metadata, METADATA_ITEM is smaller than BLOCK_GROUP_ITEM,
>> meaning it should be before BLOCK_GROUP_ITEM.
>>
>> Won't this cause the extent tree has a bad key order?
>>
>
> No because it's based on the actual bytenr.  We'll insert the extent
> item entry first, and then move to the next block and if it's past the
> first block we add the block group item, and then the actual extent
> item.  So it goes
>
> first block X gets extent item inserted
> X+1 > X, insert block group item
> insert X+1 extent item.
>

But then what if we go non-skinny metadata?

Then block group item is always before any extent item.

Thanks,
Qu

> Thanks,
>
> Josef

Re: [PATCH 6/9] btrfs-progs: add the block group item in make_btrfs()

[Josef Bacik]

On 8/23/21 7:37 PM, Qu Wenruo wrote:
> 
> 
> On 2021/8/24 上午4:04, Josef Bacik wrote:
>> On 8/23/21 5:00 AM, Qu Wenruo wrote:
>>>
>>>
>>> On 2021/8/21 上午3:11, Josef Bacik wrote:
>>>> Currently we build a bare-bones file system in make_btrfs(), and 
>>>> then we
>>>> load it up and fill in the rest of the file system after the fact.  One
>>>> thing we omit in make_btrfs() is the block group item for the temporary
>>>> system chunk we allocate, because we just add it after we've opened the
>>>> file system.
>>>>
>>>> However I want to be able to generate the free space tree at
>>>> make_btrfs() time, because extent tree v2 will not have an extent tree
>>>> that has every block allocated in the system.  In order to do this I
>>>> need to make sure that the free space tree entries are added on block
>>>> group creation, which is annoying if we have to add this chunk after
>>>> I've created a free space tree.
>>>>
>>>> So make future work simpler by simply adding our block group item at
>>>> make_btrfs() time, this way I can do the right things with the free
>>>> space tree in the generic make block group code without needing a
>>>> special case for our temporary system chunk.
>>>>
>>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>>> ---
>>>>   mkfs/common.c | 31 +++++++++++++++++++++++++++++++
>>>>   mkfs/main.c   |  9 ++-------
>>>>   2 files changed, 33 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/mkfs/common.c b/mkfs/common.c
>>>> index 9263965e..cba97687 100644
>>>> --- a/mkfs/common.c
>>>> +++ b/mkfs/common.c
>>>> @@ -190,6 +190,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config
>>>> *cfg)
>>>>       u64 num_bytes;
>>>>       u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
>>>>       u64 system_group_size = BTRFS_MKFS_SYSTEM_GROUP_SIZE;
>>>> +    bool add_block_group = true;
>>>>
>>>>       if ((cfg->features & BTRFS_FEATURE_INCOMPAT_ZONED)) {
>>>>           system_group_offset = cfg->zone_size * BTRFS_NR_SB_LOG_ZONES;
>>>> @@ -283,6 +284,36 @@ int make_btrfs(int fd, struct btrfs_mkfs_config
>>>> *cfg)
>>>>           if (blk == MKFS_SUPER_BLOCK)
>>>>               continue;
>>>>
>>>> +        /* Add the block group item for our temporary chunk. */
>>>> +        if (cfg->blocks[blk] > system_group_offset &&
>>>> +            add_block_group) {
>>>
>>> This makes the block group item always be the first item.
>>>
>>> But for skinny metadata, METADATA_ITEM is smaller than BLOCK_GROUP_ITEM,
>>> meaning it should be before BLOCK_GROUP_ITEM.
>>>
>>> Won't this cause the extent tree has a bad key order?
>>>
>>
>> No because it's based on the actual bytenr.  We'll insert the extent
>> item entry first, and then move to the next block and if it's past the
>> first block we add the block group item, and then the actual extent
>> item.  So it goes
>>
>> first block X gets extent item inserted
>> X+1 > X, insert block group item
>> insert X+1 extent item.
>>
> 
> But then what if we go non-skinny metadata?
> 
> Then block group item is always before any extent item.
> 

         item 0 key (1048576 METADATA_ITEM 0) itemoff 16259 itemsize 24
                 refs 1 gen 1 flags TREE_BLOCK
                 tree block skinny level 0
         item 1 key (1048576 TREE_BLOCK_REF 1) itemoff 16259 itemsize 0
                 tree block backref
         item 2 key (1048576 BLOCK_GROUP_ITEM 4194304) itemoff 16235 
itemsize 24
                 block group used 98304 chunk_objectid 256 flags SYSTEM
         item 3 key (1064960 METADATA_ITEM 0) itemoff 16211 itemsize 24
                 refs 1 gen 1 flags TREE_BLOCK
                 tree block skinny level 0


This is what it looks like.  We're basing it off of the key.objectid. 
If the key.objectid's match, which they will, the block group will 
always be after it all.  It's doing the right thing.  Thanks,

Josef

Re: [PATCH 6/9] btrfs-progs: add the block group item in make_btrfs()

[Qu Wenruo]

On 2021/8/24 上午7:47, Josef Bacik wrote:
> On 8/23/21 7:37 PM, Qu Wenruo wrote:
>>
>>
>> On 2021/8/24 上午4:04, Josef Bacik wrote:
>>> On 8/23/21 5:00 AM, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2021/8/21 上午3:11, Josef Bacik wrote:
>>>>> Currently we build a bare-bones file system in make_btrfs(), and
>>>>> then we
>>>>> load it up and fill in the rest of the file system after the fact.
>>>>> One
>>>>> thing we omit in make_btrfs() is the block group item for the
>>>>> temporary
>>>>> system chunk we allocate, because we just add it after we've opened
>>>>> the
>>>>> file system.
>>>>>
>>>>> However I want to be able to generate the free space tree at
>>>>> make_btrfs() time, because extent tree v2 will not have an extent tree
>>>>> that has every block allocated in the system.  In order to do this I
>>>>> need to make sure that the free space tree entries are added on block
>>>>> group creation, which is annoying if we have to add this chunk after
>>>>> I've created a free space tree.
>>>>>
>>>>> So make future work simpler by simply adding our block group item at
>>>>> make_btrfs() time, this way I can do the right things with the free
>>>>> space tree in the generic make block group code without needing a
>>>>> special case for our temporary system chunk.
>>>>>
>>>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>>>> ---
>>>>>   mkfs/common.c | 31 +++++++++++++++++++++++++++++++
>>>>>   mkfs/main.c   |  9 ++-------
>>>>>   2 files changed, 33 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/mkfs/common.c b/mkfs/common.c
>>>>> index 9263965e..cba97687 100644
>>>>> --- a/mkfs/common.c
>>>>> +++ b/mkfs/common.c
>>>>> @@ -190,6 +190,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config
>>>>> *cfg)
>>>>>       u64 num_bytes;
>>>>>       u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
>>>>>       u64 system_group_size = BTRFS_MKFS_SYSTEM_GROUP_SIZE;
>>>>> +    bool add_block_group = true;
>>>>>
>>>>>       if ((cfg->features & BTRFS_FEATURE_INCOMPAT_ZONED)) {
>>>>>           system_group_offset = cfg->zone_size *
>>>>> BTRFS_NR_SB_LOG_ZONES;
>>>>> @@ -283,6 +284,36 @@ int make_btrfs(int fd, struct btrfs_mkfs_config
>>>>> *cfg)
>>>>>           if (blk == MKFS_SUPER_BLOCK)
>>>>>               continue;
>>>>>
>>>>> +        /* Add the block group item for our temporary chunk. */
>>>>> +        if (cfg->blocks[blk] > system_group_offset &&
>>>>> +            add_block_group) {
>>>>
>>>> This makes the block group item always be the first item.
>>>>
>>>> But for skinny metadata, METADATA_ITEM is smaller than
>>>> BLOCK_GROUP_ITEM,
>>>> meaning it should be before BLOCK_GROUP_ITEM.
>>>>
>>>> Won't this cause the extent tree has a bad key order?
>>>>
>>>
>>> No because it's based on the actual bytenr.  We'll insert the extent
>>> item entry first, and then move to the next block and if it's past the
>>> first block we add the block group item, and then the actual extent
>>> item.  So it goes
>>>
>>> first block X gets extent item inserted
>>> X+1 > X, insert block group item
>>> insert X+1 extent item.
>>>
>>
>> But then what if we go non-skinny metadata?
>>
>> Then block group item is always before any extent item.
>>
>
>          item 0 key (1048576 METADATA_ITEM 0) itemoff 16259 itemsize 24
>                  refs 1 gen 1 flags TREE_BLOCK
>                  tree block skinny level 0
>          item 1 key (1048576 TREE_BLOCK_REF 1) itemoff 16259 itemsize 0
>                  tree block backref
>          item 2 key (1048576 BLOCK_GROUP_ITEM 4194304) itemoff 16235
> itemsize 24
>                  block group used 98304 chunk_objectid 256 flags SYSTEM
>          item 3 key (1064960 METADATA_ITEM 0) itemoff 16211 itemsize 24
>                  refs 1 gen 1 flags TREE_BLOCK
>                  tree block skinny level 0
>
>
> This is what it looks like.  We're basing it off of the key.objectid. If
> the key.objectid's match, which they will, the block group will always
> be after it all.  It's doing the right thing.  Thanks,

Oh my bad memory.

Both EXTENT_ITEM and METADATA_ITEM are smaller than BLOCK_GROUP_ITEM.

So these extent items should always be before BLOCK_GROUP_ITEM, no
matter if the skinny metadata is spcififed.

Then the code is completely fine.

Thanks,
Qu
>
> Josef

[PATCH 7/9] btrfs-progs: add add_block_group_free_space helper

[Josef Bacik]

This exists in the kernel free-space-tree.c but not in progs.  We need
it to generate the free space items for new block groups, which is
needed when we start creating the free space tree in make_btrfs().

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel-shared/extent-tree.c     |  2 ++
 kernel-shared/free-space-tree.c | 26 ++++++++++++++++++++++++++
 kernel-shared/free-space-tree.h |  2 ++
 3 files changed, 30 insertions(+)

diff --git a/kernel-shared/extent-tree.c b/kernel-shared/extent-tree.c
index 056a9256..b5b43c10 100644
--- a/kernel-shared/extent-tree.c
+++ b/kernel-shared/extent-tree.c
@@ -2816,6 +2816,8 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans,
 	ret = btrfs_insert_item(trans, extent_root, &key, &bgi, sizeof(bgi));
 	BUG_ON(ret);
 
+	add_block_group_free_space(trans, cache);
+
 	return 0;
 }
 
diff --git a/kernel-shared/free-space-tree.c b/kernel-shared/free-space-tree.c
index 2edc7fc7..397311c0 100644
--- a/kernel-shared/free-space-tree.c
+++ b/kernel-shared/free-space-tree.c
@@ -986,6 +986,32 @@ out:
 	return ret;
 }
 
+int add_block_group_free_space(struct btrfs_trans_handle *trans,
+			       struct btrfs_block_group *block_group)
+{
+	struct btrfs_path *path;
+	int ret;
+
+	if (!btrfs_fs_compat_ro(trans->fs_info, FREE_SPACE_TREE))
+		return 0;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	ret = add_new_free_space_info(trans, block_group, path);
+	if (ret)
+		goto out;
+	ret = __add_to_free_space_tree(trans, block_group, path,
+				       block_group->start,
+				       block_group->length);
+out:
+	btrfs_free_path(path);
+	if (ret)
+		btrfs_abort_transaction(trans, ret);
+	return ret;
+}
+
 int populate_free_space_tree(struct btrfs_trans_handle *trans,
 			     struct btrfs_block_group *block_group)
 {
diff --git a/kernel-shared/free-space-tree.h b/kernel-shared/free-space-tree.h
index 3d32e167..4f6aa5fc 100644
--- a/kernel-shared/free-space-tree.h
+++ b/kernel-shared/free-space-tree.h
@@ -34,5 +34,7 @@ int add_to_free_space_tree(struct btrfs_trans_handle *trans, u64 start,
 int remove_from_free_space_tree(struct btrfs_trans_handle *trans, u64 start,
 				u64 size);
 int btrfs_create_free_space_tree(struct btrfs_fs_info *info);
+int add_block_group_free_space(struct btrfs_trans_handle *trans,
+			       struct btrfs_block_group *block_group);
 
 #endif
-- 
2.26.3

[PATCH 8/9] btrfs-progs: generate free space tree at make_btrfs() time

[Josef Bacik]

With extent-tree-v2 we won't be able to cache block groups based on the
extent tree, so we need to have a valid free space tree before we open
the temporary file system to finish setting the file system up.  Set up
the basic free space entries for our temporary system chunk if we have
the free space tree enabled and stop generating the tree after the fact.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 mkfs/common.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++
 mkfs/common.h |  9 +++++++
 mkfs/main.c   |  9 +------
 3 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index cba97687..cf91f4d2 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -37,6 +37,7 @@ static u64 reference_root_table[] = {
 	[MKFS_DEV_TREE]		=	BTRFS_DEV_TREE_OBJECTID,
 	[MKFS_FS_TREE]		=	BTRFS_FS_TREE_OBJECTID,
 	[MKFS_CSUM_TREE]	=	BTRFS_CSUM_TREE_OBJECTID,
+	[MKFS_FREE_SPACE_TREE]	=	BTRFS_FREE_SPACE_TREE_OBJECTID,
 };
 
 static int btrfs_write_empty_tree(int fd, struct btrfs_mkfs_config *cfg,
@@ -140,6 +141,55 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 	return ret;
 }
 
+static int create_free_space_tree(int fd, struct btrfs_mkfs_config *cfg,
+				  struct extent_buffer *buf, u64 group_start,
+				  u64 group_size, u64 free_start)
+{
+	struct btrfs_free_space_info *info;
+	struct btrfs_disk_key disk_key;
+	enum btrfs_mkfs_block blk;
+	int itemoff = __BTRFS_LEAF_DATA_SIZE(cfg->nodesize);
+	int nritems = 0;
+	int i = 0;
+	int ret;
+
+	memset(buf->data + sizeof(struct btrfs_header), 0,
+	       cfg->nodesize - sizeof(struct btrfs_header));
+	itemoff -= sizeof(*info);
+
+	btrfs_set_disk_key_objectid(&disk_key, group_start);
+	btrfs_set_disk_key_offset(&disk_key, group_size);
+	btrfs_set_disk_key_type(&disk_key, BTRFS_FREE_SPACE_INFO_KEY);
+	btrfs_set_item_key(buf, &disk_key, nritems);
+	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
+	btrfs_set_item_size(buf, btrfs_item_nr(nritems), sizeof(*info));
+
+	info = btrfs_item_ptr(buf, nritems, struct btrfs_free_space_info);
+	btrfs_set_free_space_extent_count(buf, info, 1);
+	btrfs_set_free_space_flags(buf, info, 0);
+
+	nritems++;
+	btrfs_set_disk_key_objectid(&disk_key, free_start);
+	btrfs_set_disk_key_offset(&disk_key, group_start + group_size -
+				  free_start);
+	btrfs_set_disk_key_type(&disk_key, BTRFS_FREE_SPACE_EXTENT_KEY);
+	btrfs_set_item_key(buf, &disk_key, nritems);
+	btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
+	btrfs_set_item_size(buf, btrfs_item_nr(nritems), 0);
+
+	nritems++;
+	btrfs_set_header_bytenr(buf, cfg->blocks[MKFS_FREE_SPACE_TREE]);
+	btrfs_set_header_owner(buf, BTRFS_FREE_SPACE_TREE_OBJECTID);
+	btrfs_set_header_nritems(buf, nritems);
+	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
+			     cfg->csum_type);
+	ret = pwrite(fd, buf->data, cfg->nodesize,
+		     cfg->blocks[MKFS_FREE_SPACE_TREE]);
+	if (ret != cfg->nodesize)
+		return ret < 0 ? -errno : -EIO;
+	return 0;
+}
+
 /*
  * @fs_uuid - if NULL, generates a UUID, returns back the new filesystem UUID
  *
@@ -191,6 +241,12 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	u64 system_group_offset = BTRFS_BLOCK_RESERVED_1M_FOR_SUPER;
 	u64 system_group_size = BTRFS_MKFS_SYSTEM_GROUP_SIZE;
 	bool add_block_group = true;
+	bool free_space_tree = !!(cfg->runtime_features &
+				  BTRFS_RUNTIME_FEATURE_FREE_SPACE_TREE);
+
+	/* Don't include the free space tree in the blocks to process. */
+	if (!free_space_tree)
+		blocks_nr--;
 
 	if ((cfg->features & BTRFS_FEATURE_INCOMPAT_ZONED)) {
 		system_group_offset = cfg->zone_size * BTRFS_NR_SB_LOG_ZONES;
@@ -253,6 +309,11 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	else
 		btrfs_set_super_cache_generation(&super, -1);
 	btrfs_set_super_incompat_flags(&super, cfg->features);
+	if (free_space_tree) {
+		u64 ro_flags = BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE |
+			BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID;
+		btrfs_set_super_compat_ro_flags(&super, ro_flags);
+	}
 	if (cfg->label)
 		__strncpy_null(super.label, cfg->label, BTRFS_LABEL_SIZE - 1);
 
@@ -520,6 +581,14 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	if (ret)
 		goto out;
 
+	if (free_space_tree) {
+		ret = create_free_space_tree(fd, cfg, buf, system_group_offset,
+					     system_group_size,
+					     system_group_offset + total_used);
+		if (ret)
+			goto out;
+	}
+
 	/* and write out the super block */
 	memset(buf->data, 0, BTRFS_SUPER_INFO_SIZE);
 	memcpy(buf->data, &super, sizeof(super));
diff --git a/mkfs/common.h b/mkfs/common.h
index 378da6bd..e9cc62dd 100644
--- a/mkfs/common.h
+++ b/mkfs/common.h
@@ -52,6 +52,7 @@ enum btrfs_mkfs_block {
 	MKFS_DEV_TREE,
 	MKFS_FS_TREE,
 	MKFS_CSUM_TREE,
+	MKFS_FREE_SPACE_TREE,
 	MKFS_BLOCK_COUNT
 };
 
@@ -63,6 +64,12 @@ static const enum btrfs_mkfs_block extent_tree_v1_blocks[] = {
 	MKFS_DEV_TREE,
 	MKFS_FS_TREE,
 	MKFS_CSUM_TREE,
+
+	/*
+	 * Since the free space tree is optional with v1 it must always be last
+	 * in this array.
+	 */
+	MKFS_FREE_SPACE_TREE,
 };
 
 struct btrfs_mkfs_config {
@@ -74,6 +81,8 @@ struct btrfs_mkfs_config {
 	u32 stripesize;
 	/* Bitfield of incompat features, BTRFS_FEATURE_INCOMPAT_* */
 	u64 features;
+	/* Bitfield of BTRFS_RUNTIME_FEATURE_* */
+	u64 runtime_features;
 	/* Size of the filesystem in bytes */
 	u64 num_bytes;
 	/* checksum algorithm to use */
diff --git a/mkfs/main.c b/mkfs/main.c
index ea53e9c7..bf09a963 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -137,7 +137,6 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 
 	root->fs_info->system_allocs = 0;
 	ret = btrfs_commit_transaction(trans, root);
-
 err:
 	return ret;
 }
@@ -1366,6 +1365,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 	mkfs_cfg.sectorsize = sectorsize;
 	mkfs_cfg.stripesize = stripesize;
 	mkfs_cfg.features = features;
+	mkfs_cfg.runtime_features = runtime_features;
 	mkfs_cfg.csum_type = csum_type;
 	mkfs_cfg.zone_size = zone_size(file);
 
@@ -1529,13 +1529,6 @@ raid_groups:
 			goto out;
 		}
 	}
-	if (runtime_features & BTRFS_RUNTIME_FEATURE_FREE_SPACE_TREE) {
-		ret = btrfs_create_free_space_tree(fs_info);
-		if (ret < 0) {
-			error("failed to create free space tree: %d (%m)", ret);
-			goto out;
-		}
-	}
 	if (verbose) {
 		char features_buf[64];
 
-- 
2.26.3

[PATCH 9/9] btrfs-progs: add the incompat flag for extent tree v2

[Josef Bacik]

I will have a lot of preparatory patches to reduce the review pain of
this large feature.  In order to enable that work define the incompat
flag.  Once all of the work lands to support the feature there will be a
patch to actually enable us to select it and manipulate file systems
with that incompat flag set.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel-shared/ctree.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
index 3cca6032..fb918aba 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -496,6 +496,7 @@ struct btrfs_super_block {
 #define BTRFS_FEATURE_INCOMPAT_METADATA_UUID    (1ULL << 10)
 #define BTRFS_FEATURE_INCOMPAT_RAID1C34		(1ULL << 11)
 #define BTRFS_FEATURE_INCOMPAT_ZONED		(1ULL << 12)
+#define BTRFS_FEATURE_INCOMPAT_EXTENT_TREE_V2	(1ULL << 13)
 
 #define BTRFS_FEATURE_COMPAT_SUPP		0ULL
 
-- 
2.26.3