Re: [PATCH 4/4] dm-zoned: allow for device size smaller than the capacity

[Damien Le Moal <Damien.LeMoal@wdc.com>]

On 2020/03/27 16:15, Hannes Reinecke wrote:
> dm-zoned requires several zones for metadata and chunk bitmaps,
> so it cannot expose the entire capacity as the device size.
> Originally the code would check for the capacity being equal to
> the device size, which is arguably wrong.
> So relax this check and increase the interface version number
> to signal to userspace that it can set a smaller device size.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/md/dm-zoned-target.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> index 7ec9dde24516..89a825d1034e 100644
> --- a/drivers/md/dm-zoned-target.c
> +++ b/drivers/md/dm-zoned-target.c
> @@ -715,7 +715,7 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
>  	aligned_capacity = dev->capacity &
>  				~((sector_t)blk_queue_zone_sectors(q) - 1);
>  	if (ti->begin ||
> -	    ((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
> +	    ((ti->len > dev->capacity) && (ti->len > aligned_capacity))) {
>  		ti->error = "Partial mapping not supported";

The message is now wrong. Also, the condition can now be simplified to:

if ((ti->begin + ti->len) > aligned_capacity) {

Since aligned capacity is equal or smaller than dev capacity. And we have to
account for the potential non-zero begin.

>  		ret = -EINVAL;
>  		goto err;
> @@ -1008,7 +1008,7 @@ static int dmz_message(struct dm_target *ti, unsigned int argc, char **argv,
>  
>  static struct target_type dmz_type = {
>  	.name		 = "zoned",
> -	.version	 = {1, 2, 0},
> +	.version	 = {1, 3, 0},
>  	.features	 = DM_TARGET_SINGLETON | DM_TARGET_ZONED_HM,
>  	.module		 = THIS_MODULE,
>  	.ctr		 = dmz_ctr,
> 

I do not think this is nearly enough: dmz_init_zones() is still considering the
entire drive and does a zone report from 0 on the backend bdev. It should start
at ti->begin and up to ti->begin+ti->len, no ?

Furthermore, this introduce a change in the meaning of the zone ID. Since this
is set to the index of the zone in the report (patch 1), if the mapping is
partial and the zone report does not start at 0, then zone ID is not zone number
on the device anymore. So dmz_start_block() needs to be offset by ti->begin
otherwise IOs will go to the wrong zones.


-- 
Damien Le Moal
Western Digital Research