Re: [PATCH 2/2] null_blk: don't finish zones to set them read-only or offline

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

On 2026/10/22 9:36, Niklas Cassel wrote:
> On Thu, Oct 22, 2026 at 09:15:32AM +0200, Damien Le Moal wrote:
>> On 2026/10/21 22:21, Niklas Cassel wrote:
>>> null_set_zone_cond() calls null_finish_zone() to release the zone
>>> resources held by a zone before setting it read-only or offline.
>>>
>>> For an empty or closed zone, null_finish_zone() first runs the zone
>>> resource checks, which can close an unrelated implicitly open zone, or
>>> fail. As the error is ignored, a failure for a closed zone leaks
>>> nr_zones_closed, permanently lowering the effective active zone limit.
>>>
>>> Setting a zone read-only or offline requires no zone resources, so
>>> release the zone resources held by the zone directly instead, using a
>>> new helper, null_release_zone_resources().
>>>
>>> Fixes: d3a5738849e0 ("null_blk: support read-only and offline zone conditions")
>>> Signed-off-by: Niklas Cassel <cassel@kernel.org>
>>
>> Looks good, modulo one nit below.
>>
>> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> 
> (snip)
> 
>>> @@ -768,9 +795,7 @@ static void null_set_zone_cond(struct nullb_device *dev,
>>
>> This function is called only from zone_cond_store() with cond either
>> BLK_ZONE_COND_READONLY or BLK_ZONE_COND_OFFLINE. Since your change is valid only
>> for these 2 conditions, we really should check that cond is either of the
>> already used values and add a comment, to avoid invalid use of
>> null_set_zone_cond() in the future (without tweaking it).
> 
> Could you clarify with code what you mean?
> 
> 
> For a comment, there is one already:
> https://github.com/torvalds/linux/blob/v7.3/drivers/block/null_blk/zoned.c#L746
> 
> For a check that cond is either read-only or offline, there such a check
> already:
> https://github.com/torvalds/linux/blob/v7.3/drivers/block/null_blk/zoned.c#L751-L752

Ah! I missed that! So please ignore my nit comment.

That said, adding a cleanup patch that changes null_set_zone_cond() to something
like:

static void null_set_zone_readonly_or_offline(struct nullb_device *dev,
			       		      struct nullb_zone *zone,
					      bool set_offline)

would make things a lot clearer :)


-- 
Damien Le Moal
Western Digital Research