[PATCH 0/4] btrfs-progs: zoned: fix mkfs failure on various zone size

[PATCH 0/4] btrfs-progs: zoned: fix mkfs failure on various zone size

[Naohiro Aota]

There are mkfs.btrfs failures on various zone sizes. For example, with 32
MB zone size, mkfs.btrfs creates the initial system block group at 64 MB,
which collides with the regular superblock location. This series addresses
the issues.

Patches 1 and 2 fix the location of the initial system block group when the
zone size is 32 MB.

Patch 3 fixes the location of the initial data block group when the zone
size is 16 MB.

Patch 4 fixes a bug revealed by patch 3.

Naohiro Aota (4):
  btrfs-progs: zoned: export sb_zone_number() and related constants
  btrfs-progs: zoned: fix initial system BG location
  btrfs-progs: fix ordering of hole_size setting and
    dev_extent_hole_check()
  btrfs-progs: zoned: fix and simplify dev_extent_hole_check_zoned()

 kernel-shared/volumes.c | 32 ++++++++++----------------------
 kernel-shared/zoned.c   | 33 ---------------------------------
 kernel-shared/zoned.h   | 33 +++++++++++++++++++++++++++++++++
 mkfs/common.c           | 30 +++++++++++++++++++++++++++++-
 4 files changed, 72 insertions(+), 56 deletions(-)

-- 
2.35.1

[PATCH 1/4] btrfs-progs: zoned: export sb_zone_number() and related constants

[Naohiro Aota]

Move sb_zone_number() and related constants from zoned.c to the
corresponding header for later use.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/zoned.c | 33 ---------------------------------
 kernel-shared/zoned.h | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/kernel-shared/zoned.c b/kernel-shared/zoned.c
index 2a11a1d723aa..396b74f0d906 100644
--- a/kernel-shared/zoned.c
+++ b/kernel-shared/zoned.c
@@ -33,20 +33,6 @@
 /* Pseudo write pointer value for conventional zone */
 #define WP_CONVENTIONAL			((u64)-2)
 
-/*
- * Location of the first zone of superblock logging zone pairs.
- *
- * - primary superblock:    0B (zone 0)
- * - first copy:          512G (zone starting at that offset)
- * - second copy:           4T (zone starting at that offset)
- */
-#define BTRFS_SB_LOG_PRIMARY_OFFSET	(0ULL)
-#define BTRFS_SB_LOG_FIRST_OFFSET	(512ULL * SZ_1G)
-#define BTRFS_SB_LOG_SECOND_OFFSET	(4096ULL * SZ_1G)
-
-#define BTRFS_SB_LOG_FIRST_SHIFT	const_ilog2(BTRFS_SB_LOG_FIRST_OFFSET)
-#define BTRFS_SB_LOG_SECOND_SHIFT	const_ilog2(BTRFS_SB_LOG_SECOND_OFFSET)
-
 #define EMULATED_ZONE_SIZE		SZ_256M
 
 static int btrfs_get_dev_zone_info(struct btrfs_device *device);
@@ -220,25 +206,6 @@ static int sb_write_pointer(int fd, struct blk_zone *zones, u64 *wp_ret)
 	return 0;
 }
 
