[PATCH 0/3] Fixes for zoned mode

[PATCH 0/3] Fixes for zoned mode

[Naohiro Aota]

These are fixes for zoned btrfs.

Patch 01 moves the superblock location to address based. Patch 02 fixes
type conversion for zone size >= 4G. Patch 03 fixes zone_unusable acconting
when a block group is read-only.


Naohiro Aota (3):
  btrfs: zoned: move superblock logging zone location
  btrfs: zoned: add missing type conversion
  btrfs: zoned: do not account freed region of read-only block group as
    zone_unusable

 fs/btrfs/free-space-cache.c |  7 ++++++-
 fs/btrfs/zoned.c            | 23 ++++++++++++++++-------
 2 files changed, 22 insertions(+), 8 deletions(-)

-- 
2.30.1

[PATCH 1/3] btrfs: zoned: move superblock logging zone location

[Naohiro Aota]

This commit moves the location of superblock logging zones basing on the
static address instead of the static zone number.

The following zones are reserved as the circular buffer on zoned btrfs.
  - The primary superblock: zone at LBA 0 and the next zone
  - The first copy: zone at LBA 16G and the next zone
  - The second copy: zone at LBA 256G and the next zone

We disallow zone size larger than 8GB not to overlap the superblock log
zones.

Since the superblock zones overlap, we disallow zone size larger than 8GB.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/zoned.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 9a5cf153da89..40cb99854844 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -112,10 +112,9 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones,
 
 /*
  * The following zones are reserved as the circular buffer on ZONED btrfs.
- *  - The primary superblock: zones 0 and 1
- *  - The first copy: zones 16 and 17
- *  - The second copy: zones 1024 or zone at 256GB which is minimum, and
- *                     the following one
+ *  - The primary superblock: zone at LBA 0 and the next zone
+ *  - The first copy: zone at LBA 16G and the next zone
+ *  - The second copy: zone at LBA 256G and the next zone
  */
 static inline u32 sb_zone_number(int shift, int mirror)
 {
@@ -123,8 +122,8 @@ static inline u32 sb_zone_number(int shift, int mirror)
 
 	switch (mirror) {
 	case 0: return 0;
-	case 1: return 16;
-	case 2: return min_t(u64, btrfs_sb_offset(mirror) >> shift, 1024);
+	case 1: return 1 << (const_ilog2(SZ_16G) - shift);
+	case 2: return 1 << (const_ilog2(SZ_1G) + 8 - shift);
 	}
 
 	return 0;
@@ -300,6 +299,16 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device)
 		zone_sectors = bdev_zone_sectors(bdev);
 	}
 
+	/* We don't support zone size > 8G that SB log zones overlap. */
+	if (zone_sectors > (SZ_8G >> SECTOR_SHIFT)) {
+		btrfs_err_in_rcu(fs_info,
+				 "zoned: %s: zone size %llu is too large",
+				 rcu_str_deref(device->name),
+				 (u64)zone_sectors << SECTOR_SHIFT);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	nr_sectors = bdev_nr_sectors(bdev);
 	/* Check if it's power of 2 (see is_power_of_2) */
 	ASSERT(zone_sectors != 0 && (zone_sectors & (zone_sectors - 1)) == 0);
-- 
2.30.1

[PATCH 2/3] btrfs: zoned: add missing type conversion

[Naohiro Aota]

We need to cast zone_sectors from u32 to u64 when setting the zone_size, or
it set the zone size = 0 when the size >= 4G.

Fixes: 5b316468983d ("btrfs: get zone information of zoned block devices")
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/zoned.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 40cb99854844..4de82da39c10 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -312,7 +312,7 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device)
 	nr_sectors = bdev_nr_sectors(bdev);
 	/* Check if it's power of 2 (see is_power_of_2) */
 	ASSERT(zone_sectors != 0 && (zone_sectors & (zone_sectors - 1)) == 0);
