Make the calculation in sb_zone_number function to be generic and work
for both power-of-2 and non power-of-2 zone sizes.
The function signature has been modified to take block device and mirror
as input as this function is only invoked from callers that have access
to the block device. This enables to use the generic bdev_zone_no
function provided by the block layer to calculate the zone number.
Even though division is used to calculate the zone index for non
power-of-2 zone sizes, this function will not be used in the fast path as
the sb_zone_location cache is used for the superblock zone location.
Reviewed-by: Luis Chamberlain <[email protected]>
Signed-off-by: Pankaj Raghav <[email protected]>
---
fs/btrfs/zoned.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index e8c7cebb2..5be2ef7bb 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -34,9 +34,6 @@
#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)
-
/* Number of superblock log zones */
#define BTRFS_NR_SB_LOG_ZONES 2
@@ -153,15 +150,23 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones,
/*
* Get the first zone number of the superblock mirror
*/
-static inline u32 sb_zone_number(int shift, int mirror)
+static inline u32 sb_zone_number(struct block_device *bdev, int mirror)
{
u64 zone;
ASSERT(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;
+ case 0:
+ zone = 0;
+ break;
+ case 1:
+ zone = bdev_zone_no(bdev,
+ BTRFS_SB_LOG_FIRST_OFFSET >> SECTOR_SHIFT);
+ break;
+ case 2:
+ zone = bdev_zone_no(bdev,
+ BTRFS_SB_LOG_SECOND_OFFSET >> SECTOR_SHIFT);
+ break;
}
ASSERT(zone <= U32_MAX);
@@ -514,7 +519,7 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device, bool populate_cache)
/* Cache the sb zone number */
for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; ++i) {
zone_info->sb_zone_location[i] =
- sb_zone_number(zone_info->zone_size_shift, i);
+ sb_zone_number(bdev, i);
}
/* Validate superblock log */
nr_zones = BTRFS_NR_SB_LOG_ZONES;
@@ -839,7 +844,7 @@ int btrfs_sb_log_location_bdev(struct block_device *bdev, int mirror, int rw,
nr_sectors = bdev_nr_sectors(bdev);
nr_zones = nr_sectors >> zone_sectors_shift;
- sb_zone = sb_zone_number(zone_sectors_shift + SECTOR_SHIFT, mirror);
+ sb_zone = sb_zone_number(bdev, mirror);
if (sb_zone + 1 >= nr_zones)
return -ENOENT;
@@ -963,7 +968,7 @@ int btrfs_reset_sb_log_zones(struct block_device *bdev, int mirror)
nr_sectors = bdev_nr_sectors(bdev);
nr_zones = nr_sectors >> zone_sectors_shift;
- sb_zone = sb_zone_number(zone_sectors_shift + SECTOR_SHIFT, mirror);
+ sb_zone = sb_zone_number(bdev, mirror);
if (sb_zone + 1 >= nr_zones)
return -ENOENT;
--
2.25.1
On Tue, May 17, 2022 at 06:53:40AM +0000, Johannes Thumshirn wrote:
> On 16/05/2022 18:54, Pankaj Raghav wrote:
> > /* Cache the sb zone number */
> > for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; ++i) {
> > zone_info->sb_zone_location[i] =
> > - sb_zone_number(zone_info->zone_size_shift, i);
> > + sb_zone_number(bdev, i);
>
> I think this easily fits on one line now, doesn't it? But given David's
> statement, it'll probably can go away anyways.
I agree that the formatting can be adjusted, but I'm never sure if I
should point it out during the phase of functional changes so it's fixed
in the next iteration as well, or not to point it out to avoid fixups
that would go away anyway. I think I've seen more, a styling pass will
be done anyway once we're close to the final version..
On 16/05/2022 18:54, Pankaj Raghav wrote:
> /* Cache the sb zone number */
> for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; ++i) {
> zone_info->sb_zone_location[i] =
> - sb_zone_number(zone_info->zone_size_shift, i);
> + sb_zone_number(bdev, i);
I think this easily fits on one line now, doesn't it? But given David's
statement, it'll probably can go away anyways.