diff mbox series

[v11,04/40] btrfs: change superblock location on conventional zone

Message ID 42c1712556e6865837151ad58252fb5f6ecff8f7.1608608848.git.naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series btrfs: zoned block device support | expand

Commit Message

Naohiro Aota Dec. 22, 2020, 3:48 a.m. UTC
We cannot use log-structured superblock writing in conventional zones since
there is no write pointer to determine the last written superblock
position. So, we write a superblock at a static location in a conventional
zone.

The written position is at the beginning of a zone, which is different from
an SB position of regular btrfs. This difference causes a "chicken-and-egg
problem" when supporting zoned emulation on a regular device. To know if
btrfs is (emulated) zoned btrfs, we need to load an SB and check the
feature flag. However, to load an SB, we need to know that it is zoned
btrfs to load it from a different position.

This patch moves the SB location on conventional zones so that the first SB
location will be the same as regular btrfs.

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

Comments

Josef Bacik Jan. 11, 2021, 7:47 p.m. UTC | #1
On 12/21/20 10:48 PM, Naohiro Aota wrote:
> We cannot use log-structured superblock writing in conventional zones since
> there is no write pointer to determine the last written superblock
> position. So, we write a superblock at a static location in a conventional
> zone.
> 
> The written position is at the beginning of a zone, which is different from
> an SB position of regular btrfs. This difference causes a "chicken-and-egg
> problem" when supporting zoned emulation on a regular device. To know if
> btrfs is (emulated) zoned btrfs, we need to load an SB and check the
> feature flag. However, to load an SB, we need to know that it is zoned
> btrfs to load it from a different position.
> 
> This patch moves the SB location on conventional zones so that the first SB
> location will be the same as regular btrfs.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>   fs/btrfs/zoned.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index 90b8d1d5369f..e5619c8bcebb 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -465,7 +465,8 @@ static int sb_log_location(struct block_device *bdev, struct blk_zone *zones,
>   	int ret;
>   
>   	if (zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL) {
> -		*bytenr_ret = zones[0].start << SECTOR_SHIFT;
> +		*bytenr_ret = (zones[0].start << SECTOR_SHIFT) +
> +			btrfs_sb_offset(0);
>   		return 0;
>   	}
>   

I'm confused, we call btrfs_sb_log_location_bdev(), which does

         if (!bdev_is_zoned(bdev)) {
                 *bytenr_ret = btrfs_sb_offset(mirror);
                 return 0;
         }

so how does the emulation work, if we short circuit this if the block device 
isn't zoned?  And then why does it matter where in the conventional zone that we 
put the super block?  Can't we just emulate a conventional zone that starts at 
offset 0, and then the btrfs_sb_offset() will be the same as zones[0].start + 
btrfs_sb_offset(0)?  Thanks,

Josef
Naohiro Aota Jan. 14, 2021, 3:10 p.m. UTC | #2
On Mon, Jan 11, 2021 at 02:47:26PM -0500, Josef Bacik wrote:
>On 12/21/20 10:48 PM, Naohiro Aota wrote:
>>We cannot use log-structured superblock writing in conventional zones since
>>there is no write pointer to determine the last written superblock
>>position. So, we write a superblock at a static location in a conventional
>>zone.
>>
>>The written position is at the beginning of a zone, which is different from
>>an SB position of regular btrfs. This difference causes a "chicken-and-egg
>>problem" when supporting zoned emulation on a regular device. To know if
>>btrfs is (emulated) zoned btrfs, we need to load an SB and check the
>>feature flag. However, to load an SB, we need to know that it is zoned
>>btrfs to load it from a different position.
>>
>>This patch moves the SB location on conventional zones so that the first SB
>>location will be the same as regular btrfs.
>>
>>Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
>>---
>>  fs/btrfs/zoned.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>>diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>>index 90b8d1d5369f..e5619c8bcebb 100644
>>--- a/fs/btrfs/zoned.c
>>+++ b/fs/btrfs/zoned.c
>>@@ -465,7 +465,8 @@ static int sb_log_location(struct block_device *bdev, struct blk_zone *zones,
>>  	int ret;
>>  	if (zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL) {
>>-		*bytenr_ret = zones[0].start << SECTOR_SHIFT;
>>+		*bytenr_ret = (zones[0].start << SECTOR_SHIFT) +
>>+			btrfs_sb_offset(0);
>>  		return 0;
>>  	}
>
>I'm confused, we call btrfs_sb_log_location_bdev(), which does
>
>        if (!bdev_is_zoned(bdev)) {
>                *bytenr_ret = btrfs_sb_offset(mirror);
>                return 0;
>        }
>
>so how does the emulation work, if we short circuit this if the block 
>device isn't zoned?  And then why does it matter where in the 
>conventional zone that we put the super block?  Can't we just emulate 
>a conventional zone that starts at offset 0, and then the 
>btrfs_sb_offset() will be the same as zones[0].start + 
>btrfs_sb_offset(0)?  Thanks,
>
>Josef

I'm sorry for the confusion. Yes, it's really confusing we get
btrfs_sb_log_location() != btrfs_sb_log_location_bdev() with zoned btrfs on
regular devices.

What I'd like to do here is to place the primary SB at the same position as
regular btrfs. Then, we can read the SB on a regular device at mount time
without knowing if the btrfs is zoned or not.

But, I noticed this solution is not working for SB mirrors and confusing to
have different location between btrfs_sb_log_location() vs
btrfs_sb_log_location_bdev().

So, I'll replace this patch in the next version with a patch to use the
regular btrfs' SB locations on non-zoned devices. Since it is non-zoned
device, so we can just write SBs at the regular locations without
consulting the emulated zones... And, we'll have the same results from
btrfs_sb_log_location() and btrfs_sb_log_location_bdev().

I think this change will solve the confusion.

Regards,
diff mbox series

Patch

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 90b8d1d5369f..e5619c8bcebb 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -465,7 +465,8 @@  static int sb_log_location(struct block_device *bdev, struct blk_zone *zones,
 	int ret;
 
 	if (zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL) {
-		*bytenr_ret = zones[0].start << SECTOR_SHIFT;
+		*bytenr_ret = (zones[0].start << SECTOR_SHIFT) +
+			btrfs_sb_offset(0);
 		return 0;
 	}