-/*
- * Get the first zone number of the superblock mirror
- */
-static inline u32 sb_zone_number(int shift, int mirror)
-{
-	u64 zone = 0;
-
-	ASSERT(0 <= mirror && mirror < BTRFS_SUPER_MIRROR_MAX);
-	switch (mirror) {
-	case 0: zone = 0; break;
-	case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break;
-	case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break;
-	}
-
-	ASSERT(zone <= U32_MAX);
-
-	return (u32)zone;
-}
-
 int btrfs_reset_dev_zone(int fd, struct blk_zone *zone)
 {
 	struct blk_zone_range range;
diff --git a/kernel-shared/zoned.h b/kernel-shared/zoned.h
index 75327610e537..cc0d6b6f166d 100644
--- a/kernel-shared/zoned.h
+++ b/kernel-shared/zoned.h
@@ -36,6 +36,20 @@ struct blk_zone {
 /* Number of superblock log zones */
 #define BTRFS_NR_SB_LOG_ZONES		2
 
+/*
+ * Location of the first zone of superblock logging zone pairs.
+ *
+ * - primary superblock:    0B (zone 0)
+ * - first copy:          512G (zone starting at that offset)
+ * - second copy:           4T (zone starting at that offset)
+ */
+#define BTRFS_SB_LOG_PRIMARY_OFFSET	(0ULL)
+#define BTRFS_SB_LOG_FIRST_OFFSET	(512ULL * SZ_1G)
+#define BTRFS_SB_LOG_SECOND_OFFSET	(4096ULL * SZ_1G)
+
+#define BTRFS_SB_LOG_FIRST_SHIFT	const_ilog2(BTRFS_SB_LOG_FIRST_OFFSET)
+#define BTRFS_SB_LOG_SECOND_SHIFT	const_ilog2(BTRFS_SB_LOG_SECOND_OFFSET)
+
 /*
  * Zoned block device models
  */
@@ -206,6 +220,25 @@ static inline bool zoned_profile_supported(u64 map_type)
 
 #endif /* BTRFS_ZONED */
 
+/*
+ * Get the first zone number of the superblock mirror
+ */
+static inline u32 sb_zone_number(int shift, int mirror)
+{
+	u64 zone = 0;
+
+	ASSERT(0 <= mirror && mirror < BTRFS_SUPER_MIRROR_MAX);
+	switch (mirror) {
+	case 0: zone = 0; break;
+	case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break;
+	case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break;
+	}
+
+	ASSERT(zone <= U32_MAX);
+
+	return (u32)zone;
+}
+
 static inline bool btrfs_dev_is_sequential(struct btrfs_device *device, u64 pos)
 {
 	return zone_is_sequential(device->zone_info, pos);
-- 
2.35.1

[PATCH 2/4] btrfs-progs: zoned: fix initial system BG location

[Naohiro Aota]

Currently, we create the initial system block group in the zone 2. That
will create the BG at 64MB when the zone size is 32 MB, which collides with
the regular superblock location. It results in mount failure with:

  BTRFS info (device nullb0): zoned mode enabled with zone size 33554432
  BTRFS error (device nullb0): zoned: block group 67108864 must not contain super block
  BTRFS error (device nullb0): failed to read block groups: -117
  BTRFS error (device nullb0): open_ctree failed

Fix that by calculating the proper location of the initial system BG. It
avoids using zones reserved for zoned superblock logging and the zones
where a regular superblock resides.

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

diff --git a/mkfs/common.c b/mkfs/common.c
index 75680d032d30..218854491c14 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -232,6 +232,34 @@ static int create_block_group_tree(int fd, struct btrfs_mkfs_config *cfg,
 	return 0;
 }
 
+static u64 zoned_system_group_offset(u64 zone_size)
+{
+	const int zone_shift = ilog2(zone_size);
+	u32 zone_num = BTRFS_NR_SB_LOG_ZONES;
+	u64 start = (u64)zone_num * zone_size;
+	u32 sb_zones[BTRFS_SUPER_MIRROR_MAX];
+	int i;
+
+	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++)
+		sb_zones[i] = sb_zone_number(zone_shift, i);
+
+	for (;;) {
+		for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
+			if (zone_num == sb_zones[i] ||
+			    !(btrfs_sb_offset(i) + BTRFS_SUPER_INFO_SIZE <= start ||
+			      start + zone_size <= btrfs_sb_offset(i)))
+				goto next;
+		}
+
+		return start;
+next:
+		zone_num++;
+		start += zone_size;
+	}
+
+	__builtin_unreachable();
+}
+
 /*
  * @fs_uuid - if NULL, generates a UUID, returns back the new filesystem UUID
  *
@@ -298,7 +326,7 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	}
 
 	if ((cfg->features & BTRFS_FEATURE_INCOMPAT_ZONED)) {
-		system_group_offset = cfg->zone_size * BTRFS_NR_SB_LOG_ZONES;
+		system_group_offset = zoned_system_group_offset(cfg->zone_size);
 		system_group_size = cfg->zone_size;
 	}
 
-- 
2.35.1

[PATCH 3/4] btrfs-progs: fix ordering of hole_size setting and dev_extent_hole_check()

[Naohiro Aota]

The hole_size is used by dev_extent_hole_check() to check the hole is OK as
a device extent. However, commit b031fe84fda8 ("btrfs-progs: zoned:
implement zoned chunk allocator") mis-ported the kernel code and placed
dev_extent_hole_check() before setting hole_check. That made the
dev_extent_hole_check() call here essentially pass through as we have
hole_size == 0 on mkfs time.

As a result, mkfs.btrfs creates data BG at 64 MB where the regular
superblock exists, when zone size is 16 MB.

Fix the ordering of hole_size setting and calling dev_extent_hole_check().

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index e24428db8412..0199bc26a8b4 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -774,14 +774,13 @@ next:
 	 * search_end may be smaller than search_start.
 	 */
 	if (search_end > search_start) {
+		hole_size = search_end - search_start;
 		if (dev_extent_hole_check(device, &search_start, &hole_size,
 					  num_bytes)) {
 			btrfs_release_path(path);
 			goto again;
 		}
 
-		hole_size = search_end - search_start;
-
 		if (hole_size > max_hole_size) {
 			max_hole_start = search_start;
 			max_hole_size = hole_size;
-- 
2.35.1

[PATCH 4/4] btrfs-progs: zoned: fix and simplify dev_extent_hole_check_zoned()

[Naohiro Aota]

The previous patch revealed a bug in dev_extent_hole_check_zoned(). If the
given hole is OK to use as is, it should have just returned the hole. But
on the contrary, it shifts the hole start position by one zone. That
results in refusing any hole.

We don't use btrfs_ensure_empty_zones() in the btrfs-progs version of
dev_extent_hole_check_zoned() unlike the kernel side, because
btrfs_find_allocatable_zones() itself is doing the necessary checks. So, we
can just "return changed" if the "pos" is unchanged. That also makes the
loop and "changed" variable unnecessary.

So, fix and simplify the code in one shot.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 kernel-shared/volumes.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 0199bc26a8b4..598ac553442c 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -583,30 +583,19 @@ static bool dev_extent_hole_check_zoned(struct btrfs_device *device,
 					u64 *hole_start, u64 *hole_size,
 					u64 num_bytes)
 {
-	u64 zone_size = device->zone_info->zone_size;
 	u64 pos;
-	bool changed = false;
-
-	ASSERT(IS_ALIGNED(*hole_start, zone_size));
-
-	while (*hole_size > 0) {
-		pos = btrfs_find_allocatable_zones(device, *hole_start,
-						   *hole_start + *hole_size,
-						   num_bytes);
-		if (pos != *hole_start) {
-			*hole_size = *hole_start + *hole_size - pos;
-			*hole_start = pos;
-			changed = true;
-			if (*hole_size < num_bytes)
-				break;
-		}
 
-		*hole_start += zone_size;
-		*hole_size -= zone_size;
-		changed = true;
+	ASSERT(IS_ALIGNED(*hole_start, device->zone_info->zone_size));
+
+	pos = btrfs_find_allocatable_zones(device, *hole_start,
+					   *hole_start + *hole_size, num_bytes);
+	if (pos != *hole_start) {
+		*hole_size = *hole_start + *hole_size - pos;
+		*hole_start = pos;
+		return true;
 	}
 
-	return changed;
+	return false;
 }
 
 /**
-- 
2.35.1

Re: [PATCH 1/4] btrfs-progs: zoned: export sb_zone_number() and related constants

[David Sterba]

On Wed, Apr 06, 2022 at 10:43:10AM +0900, Naohiro Aota wrote:
> Move sb_zone_number() and related constants from zoned.c to the
> corresponding header for later use.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  kernel-shared/zoned.c | 33 ---------------------------------
>  kernel-shared/zoned.h | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/kernel-shared/zoned.c b/kernel-shared/zoned.c
> index 2a11a1d723aa..396b74f0d906 100644
> --- a/kernel-shared/zoned.c
> +++ b/kernel-shared/zoned.c
> @@ -33,20 +33,6 @@
>  /* Pseudo write pointer value for conventional zone */
>  #define WP_CONVENTIONAL			((u64)-2)
>  
> -/*
> - * Location of the first zone of superblock logging zone pairs.
> - *
> - * - primary superblock:    0B (zone 0)
> - * - first copy:          512G (zone starting at that offset)
> - * - second copy:           4T (zone starting at that offset)
> - */
> -#define BTRFS_SB_LOG_PRIMARY_OFFSET	(0ULL)
> -#define BTRFS_SB_LOG_FIRST_OFFSET	(512ULL * SZ_1G)
> -#define BTRFS_SB_LOG_SECOND_OFFSET	(4096ULL * SZ_1G)
> -
> -#define BTRFS_SB_LOG_FIRST_SHIFT	const_ilog2(BTRFS_SB_LOG_FIRST_OFFSET)
> -#define BTRFS_SB_LOG_SECOND_SHIFT	const_ilog2(BTRFS_SB_LOG_SECOND_OFFSET)
> -
>  #define EMULATED_ZONE_SIZE		SZ_256M
>  
>  static int btrfs_get_dev_zone_info(struct btrfs_device *device);
> @@ -220,25 +206,6 @@ static int sb_write_pointer(int fd, struct blk_zone *zones, u64 *wp_ret)
>  	return 0;
>  }
>  
> -/*
> - * Get the first zone number of the superblock mirror
> - */
> -static inline u32 sb_zone_number(int shift, int mirror)
> -{
> -	u64 zone = 0;
> -
> -	ASSERT(0 <= mirror && mirror < BTRFS_SUPER_MIRROR_MAX);
> -	switch (mirror) {
> -	case 0: zone = 0; break;
> -	case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break;
> -	case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break;
> -	}
> -
> -	ASSERT(zone <= U32_MAX);
> -
> -	return (u32)zone;
> -}
> -
>  int btrfs_reset_dev_zone(int fd, struct blk_zone *zone)
>  {
>  	struct blk_zone_range range;
> diff --git a/kernel-shared/zoned.h b/kernel-shared/zoned.h
> index 75327610e537..cc0d6b6f166d 100644
> --- a/kernel-shared/zoned.h
> +++ b/kernel-shared/zoned.h
> @@ -36,6 +36,20 @@ struct blk_zone {
>  /* Number of superblock log zones */
>  #define BTRFS_NR_SB_LOG_ZONES		2
>  
> +/*
> + * Location of the first zone of superblock logging zone pairs.
> + *
> + * - primary superblock:    0B (zone 0)
> + * - first copy:          512G (zone starting at that offset)
> + * - second copy:           4T (zone starting at that offset)
> + */
> +#define BTRFS_SB_LOG_PRIMARY_OFFSET	(0ULL)
> +#define BTRFS_SB_LOG_FIRST_OFFSET	(512ULL * SZ_1G)
> +#define BTRFS_SB_LOG_SECOND_OFFSET	(4096ULL * SZ_1G)
> +
> +#define BTRFS_SB_LOG_FIRST_SHIFT	const_ilog2(BTRFS_SB_LOG_FIRST_OFFSET)
> +#define BTRFS_SB_LOG_SECOND_SHIFT	const_ilog2(BTRFS_SB_LOG_SECOND_OFFSET)
> +
>  /*
>   * Zoned block device models
>   */
> @@ -206,6 +220,25 @@ static inline bool zoned_profile_supported(u64 map_type)
>  
>  #endif /* BTRFS_ZONED */
>  
> +/*
> + * Get the first zone number of the superblock mirror
> + */
> +static inline u32 sb_zone_number(int shift, int mirror)
> +{

This does not need to be static inline but not a big deal.

> +	u64 zone = 0;
> +
> +	ASSERT(0 <= mirror && mirror < BTRFS_SUPER_MIRROR_MAX);
> +	switch (mirror) {
> +	case 0: zone = 0; break;
> +	case 1: zone = 1ULL << (BTRFS_SB_LOG_FIRST_SHIFT - shift); break;
> +	case 2: zone = 1ULL << (BTRFS_SB_LOG_SECOND_SHIFT - shift); break;
> +	}
> +
> +	ASSERT(zone <= U32_MAX);
> +
> +	return (u32)zone;
> +}
> +
>  static inline bool btrfs_dev_is_sequential(struct btrfs_device *device, u64 pos)
>  {
>  	return zone_is_sequential(device->zone_info, pos);
> -- 
> 2.35.1

Re: [PATCH 0/4] btrfs-progs: zoned: fix mkfs failure on various zone size

[David Sterba]

On Wed, Apr 06, 2022 at 10:43:09AM +0900, Naohiro Aota wrote:
> There are mkfs.btrfs failures on various zone sizes. For example, with 32
> MB zone size, mkfs.btrfs creates the initial system block group at 64 MB,
> which collides with the regular superblock location. This series addresses
> the issues.
> 
> Patches 1 and 2 fix the location of the initial system block group when the
> zone size is 32 MB.
> 
> Patch 3 fixes the location of the initial data block group when the zone
> size is 16 MB.
> 
> Patch 4 fixes a bug revealed by patch 3.
> 
> Naohiro Aota (4):
>   btrfs-progs: zoned: export sb_zone_number() and related constants
>   btrfs-progs: zoned: fix initial system BG location
>   btrfs-progs: fix ordering of hole_size setting and
>     dev_extent_hole_check()
>   btrfs-progs: zoned: fix and simplify dev_extent_hole_check_zoned()

Added to devel, thanks.