[PATCH v2 0/3] null_blk: zone resource management fixes

[PATCH v2 0/3] null_blk: zone resource management fixes

[Niklas Cassel]

Patch 1 fixes writing, opening and finishing a closed zone when both the
open and the active zone limits are reached: null_blk fails the request
instead of closing an implicitly open zone, as ZBC-3 and NVMe ZNS
require.

Patch 2 fixes zone_readonly and zone_offline, which can close an
unrelated zone, or leak the closed zone count.

Patch 3 cleans up the code that sets zones read-only or offline, with
no functional change.

Both issues were reproduced, and the fixes tested, in QEMU, with
null_blk devices created through configfs (memory_backed=0 and 1,
CONFIG_PROVE_LOCKING=y).

Developed with the help of an AI coding assistant.

Changes since v1:
- Picked up Reviewed-by tags from Damien on patches 1 and 2.
- New patch 3, a cleanup suggested by Damien.

Niklas Cassel (3):
  null_blk: fix opening closed zones at the active zone limit
  null_blk: don't finish zones to set them read-only or offline
  null_blk: clean up setting zones read-only or offline

 drivers/block/null_blk/zoned.c | 63 +++++++++++++++++++++++++---------
 1 file changed, 46 insertions(+), 17 deletions(-)


base-commit: f47d223627f80cb7a1eb14c585fe6cb35de5e984
-- 
2.56.0

[PATCH v2 1/3] null_blk: fix opening closed zones at the active zone limit

[Niklas Cassel]

When the open zone limit is reached, null_check_open() closes an
implicitly open zone only if the active zone limit is not reached.

Closing an implicitly open zone does not change the number of active
zones, so this check is wrong. For an empty zone, the active zone limit
was already checked by null_check_zone_resources(). For a closed zone,
the check makes writing to, opening or finishing the zone fail with
BLK_STS_ZONE_OPEN_RESOURCE at the active zone limit, whereas ZBC-3
(4.5.3.2.7.2) and NVMe ZNS (2.1.1.4.1) require closing an implicitly
open zone instead.

Remove the check, and update the comment above
null_check_zone_resources() accordingly.

Fixes: dc4d137ee3b7 ("null_blk: add support for max open/active zone limit for zoned devices")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/block/null_blk/zoned.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 7e9b5ffb9e841..ecdc8d68ca2a9 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -311,10 +311,8 @@ static blk_status_t null_check_open(struct nullb_device *dev)
 		return BLK_STS_OK;
 
 	if (dev->nr_zones_imp_open) {
-		if (null_check_active(dev) == BLK_STS_OK) {
-			null_close_imp_open_zone(dev);
-			return BLK_STS_OK;
-		}
+		null_close_imp_open_zone(dev);
+		return BLK_STS_OK;
 	}
 
 	return BLK_STS_ZONE_OPEN_RESOURCE;
@@ -330,8 +328,11 @@ static blk_status_t null_check_open(struct nullb_device *dev)
  *
  * ZBC states that an implicit open zone shall be closed only if there is not
  * room within the open limit. However, with the addition of an active limit,
- * it is not certain that closing an implicit open zone will allow a new zone
- * to be opened, since we might already be at the active limit capacity.
+ * it is not certain that closing an implicit open zone will allow an empty
+ * zone to be opened, since we might already be at the active limit capacity.
+ * Therefore, the active limit is checked first for an empty zone. A closed
+ * zone already holds an active zone resource, so only the open limit has to
+ * be checked for a closed zone.
  */
 static blk_status_t null_check_zone_resources(struct nullb_device *dev,
 					      struct nullb_zone *zone)
-- 
2.56.0

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

[Niklas Cassel]

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")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/block/null_blk/zoned.c | 35 +++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index ecdc8d68ca2a9..015f5d304c7ce 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -743,6 +743,33 @@ blk_status_t null_process_zoned_cmd(struct nullb_cmd *cmd, enum req_op op,
 	}
 }
 
+/*
+ * Release the zone resources held by a zone, before changing the condition of
+ * the zone to a condition that does not hold any zone resources.
+ */
+static void null_release_zone_resources(struct nullb_device *dev,
+					struct nullb_zone *zone)
+{
+	if (!dev->need_zone_res_mgmt)
+		return;
+
+	spin_lock(&dev->zone_res_lock);
+	switch (zone->cond) {
+	case BLK_ZONE_COND_IMP_OPEN:
+		dev->nr_zones_imp_open--;
+		break;
+	case BLK_ZONE_COND_EXP_OPEN:
+		dev->nr_zones_exp_open--;
+		break;
+	case BLK_ZONE_COND_CLOSED:
+		dev->nr_zones_closed--;
+		break;
+	default:
+		break;
+	}
+	spin_unlock(&dev->zone_res_lock);
+}
+
 /*
  * Set a zone in the read-only or offline condition.
  */
@@ -759,8 +786,8 @@ static void null_set_zone_cond(struct nullb_device *dev,
 	 * If the read-only condition is requested again to zones already in
 	 * read-only condition, restore back normal empty condition. Do the same
 	 * if the offline condition is requested for offline zones. Otherwise,
-	 * set the specified zone condition to the zones. Finish the zones
-	 * beforehand to free up zone resources.
+	 * set the specified zone condition to the zones. Release the zone
+	 * resources held by the zones beforehand.
 	 */
 	if (zone->cond == cond) {
 		zone->cond = BLK_ZONE_COND_EMPTY;
@@ -768,9 +795,7 @@ static void null_set_zone_cond(struct nullb_device *dev,
 		if (dev->memory_backed)
 			null_handle_discard(dev, zone->start, zone->len);
 	} else {
-		if (zone->cond != BLK_ZONE_COND_READONLY &&
-		    zone->cond != BLK_ZONE_COND_OFFLINE)
-			null_finish_zone(dev, zone);
+		null_release_zone_resources(dev, zone);
 		zone->cond = cond;
 		zone->wp = NULL_ZONE_INVALID_WP;
 	}
-- 
2.56.0

[PATCH v2 3/3] null_blk: clean up setting zones read-only or offline

[Niklas Cassel]

null_set_zone_cond() takes a zone condition, even though read-only and
offline are the only zone conditions that it can set. Because of this,
it needs a WARN_ON_ONCE() to reject any other zone condition, and its
name suggests a generic helper that can set any zone condition.

Rename null_set_zone_cond() to null_set_zone_readonly_or_offline(), and
replace its zone condition argument with a bool, set_offline, which
selects between the read-only and offline conditions. Also remove the
comment above the function, as it only repeats the new function name.

zone_cond_store() keeps taking a zone condition, as it, unlike
null_set_zone_readonly_or_offline(), contains generic code, and might
be extended to support additional zone conditions in the future.

No functional change, as zone_cond_store() is only called with the
read-only or offline condition.

Suggested-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/block/null_blk/zoned.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 015f5d304c7ce..b271d0a88ff21 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -770,15 +770,12 @@ static void null_release_zone_resources(struct nullb_device *dev,
 	spin_unlock(&dev->zone_res_lock);
 }
 
-/*
- * Set a zone in the read-only or offline condition.
- */
-static void null_set_zone_cond(struct nullb_device *dev,
-			       struct nullb_zone *zone, enum blk_zone_cond cond)
+static void null_set_zone_readonly_or_offline(struct nullb_device *dev,
+					      struct nullb_zone *zone,
+					      bool set_offline)
 {
-	if (WARN_ON_ONCE(cond != BLK_ZONE_COND_READONLY &&
-			 cond != BLK_ZONE_COND_OFFLINE))
-		return;
+	enum blk_zone_cond cond =
+		set_offline ? BLK_ZONE_COND_OFFLINE : BLK_ZONE_COND_READONLY;
 
 	null_lock_zone(dev, zone);
 
@@ -814,6 +811,11 @@ ssize_t zone_cond_store(struct nullb_device *dev, const char *page,
 	unsigned int zone_no;
 	int ret;
 
+	/* Read-only and offline are the only conditions supported for now */
+	if (WARN_ON_ONCE(cond != BLK_ZONE_COND_READONLY &&
+			 cond != BLK_ZONE_COND_OFFLINE))
+		return -EINVAL;
+
 	if (!dev->zoned) {
 		pr_err("null_blk device is not zoned\n");
 		return -EINVAL;
@@ -839,7 +841,8 @@ ssize_t zone_cond_store(struct nullb_device *dev, const char *page,
 		return -EINVAL;
 	}
 
-	null_set_zone_cond(dev, &dev->zones[zone_no], cond);
+	null_set_zone_readonly_or_offline(dev, &dev->zones[zone_no],
+					  cond == BLK_ZONE_COND_OFFLINE);
 
 	return count;
 }
-- 
2.56.0

Re: [PATCH v2 3/3] null_blk: clean up setting zones read-only or offline

[Damien Le Moal]

On 2026/10/22 11:22, Niklas Cassel wrote:
> null_set_zone_cond() takes a zone condition, even though read-only and
> offline are the only zone conditions that it can set. Because of this,
> it needs a WARN_ON_ONCE() to reject any other zone condition, and its
> name suggests a generic helper that can set any zone condition.
> 
> Rename null_set_zone_cond() to null_set_zone_readonly_or_offline(), and
> replace its zone condition argument with a bool, set_offline, which
> selects between the read-only and offline conditions. Also remove the
> comment above the function, as it only repeats the new function name.
> 
> zone_cond_store() keeps taking a zone condition, as it, unlike
> null_set_zone_readonly_or_offline(), contains generic code, and might
> be extended to support additional zone conditions in the future.
> 
> No functional change, as zone_cond_store() is only called with the
> read-only or offline condition.
> 
> Suggested-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research