-	zone_info->zone_size = zone_sectors << SECTOR_SHIFT;
+	zone_info->zone_size = (u64)zone_sectors << SECTOR_SHIFT;
 	zone_info->zone_size_shift = ilog2(zone_info->zone_size);
 	zone_info->max_zone_append_size =
 		(u64)queue_max_zone_append_sectors(queue) << SECTOR_SHIFT;
-- 
2.30.1

[PATCH 3/3] btrfs: zoned: do not account freed region of read-only block group as zone_unusable

[Naohiro Aota]

We migrate zone unusable bytes to read-only bytes when a block group is set
to read-only, and account all the free region as bytes_readonly. Thus, we
should not increase block_group->zone_unusable when the block group is
read-only.

Fixes: 169e0da91a21 ("btrfs: zoned: track unusable bytes for zones")
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/free-space-cache.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 711a6a751ae9..81835153f747 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -2555,7 +2555,12 @@ static int __btrfs_add_free_space_zoned(struct btrfs_block_group *block_group,
 	to_unusable = size - to_free;
 
 	ctl->free_space += to_free;
-	block_group->zone_unusable += to_unusable;
+	/*
+	 * If the block group is read-only, we should account freed
+	 * space into bytes_readonly.
+	 */
+	if (!block_group->ro)
+		block_group->zone_unusable += to_unusable;
 	spin_unlock(&ctl->tree_lock);
 	if (!used) {
 		spin_lock(&block_group->lock);
-- 
2.30.1

Re: [PATCH 1/3] btrfs: zoned: move superblock logging zone location

[David Sterba]

On Fri, Feb 26, 2021 at 06:34:36PM +0900, Naohiro Aota wrote:
> This commit moves the location of superblock logging zones basing on the
> static address instead of the static zone number.
> 
> The following zones are reserved as the circular buffer on zoned btrfs.
>   - The primary superblock: zone at LBA 0 and the next zone
>   - The first copy: zone at LBA 16G and the next zone
>   - The second copy: zone at LBA 256G and the next zone

This contains all the important information but somehow feels too short
given how many mails we've exchanged and all the reasoning why we do
that

> 
> We disallow zone size larger than 8GB not to overlap the superblock log
> zones.
> 
> Since the superblock zones overlap, we disallow zone size larger than 8GB.

or why we chose 8G to be the reasonable upper limit for the zone size.

> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  fs/btrfs/zoned.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index 9a5cf153da89..40cb99854844 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -112,10 +112,9 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones,
>  
>  /*
>   * The following zones are reserved as the circular buffer on ZONED btrfs.
> - *  - The primary superblock: zones 0 and 1
> - *  - The first copy: zones 16 and 17
> - *  - The second copy: zones 1024 or zone at 256GB which is minimum, and
> - *                     the following one
> + *  - The primary superblock: zone at LBA 0 and the next zone
> + *  - The first copy: zone at LBA 16G and the next zone
> + *  - The second copy: zone at LBA 256G and the next zone
>   */
>  static inline u32 sb_zone_number(int shift, int mirror)
>  {
> @@ -123,8 +122,8 @@ static inline u32 sb_zone_number(int shift, int mirror)
>  
>  	switch (mirror) {
>  	case 0: return 0;
> -	case 1: return 16;
> -	case 2: return min_t(u64, btrfs_sb_offset(mirror) >> shift, 1024);
> +	case 1: return 1 << (const_ilog2(SZ_16G) - shift);
> +	case 2: return 1 << (const_ilog2(SZ_1G) + 8 - shift);

This ilog(SZ_1G) + 8 is confusing, it should have been 256G for clarity,
as it's a constant it'll get expanded at compile time.

>  	}
>  
>  	return 0;
> @@ -300,6 +299,16 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device)
>  		zone_sectors = bdev_zone_sectors(bdev);
>  	}
>  
> +	/* We don't support zone size > 8G that SB log zones overlap. */
> +	if (zone_sectors > (SZ_8G >> SECTOR_SHIFT)) {
> +		btrfs_err_in_rcu(fs_info,
> +				 "zoned: %s: zone size %llu is too large",
> +				 rcu_str_deref(device->name),
> +				 (u64)zone_sectors << SECTOR_SHIFT);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
>  	nr_sectors = bdev_nr_sectors(bdev);
>  	/* Check if it's power of 2 (see is_power_of_2) */
>  	ASSERT(zone_sectors != 0 && (zone_sectors & (zone_sectors - 1)) == 0);
> -- 
> 2.30.1

Re: [PATCH 2/3] btrfs: zoned: add missing type conversion

[David Sterba]

On Fri, Feb 26, 2021 at 06:34:37PM +0900, Naohiro Aota wrote:
> We need to cast zone_sectors from u32 to u64 when setting the zone_size, or
> it set the zone size = 0 when the size >= 4G.
> 
> Fixes: 5b316468983d ("btrfs: get zone information of zoned block devices")
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  fs/btrfs/zoned.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index 40cb99854844..4de82da39c10 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -312,7 +312,7 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device)
>  	nr_sectors = bdev_nr_sectors(bdev);
>  	/* Check if it's power of 2 (see is_power_of_2) */
>  	ASSERT(zone_sectors != 0 && (zone_sectors & (zone_sectors - 1)) == 0);
> -	zone_info->zone_size = zone_sectors << SECTOR_SHIFT;
> +	zone_info->zone_size = (u64)zone_sectors << SECTOR_SHIFT;

That should be fixed by changing type to sector_t, it's already so in
other functions (btrfs_reset_sb_log_zones, emulate_report_zones).

In btrfs_get_dev_zone_info thers's already a type mismatch near line
300:

	zone_sectors = bdev_zone_sectors(bdev);

linux/blkdev.h:
static inline sector_t bdev_zone_sectors(struct block_device *bdev)

>  	zone_info->zone_size_shift = ilog2(zone_info->zone_size);
>  	zone_info->max_zone_append_size =
>  		(u64)queue_max_zone_append_sectors(queue) << SECTOR_SHIFT;
> -- 
> 2.30.1

Re: [PATCH 1/3] btrfs: zoned: move superblock logging zone location

[Naohiro Aota]

On Fri, Feb 26, 2021 at 08:11:30PM +0100, David Sterba wrote:
> On Fri, Feb 26, 2021 at 06:34:36PM +0900, Naohiro Aota wrote:
> > This commit moves the location of superblock logging zones basing on the
> > static address instead of the static zone number.
> > 
> > The following zones are reserved as the circular buffer on zoned btrfs.
> >   - The primary superblock: zone at LBA 0 and the next zone
> >   - The first copy: zone at LBA 16G and the next zone
> >   - The second copy: zone at LBA 256G and the next zone
> 
> This contains all the important information but somehow feels too short
> given how many mails we've exchanged and all the reasoning why we do
> that

Yep, sure. I'll expand the description and repost.

> > 
> > We disallow zone size larger than 8GB not to overlap the superblock log
> > zones.
> > 
> > Since the superblock zones overlap, we disallow zone size larger than 8GB.
> 
> or why we chose 8G to be the reasonable upper limit for the zone size.
> 
> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> > ---
> >  fs/btrfs/zoned.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> > index 9a5cf153da89..40cb99854844 100644
> > --- a/fs/btrfs/zoned.c
> > +++ b/fs/btrfs/zoned.c
> > @@ -112,10 +112,9 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones,
> >  
> >  /*
> >   * The following zones are reserved as the circular buffer on ZONED btrfs.
> > - *  - The primary superblock: zones 0 and 1
> > - *  - The first copy: zones 16 and 17
> > - *  - The second copy: zones 1024 or zone at 256GB which is minimum, and
> > - *                     the following one
> > + *  - The primary superblock: zone at LBA 0 and the next zone
> > + *  - The first copy: zone at LBA 16G and the next zone
> > + *  - The second copy: zone at LBA 256G and the next zone
> >   */
> >  static inline u32 sb_zone_number(int shift, int mirror)
> >  {
> > @@ -123,8 +122,8 @@ static inline u32 sb_zone_number(int shift, int mirror)
> >  
> >  	switch (mirror) {
> >  	case 0: return 0;
> > -	case 1: return 16;
> > -	case 2: return min_t(u64, btrfs_sb_offset(mirror) >> shift, 1024);
> > +	case 1: return 1 << (const_ilog2(SZ_16G) - shift);
> > +	case 2: return 1 << (const_ilog2(SZ_1G) + 8 - shift);
> 
> This ilog(SZ_1G) + 8 is confusing, it should have been 256G for clarity,
> as it's a constant it'll get expanded at compile time.

I'd like to use SZ_256G here, but linux/sizes.h does not define
it. I'll define one for us and use it in the next version.

> >  	}
> >  
> >  	return 0;
> > @@ -300,6 +299,16 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device)
> >  		zone_sectors = bdev_zone_sectors(bdev);
> >  	}
> >  
> > +	/* We don't support zone size > 8G that SB log zones overlap. */
> > +	if (zone_sectors > (SZ_8G >> SECTOR_SHIFT)) {
> > +		btrfs_err_in_rcu(fs_info,
> > +				 "zoned: %s: zone size %llu is too large",
> > +				 rcu_str_deref(device->name),
> > +				 (u64)zone_sectors << SECTOR_SHIFT);
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> >  	nr_sectors = bdev_nr_sectors(bdev);
> >  	/* Check if it's power of 2 (see is_power_of_2) */
> >  	ASSERT(zone_sectors != 0 && (zone_sectors & (zone_sectors - 1)) == 0);
> > -- 
> > 2.30.1

Re: [PATCH 2/3] btrfs: zoned: add missing type conversion

[Naohiro Aota]

On Fri, Feb 26, 2021 at 08:18:58PM +0100, David Sterba wrote:
> On Fri, Feb 26, 2021 at 06:34:37PM +0900, Naohiro Aota wrote:
> > We need to cast zone_sectors from u32 to u64 when setting the zone_size, or
> > it set the zone size = 0 when the size >= 4G.
> > 
> > Fixes: 5b316468983d ("btrfs: get zone information of zoned block devices")
> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> > ---
> >  fs/btrfs/zoned.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> > index 40cb99854844..4de82da39c10 100644
> > --- a/fs/btrfs/zoned.c
> > +++ b/fs/btrfs/zoned.c
> > @@ -312,7 +312,7 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device)
> >  	nr_sectors = bdev_nr_sectors(bdev);
> >  	/* Check if it's power of 2 (see is_power_of_2) */
> >  	ASSERT(zone_sectors != 0 && (zone_sectors & (zone_sectors - 1)) == 0);
> > -	zone_info->zone_size = zone_sectors << SECTOR_SHIFT;
> > +	zone_info->zone_size = (u64)zone_sectors << SECTOR_SHIFT;
> 
> That should be fixed by changing type to sector_t, it's already so in
> other functions (btrfs_reset_sb_log_zones, emulate_report_zones).
> 
> In btrfs_get_dev_zone_info thers's already a type mismatch near line
> 300:
> 
> 	zone_sectors = bdev_zone_sectors(bdev);
> 
> linux/blkdev.h:
> static inline sector_t bdev_zone_sectors(struct block_device *bdev)

