diff mbox series

[v2,4/8] btrfs-progs: mkfs: fix minimum size calculation for zoned mode

Message ID 20240514182227.1197664-5-naohiro.aota@wdc.com (mailing list archive)
State New
Headers show
Series btrfs-progs: zoned: proper "mkfs.btrfs -b" support | expand

Commit Message

Naohiro Aota May 14, 2024, 6:22 p.m. UTC
Currently, we check if a device is larger than 5 zones to determine we can
create btrfs on the device or not. Actually, we need more zones to create
DUP block groups, so it fails with "ERROR: not enough free space to
allocate chunk". Implement proper support for non-SINGLE profile.

Also, current code does not ensure we can create tree-log BG and data
relocation BG, which are essential for the real usage. Count them as
requirement too.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 mkfs/common.c | 53 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 8 deletions(-)

Comments

Qu Wenruo May 14, 2024, 10:54 p.m. UTC | #1
在 2024/5/15 03:52, Naohiro Aota 写道:
> Currently, we check if a device is larger than 5 zones to determine we can
> create btrfs on the device or not. Actually, we need more zones to create
> DUP block groups, so it fails with "ERROR: not enough free space to
> allocate chunk". Implement proper support for non-SINGLE profile.
>
> Also, current code does not ensure we can create tree-log BG and data
> relocation BG, which are essential for the real usage. Count them as
> requirement too.
>
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>   mkfs/common.c | 53 +++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/mkfs/common.c b/mkfs/common.c
> index af54089654a0..a5100b296f65 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -818,14 +818,51 @@ u64 btrfs_min_dev_size(u32 nodesize, bool mixed, u64 zone_size, u64 meta_profile
>   	u64 meta_size;
>   	u64 data_size;
>
> -	/*
> -	 * 2 zones for the primary superblock
> -	 * 1 zone for the system block group
> -	 * 1 zone for a metadata block group
> -	 * 1 zone for a data block group
> -	 */
> -	if (zone_size)
> -		return 5 * zone_size;
> +	if (zone_size) {
> +		/* 2 zones for the primary superblock. */
> +		reserved += 2 * zone_size;
> +
> +		/*
> +		 * 1 zone each for the initial system, metadata, and data block
> +		 * group
> +		 */
> +		reserved += 3 * zone_size;
> +
> +		/*
> +		 * non-SINGLE profile needs:
> +		 * 1 zone for system block group
> +		 * 1 zone for normal metadata block group
> +		 * 1 zone for tree-log block group
> +		 *
> +		 * SINGLE profile only need to add tree-log block group

This comments looks a little confusing to me.

As (for now) the non-SINGLE profiles for metadata is only DUP, thus they
needs at least 2 zones for each bg.

It's only explained later in the "meta_size *= 2;" line.

Would the following ones be a little better?

/*
  * non-SINGLE profile needs:
  * 1 extra system block group
  * 1 extra normal metadata block group
  * 1 extra tree-log block group
  *
  * SINGLE profiles needs:
  * 1 extra tree-log block group
  */
  if (meta_profiles & BTRFS_BLOCK_GROUP_DUP)
      factor = 2;
  if (meta_profiles & BTRFS_BLOCK_GROUP_PROFILE_MASK)
      meta_size = 3 * zone_size * factor;
  else
      meta_size = 1 * zone_size * factor;

Otherwise looks reasonable to me.

Thanks,
Qu
> +		 */
> +		if (meta_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
> +			meta_size = 3 * zone_size;
> +		else
> +			meta_size = zone_size;
> +		/* DUP profile needs two zones for each block group. */
> +		if (meta_profile & BTRFS_BLOCK_GROUP_DUP)
> +			meta_size *= 2;
> +		reserved += meta_size;
> +
> +		/*
> +		 * non-SINGLE profile needs:
> +		 * 1 zone for data block group
> +		 * 1 zone for data relocation block group
> +		 *
> +		 * SINGLE profile only need to add data relocationblock group
> +		 */
> +		if (data_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
> +			data_size = 2 * zone_size;
> +		else
> +			data_size = zone_size;
> +		/* DUP profile needs two zones for each block group. */
> +		if (data_profile & BTRFS_BLOCK_GROUP_DUP)
> +			data_size *= 2;
> +		reserved += data_size;
> +
> +		return reserved;
> +	}
>
>   	if (mixed)
>   		return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +
Naohiro Aota May 15, 2024, 4:25 p.m. UTC | #2
On Wed, May 15, 2024 at 08:24:46AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2024/5/15 03:52, Naohiro Aota 写道:
> > Currently, we check if a device is larger than 5 zones to determine we can
> > create btrfs on the device or not. Actually, we need more zones to create
> > DUP block groups, so it fails with "ERROR: not enough free space to
> > allocate chunk". Implement proper support for non-SINGLE profile.
> > 
> > Also, current code does not ensure we can create tree-log BG and data
> > relocation BG, which are essential for the real usage. Count them as
> > requirement too.
> > 
> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> > ---
> >   mkfs/common.c | 53 +++++++++++++++++++++++++++++++++++++++++++--------
> >   1 file changed, 45 insertions(+), 8 deletions(-)
> > 
> > diff --git a/mkfs/common.c b/mkfs/common.c
> > index af54089654a0..a5100b296f65 100644
> > --- a/mkfs/common.c
> > +++ b/mkfs/common.c
> > @@ -818,14 +818,51 @@ u64 btrfs_min_dev_size(u32 nodesize, bool mixed, u64 zone_size, u64 meta_profile
> >   	u64 meta_size;
> >   	u64 data_size;
> > 
> > -	/*
> > -	 * 2 zones for the primary superblock
> > -	 * 1 zone for the system block group
> > -	 * 1 zone for a metadata block group
> > -	 * 1 zone for a data block group
> > -	 */
> > -	if (zone_size)
> > -		return 5 * zone_size;
> > +	if (zone_size) {
> > +		/* 2 zones for the primary superblock. */
> > +		reserved += 2 * zone_size;
> > +
> > +		/*
> > +		 * 1 zone each for the initial system, metadata, and data block
> > +		 * group
> > +		 */
> > +		reserved += 3 * zone_size;
> > +
> > +		/*
> > +		 * non-SINGLE profile needs:
> > +		 * 1 zone for system block group
> > +		 * 1 zone for normal metadata block group
> > +		 * 1 zone for tree-log block group
> > +		 *
> > +		 * SINGLE profile only need to add tree-log block group
> 
> This comments looks a little confusing to me.
> 
> As (for now) the non-SINGLE profiles for metadata is only DUP, thus they
> needs at least 2 zones for each bg.

RAID is also supported in an experimetanl build ;-)

> It's only explained later in the "meta_size *= 2;" line.
> 
> Would the following ones be a little better?
> 
> /*
>  * non-SINGLE profile needs:
>  * 1 extra system block group
>  * 1 extra normal metadata block group
>  * 1 extra tree-log block group
>  *
>  * SINGLE profiles needs:
>  * 1 extra tree-log block group
>  */
>  if (meta_profiles & BTRFS_BLOCK_GROUP_DUP)
>      factor = 2;
>  if (meta_profiles & BTRFS_BLOCK_GROUP_PROFILE_MASK)
>      meta_size = 3 * zone_size * factor;
>  else
>      meta_size = 1 * zone_size * factor;
> 
> Otherwise looks reasonable to me.

I followed the regular case code, but this looks cleaner to me. I'll follow
your suggestion and tweak the comment as well.

> Thanks,
> Qu
> > +		 */
> > +		if (meta_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
> > +			meta_size = 3 * zone_size;
> > +		else
> > +			meta_size = zone_size;
> > +		/* DUP profile needs two zones for each block group. */
> > +		if (meta_profile & BTRFS_BLOCK_GROUP_DUP)
> > +			meta_size *= 2;
> > +		reserved += meta_size;
> > +
> > +		/*
> > +		 * non-SINGLE profile needs:
> > +		 * 1 zone for data block group
> > +		 * 1 zone for data relocation block group
> > +		 *
> > +		 * SINGLE profile only need to add data relocationblock group
> > +		 */
> > +		if (data_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
> > +			data_size = 2 * zone_size;
> > +		else
> > +			data_size = zone_size;
> > +		/* DUP profile needs two zones for each block group. */
> > +		if (data_profile & BTRFS_BLOCK_GROUP_DUP)
> > +			data_size *= 2;
> > +		reserved += data_size;
> > +
> > +		return reserved;
> > +	}
> > 
> >   	if (mixed)
> >   		return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +
diff mbox series

Patch

diff --git a/mkfs/common.c b/mkfs/common.c
index af54089654a0..a5100b296f65 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -818,14 +818,51 @@  u64 btrfs_min_dev_size(u32 nodesize, bool mixed, u64 zone_size, u64 meta_profile
 	u64 meta_size;
 	u64 data_size;
 
-	/*
-	 * 2 zones for the primary superblock
-	 * 1 zone for the system block group
-	 * 1 zone for a metadata block group
-	 * 1 zone for a data block group
-	 */
-	if (zone_size)
-		return 5 * zone_size;
+	if (zone_size) {
+		/* 2 zones for the primary superblock. */
+		reserved += 2 * zone_size;
+
+		/*
+		 * 1 zone each for the initial system, metadata, and data block
+		 * group
+		 */
+		reserved += 3 * zone_size;
+
+		/*
+		 * non-SINGLE profile needs:
+		 * 1 zone for system block group
+		 * 1 zone for normal metadata block group
+		 * 1 zone for tree-log block group
+		 *
+		 * SINGLE profile only need to add tree-log block group
+		 */
+		if (meta_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
+			meta_size = 3 * zone_size;
+		else
+			meta_size = zone_size;
+		/* DUP profile needs two zones for each block group. */
+		if (meta_profile & BTRFS_BLOCK_GROUP_DUP)
+			meta_size *= 2;
+		reserved += meta_size;
+
+		/*
+		 * non-SINGLE profile needs:
+		 * 1 zone for data block group
+		 * 1 zone for data relocation block group
+		 *
+		 * SINGLE profile only need to add data relocationblock group
+		 */
+		if (data_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
+			data_size = 2 * zone_size;
+		else
+			data_size = zone_size;
+		/* DUP profile needs two zones for each block group. */
+		if (data_profile & BTRFS_BLOCK_GROUP_DUP)
+			data_size *= 2;
+		reserved += data_size;
+
+		return reserved;
+	}
 
 	if (mixed)
 		return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +