Re: [PATCH 6/8] btrfs: extract out zone cache usage into it's own helper

[Naohiro Aota <Naohiro.Aota@wdc.com>]

On Fri, Dec 16, 2022 at 03:15:56PM -0500, Josef Bacik wrote:
> There's a special case for loading the device zone info if we have the
> zone cache which is a fair bit of code.  Extract this out into it's own
> helper to clean up the code a little bit, and as a side effect it fixes
> an uninitialized error we get with -Wmaybe-uninitialized where it
> thought zno may have been uninitialized.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

I'm going to rewrite the code around here with the following WIP branch, to
improve the zone caching.

https://github.com/naota/linux/commits/feature/zone-cache

Specifically, this commit removes the for-loop and the "if (i ==
*nr_zones)" block you moved in this patch. So, the resulting code will be
small enough to keep it there.

https://github.com/naota/linux/commit/8d592ac744111bb2f51595a1608beecadb2c5d03

Could you wait for a while for me to clean-up and send the series? I'll
also check the series with -Wmaybe-uninitialized.

> ---
>  fs/btrfs/zoned.c | 73 +++++++++++++++++++++++++++++-------------------
>  1 file changed, 45 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index a759668477bb..f3640ab95e5e 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -216,11 +216,46 @@ static int emulate_report_zones(struct btrfs_device *device, u64 pos,
>  	return i;
>  }
>  
> +static int load_zones_from_cache(struct btrfs_zoned_device_info *zinfo, u64 pos,
> +				 struct blk_zone *zones, unsigned int *nr_zones)
> +{
> +	unsigned int i;
> +	u32 zno;
> +
> +	if (!zinfo->zone_cache)
> +		return -ENOENT;
> +
> +	ASSERT(IS_ALIGNED(pos, zinfo->zone_size));
> +	zno = pos >> zinfo->zone_size_shift;
> +
> +	/*
> +	 * We cannot report zones beyond the zone end. So, it is OK to
> +	 * cap *nr_zones to at the end.
> +	 */
> +	*nr_zones = min_t(u32, *nr_zones, zinfo->nr_zones - zno);
> +
> +	for (i = 0; i < *nr_zones; i++) {
> +		struct blk_zone *zone_info;
> +
> +		zone_info = &zinfo->zone_cache[zno + i];
> +		if (!zone_info->len)
> +			break;
> +	}
> +
> +	if (i == *nr_zones) {
> +		/* Cache hit on all the zones */
> +		memcpy(zones, zinfo->zone_cache + zno,
> +		       sizeof(*zinfo->zone_cache) * *nr_zones);
> +		return 0;
> +	}
> +
> +	return -ENOENT;
> +}
> +
>  static int btrfs_get_dev_zones(struct btrfs_device *device, u64 pos,
>  			       struct blk_zone *zones, unsigned int *nr_zones)
>  {
>  	struct btrfs_zoned_device_info *zinfo = device->zone_info;
> -	u32 zno;
>  	int ret;
>  
>  	if (!*nr_zones)
> @@ -233,32 +268,8 @@ static int btrfs_get_dev_zones(struct btrfs_device *device, u64 pos,
>  	}
>  
>  	/* Check cache */
> -	if (zinfo->zone_cache) {
> -		unsigned int i;
> -
> -		ASSERT(IS_ALIGNED(pos, zinfo->zone_size));
> -		zno = pos >> zinfo->zone_size_shift;
> -		/*
> -		 * We cannot report zones beyond the zone end. So, it is OK to
> -		 * cap *nr_zones to at the end.
> -		 */
> -		*nr_zones = min_t(u32, *nr_zones, zinfo->nr_zones - zno);
> -
> -		for (i = 0; i < *nr_zones; i++) {
> -			struct blk_zone *zone_info;
> -
> -			zone_info = &zinfo->zone_cache[zno + i];
> -			if (!zone_info->len)
> -				break;
> -		}
> -
> -		if (i == *nr_zones) {
> -			/* Cache hit on all the zones */
> -			memcpy(zones, zinfo->zone_cache + zno,
> -			       sizeof(*zinfo->zone_cache) * *nr_zones);
> -			return 0;
> -		}
> -	}
> +	if (!load_zones_from_cache(zinfo, pos, zones, nr_zones))
> +		return 0;
>  
>  	ret = blkdev_report_zones(device->bdev, pos >> SECTOR_SHIFT, *nr_zones,
>  				  copy_zone_info_cb, zones);
> @@ -274,9 +285,15 @@ static int btrfs_get_dev_zones(struct btrfs_device *device, u64 pos,
>  		return -EIO;
>  
>  	/* Populate cache */
> -	if (zinfo->zone_cache)
> +	if (zinfo->zone_cache) {
> +		u32 zno;
> +
> +		ASSERT(IS_ALIGNED(pos, zinfo->zone_size));
> +		zno = pos >> zinfo->zone_size_shift;
> +
>  		memcpy(zinfo->zone_cache + zno, zones,
>  		       sizeof(*zinfo->zone_cache) * *nr_zones);
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.26.3
>