Ah, yes, I forgot that point. I'll repost by fixing the type of
zone_sectors.

> >  	zone_info->zone_size_shift = ilog2(zone_info->zone_size);
> >  	zone_info->max_zone_append_size =
> >  		(u64)queue_max_zone_append_sectors(queue) << SECTOR_SHIFT;
> > -- 
> > 2.30.1

Re: [PATCH 1/3] btrfs: zoned: move superblock logging zone location

[Damien Le Moal]

On 2021/03/01 14:02, Naohiro Aota wrote:
> On Fri, Feb 26, 2021 at 08:11:30PM +0100, David Sterba wrote:
>> On Fri, Feb 26, 2021 at 06:34:36PM +0900, Naohiro Aota wrote:
>>> This commit moves the location of superblock logging zones basing on the
>>> static address instead of the static zone number.
>>>
>>> The following zones are reserved as the circular buffer on zoned btrfs.
>>>   - The primary superblock: zone at LBA 0 and the next zone
>>>   - The first copy: zone at LBA 16G and the next zone
>>>   - The second copy: zone at LBA 256G and the next zone
>>
>> This contains all the important information but somehow feels too short
>> given how many mails we've exchanged and all the reasoning why we do
>> that
> 
> Yep, sure. I'll expand the description and repost.
> 
>>>
>>> We disallow zone size larger than 8GB not to overlap the superblock log
>>> zones.
>>>
>>> Since the superblock zones overlap, we disallow zone size larger than 8GB.
>>
>> or why we chose 8G to be the reasonable upper limit for the zone size.
>>
>>> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
>>> ---
>>>  fs/btrfs/zoned.c | 21 +++++++++++++++------
>>>  1 file changed, 15 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>>> index 9a5cf153da89..40cb99854844 100644
>>> --- a/fs/btrfs/zoned.c
>>> +++ b/fs/btrfs/zoned.c
>>> @@ -112,10 +112,9 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones,
>>>  
>>>  /*
>>>   * The following zones are reserved as the circular buffer on ZONED btrfs.
>>> - *  - The primary superblock: zones 0 and 1
>>> - *  - The first copy: zones 16 and 17
>>> - *  - The second copy: zones 1024 or zone at 256GB which is minimum, and
>>> - *                     the following one
>>> + *  - The primary superblock: zone at LBA 0 and the next zone
>>> + *  - The first copy: zone at LBA 16G and the next zone
>>> + *  - The second copy: zone at LBA 256G and the next zone
>>>   */
>>>  static inline u32 sb_zone_number(int shift, int mirror)
>>>  {
>>> @@ -123,8 +122,8 @@ static inline u32 sb_zone_number(int shift, int mirror)
>>>  
>>>  	switch (mirror) {
>>>  	case 0: return 0;
>>> -	case 1: return 16;
>>> -	case 2: return min_t(u64, btrfs_sb_offset(mirror) >> shift, 1024);
>>> +	case 1: return 1 << (const_ilog2(SZ_16G) - shift);
>>> +	case 2: return 1 << (const_ilog2(SZ_1G) + 8 - shift);
>>
>> This ilog(SZ_1G) + 8 is confusing, it should have been 256G for clarity,
>> as it's a constant it'll get expanded at compile time.
> 
> I'd like to use SZ_256G here, but linux/sizes.h does not define
> it. I'll define one for us and use it in the next version.

Or just use const_ilog2(256 * SZ_1G)... That is fairly easy to understand :)

I would even go further and add:

#define BTRFS_SB_FIRST_COPY_OFST		(16ULL * SZ_1G)
#define BTRFS_SB_SECOND_COPY_OFST		(256ULL * SZ_1G)

To be clear about what the values represent.
Then you can have:

+	case 1: return 1 << (const_ilog2(BTRFS_SB_FIRST_COPY_OFST) - shift);
+	case 2: return 1 << (const_ilog2(BTRFS_SB_SECOND_COPY_OFST) - shift);

> 
>>>  	}
>>>  
>>>  	return 0;
>>> @@ -300,6 +299,16 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device)
>>>  		zone_sectors = bdev_zone_sectors(bdev);
>>>  	}
>>>  
>>> +	/* We don't support zone size > 8G that SB log zones overlap. */
>>> +	if (zone_sectors > (SZ_8G >> SECTOR_SHIFT)) {
>>> +		btrfs_err_in_rcu(fs_info,
>>> +				 "zoned: %s: zone size %llu is too large",
>>> +				 rcu_str_deref(device->name),
>>> +				 (u64)zone_sectors << SECTOR_SHIFT);
>>> +		ret = -EINVAL;
>>> +		goto out;
>>> +	}
>>> +
>>>  	nr_sectors = bdev_nr_sectors(bdev);
>>>  	/* Check if it's power of 2 (see is_power_of_2) */
>>>  	ASSERT(zone_sectors != 0 && (zone_sectors & (zone_sectors - 1)) == 0);
>>> -- 
>>> 2.30.1
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/3] btrfs: zoned: move superblock logging zone location

