On 6/5/24 13:17, Christoph Hellwig wrote: > improve is misspelled in the subject. > >> @@ -80,6 +80,10 @@ static int blk_validate_zoned_limits(struct queue_limits *lim) >> if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_BLK_DEV_ZONED))) >> return -EINVAL; >> >> + if (lim->max_active_zones && >> + WARN_ON_ONCE(lim->max_open_zones > lim->max_active_zones)) >> + lim->max_open_zones = lim->max_active_zones; > > Given how active zones are defined this is an error condition, and > should return -EINVAL. > >> diff --git a/block/blk-zoned.c b/block/blk-zoned.c >> index 52abebf56027..2af4d5ca81d2 100644 >> --- a/block/blk-zoned.c >> +++ b/block/blk-zoned.c >> @@ -1660,6 +1660,11 @@ static int disk_update_zone_resources(struct gendisk *disk, >> lim = queue_limits_start_update(q); >> >> nr_seq_zones = disk->nr_zones - nr_conv_zones; >> + if (WARN_ON_ONCE(lim.max_active_zones > nr_seq_zones)) >> + lim.max_active_zones = 0; >> + if (WARN_ON_ONCE(lim.max_open_zones > nr_seq_zones)) >> + lim.max_open_zones = 0; > > Why would you warn about this? Offering an open/active limit larger > than the number of sequential zones is a pretty natural condition > for certain corner cases (e.g. create only a tiny namespace on a ZNS > SSD). This could also use a code comment explaining why the limit > is adjusted. Right. I actually did not consider that case, which is indeed valid given that for nvme, the limits are per controller, not namespace (which is a very unfortunate design flaw...). I will remove the warn and add a comment. -- Damien Le Moal Western Digital Research
