Re: [PATCH 2/4] zloop: share more code for opening a zone backing file

[Damien Le Moal <dlemoal@kernel.org>]

On 2026/10/29 22:03, Christoph Hellwig wrote:
> Use a common path for opening and querying the block size.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/block/zloop.c | 65 +++++++++++++++++--------------------------
>  1 file changed, 26 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/block/zloop.c b/drivers/block/zloop.c
> index 71f7685efb13..3dc484e3c700 100644
> --- a/drivers/block/zloop.c
> +++ b/drivers/block/zloop.c
> @@ -1107,8 +1107,6 @@ static int zloop_init_zone(struct zloop_device *zlo, struct zloop_options *opts,
>  {
>  	struct zloop_zone *zone = &zlo->zones[zone_no];
>  	int oflags = O_RDWR;
> -	struct kstat stat;
> -	sector_t file_sectors;
>  	int ret;
>  
>  	mutex_init(&zone->lock);
> @@ -1122,27 +1120,33 @@ static int zloop_init_zone(struct zloop_device *zlo, struct zloop_options *opts,
>  	if (!opts->buffered_io)
>  		oflags |= O_DIRECT;
>  
> +	zone->file = zloop_filp_open_fmt(oflags, 0600, "%s/%u/%s-%06u",
> +				zlo->base_dir, zlo->id,
> +				zone_no < zlo->nr_conv_zones ? "cnv" : "seq",
> +				zone_no);
> +	if (IS_ERR(zone->file)) {
> +		pr_err("Failed to open zone %u file %s/%u/%s-%06u (err=%ld)",
> +		       zone_no, zlo->base_dir, zlo->id,
> +		       zone_no < zlo->nr_conv_zones ? "cnv" : "seq",
> +		       zone_no, PTR_ERR(zone->file));
> +		return PTR_ERR(zone->file);
> +	}
> +
> +	if (zlo->block_size) {

Shouldn't this be:

	if (!zlo->block_size) {

?

Not a huge problem, but no need to repeatedly get the block size for all zones.
Other than that, this is a nice cleanup!

> +		ret = zloop_get_block_size(zlo, zone);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	if (zone_no < zlo->nr_conv_zones) {
> +		sector_t file_sectors;
> +		struct kstat stat;
> +
>  		/* Conventional zone file. */
>  		set_bit(ZLOOP_ZONE_CONV, &zone->flags);
>  		zone->cond = BLK_ZONE_COND_NOT_WP;
>  		zone->wp = U64_MAX;
>  
> -		zone->file = zloop_filp_open_fmt(oflags, 0600, "%s/%u/cnv-%06u",
> -					zlo->base_dir, zlo->id, zone_no);
> -		if (IS_ERR(zone->file)) {
> -			pr_err("Failed to open zone %u file %s/%u/cnv-%06u (err=%ld)",
> -			       zone_no, zlo->base_dir, zlo->id, zone_no,
> -			       PTR_ERR(zone->file));
> -			return PTR_ERR(zone->file);
> -		}
> -
> -		if (!zlo->block_size) {
> -			ret = zloop_get_block_size(zlo, zone);
> -			if (ret)
> -				return ret;
> -		}
> -
>  		ret = vfs_getattr(&zone->file->f_path, &stat, STATX_SIZE, 0);
>  		if (ret < 0) {
>  			pr_err("Failed to get zone %u file stat\n", zone_no);
> @@ -1163,30 +1167,13 @@ static int zloop_init_zone(struct zloop_device *zlo, struct zloop_options *opts,
>  			       zone_no, ret);
>  			return ret;
>  		}
> -
> -		return 0;
> -	}
> -
> -	/* Sequential zone file. */
> -	zone->file = zloop_filp_open_fmt(oflags, 0600, "%s/%u/seq-%06u",
> -					 zlo->base_dir, zlo->id, zone_no);
> -	if (IS_ERR(zone->file)) {
> -		pr_err("Failed to open zone %u file %s/%u/seq-%06u (err=%ld)",
> -		       zone_no, zlo->base_dir, zlo->id, zone_no,
> -		       PTR_ERR(zone->file));
> -		return PTR_ERR(zone->file);
> -	}
> -
> -	if (!zlo->block_size) {
> -		ret = zloop_get_block_size(zlo, zone);
> -		if (ret)
> -			return ret;
> +	} else {
> +		/* Sequential zone file. */
> +		mutex_lock(&zone->lock);
> +		ret = zloop_update_seq_zone(zlo, zone_no);
> +		mutex_unlock(&zone->lock);
>  	}
>  
> -	mutex_lock(&zone->lock);
> -	ret = zloop_update_seq_zone(zlo, zone_no);
> -	mutex_unlock(&zone->lock);
> -
>  	return ret;
>  }
>  


-- 
Damien Le Moal
Western Digital Research