[David Sterba]

On Mon, Mar 01, 2021 at 05:17:52AM +0000, Damien Le Moal wrote:
> On 2021/03/01 14:02, Naohiro Aota wrote:
> > On Fri, Feb 26, 2021 at 08:11:30PM +0100, David Sterba wrote:
> >> On Fri, Feb 26, 2021 at 06:34:36PM +0900, Naohiro Aota wrote:
> >>> This commit moves the location of superblock logging zones basing on the
> >>> static address instead of the static zone number.
> >>>
> >>> The following zones are reserved as the circular buffer on zoned btrfs.
> >>>   - The primary superblock: zone at LBA 0 and the next zone
> >>>   - The first copy: zone at LBA 16G and the next zone
> >>>   - The second copy: zone at LBA 256G and the next zone
> >>
> >> This contains all the important information but somehow feels too short
> >> given how many mails we've exchanged and all the reasoning why we do
> >> that
> > 
> > Yep, sure. I'll expand the description and repost.
> > 
> >>>
> >>> We disallow zone size larger than 8GB not to overlap the superblock log
> >>> zones.
> >>>
> >>> Since the superblock zones overlap, we disallow zone size larger than 8GB.
> >>
> >> or why we chose 8G to be the reasonable upper limit for the zone size.
> >>
> >>> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> >>> ---
> >>>  fs/btrfs/zoned.c | 21 +++++++++++++++------
> >>>  1 file changed, 15 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> >>> index 9a5cf153da89..40cb99854844 100644
> >>> --- a/fs/btrfs/zoned.c
> >>> +++ b/fs/btrfs/zoned.c
> >>> @@ -112,10 +112,9 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones,
> >>>  
> >>>  /*
> >>>   * The following zones are reserved as the circular buffer on ZONED btrfs.
> >>> - *  - The primary superblock: zones 0 and 1
> >>> - *  - The first copy: zones 16 and 17
> >>> - *  - The second copy: zones 1024 or zone at 256GB which is minimum, and
> >>> - *                     the following one
> >>> + *  - The primary superblock: zone at LBA 0 and the next zone
> >>> + *  - The first copy: zone at LBA 16G and the next zone
> >>> + *  - The second copy: zone at LBA 256G and the next zone
> >>>   */
> >>>  static inline u32 sb_zone_number(int shift, int mirror)
> >>>  {
> >>> @@ -123,8 +122,8 @@ static inline u32 sb_zone_number(int shift, int mirror)
> >>>  
> >>>  	switch (mirror) {
> >>>  	case 0: return 0;
> >>> -	case 1: return 16;
> >>> -	case 2: return min_t(u64, btrfs_sb_offset(mirror) >> shift, 1024);
> >>> +	case 1: return 1 << (const_ilog2(SZ_16G) - shift);
> >>> +	case 2: return 1 << (const_ilog2(SZ_1G) + 8 - shift);
> >>
> >> This ilog(SZ_1G) + 8 is confusing, it should have been 256G for clarity,
> >> as it's a constant it'll get expanded at compile time.
> > 
> > I'd like to use SZ_256G here, but linux/sizes.h does not define
> > it. I'll define one for us and use it in the next version.
> 
> Or just use const_ilog2(256 * SZ_1G)... That is fairly easy to understand :)
> 
> I would even go further and add:
> 
> #define BTRFS_SB_FIRST_COPY_OFST		(16ULL * SZ_1G)
> #define BTRFS_SB_SECOND_COPY_OFST		(256ULL * SZ_1G)
> 
> To be clear about what the values represent.
> Then you can have:
> 
> +	case 1: return 1 << (const_ilog2(BTRFS_SB_FIRST_COPY_OFST) - shift);
> +	case 2: return 1 << (const_ilog2(BTRFS_SB_SECOND_COPY_OFST) - shift);

That's even better, so the magic constants are defined in a more visible
place. Maybe the shift can be also defined separately and then just used
here, like we have PAGE_SIZE/PAGE_SHIFT and such.