Re: [PATCH] zloop: round capacity up to a zone-size multiple

On 8/3/26 18:58, raoxu wrote:
> From: Xu Rao <[email protected]>
> 
> zloop_ctl_add() derives the number of zones by shifting the requested
> capacity by ilog2(zone_size). Since zone_size is validated as a power of
> two, this is equivalent to integer division and discards any remainder.
> However, Documentation/admin-guide/blockdev/zoned_loop.rst specifies that
> capacity_mb is always rounded up to the nearest higher multiple of the
> zone size.
> 
> Only configurations whose requested capacity is not aligned to the zone
> size are affected. Aligned configurations, including the defaults of
> 16384 MiB capacity and 256 MiB zones, keep the same geometry. An affected
> device is also internally consistent and remains usable, but it is
> smaller than requested. For example, capacity_mb=130 with
> zone_size_mb=64 currently creates two zones and exposes 128 MiB instead
> of the documented three zones and 192 MiB. Since this does not cause an
> error or an I/O failure, and normal test configurations generally use an
> integral number of zones, the discrepancy can remain unnoticed.
> 
> Calculate the number of zones with DIV_ROUND_UP_SECTOR_T() so a partial
> final zone request is represented by one additional full zone. This
> matches the documented control interface while preserving the existing
> sector_t handling on both 32-bit and 64-bit architectures.
> 
> Fixes: eb0570c7df23 ("block: new zoned loop block device driver")
> Cc: [email protected]
> Signed-off-by: Xu Rao <[email protected]>
> ---
>  drivers/block/zloop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/zloop.c b/drivers/block/zloop.c
> index b69d798f203c..e29d82ff52ae 100644
> --- a/drivers/block/zloop.c
> +++ b/drivers/block/zloop.c
> @@ -1187,7 +1187,7 @@ static int zloop_ctl_add(struct zloop_options *opts)
> 
>  	__module_get(THIS_MODULE);
> 
> -	nr_zones = opts->capacity >> ilog2(opts->zone_size);
> +	nr_zones = DIV_ROUND_UP_SECTOR_T(opts->capacity, opts->zone_size);

Your explanation and this change are consistent, but this is going to break many
things because we do not maintain the size of zone per zone and instead always
use zlo->zone_size, assuming that all zones have the same size.
Your change would result in bad things. For instance, zloop_finish_zone()
truncates a zone file to zlo->zone_size, which would be too big for a last runt
zone smaller than other zones.

So nack for this patch. Instead, I think we should fix the documentation to make
more explicit that we do not support a smaller last zone.

>  	if (opts->nr_conv_zones >= nr_zones) {
>  		pr_err("Invalid number of conventional zones %u\n",
>  		       opts->nr_conv_zones);
> --
> 2.50.1
> 


-- 
Damien Le Moal
Western Digital Research