2023-12-17 16:54:41

by Christoph Hellwig

[permalink] [raw]
Subject: remove support for the host aware zoned model

Hi all,

hen zones were first added the SCSI and ATA specs, two different
models were supported (in addition to the drive managed one that
is invisible to the host):

- host managed where non-conventional zones there is strict requirement
to write at the write pointer, or else an error is returned
- host aware where a write point is maintained if writes always happen
at it, otherwise it is left in an under-defined state and the
sequential write preferred zones behave like conventional zones
(probably very badly performing ones, though)

Not surprisingly this lukewarm model didn't prove to be very useful and
was finally removed from the ZBC and SBC specs (NVMe never implemented
it). Due to to the easily disappearing write pointer host software
could never rely on the write pointer to actually be useful for say
recovery.

Fortunately only a few HDD prototypes shipped using this model which
never made it to mass production. Drop the support before it is too
late. Note that any such host aware prototype HDD can still be used
with Linux as we'll now treat it as a conventional HDD.

Diffstat:
block/blk-settings.c | 83 +++++------------------------------------
block/blk-sysfs.c | 9 ----
block/blk-zoned.c | 3 -
block/blk.h | 2
block/partitions/core.c | 12 -----
drivers/block/null_blk/zoned.c | 2
drivers/block/ublk_drv.c | 2
drivers/block/virtio_blk.c | 78 +++++++++++---------------------------
drivers/md/dm-kcopyd.c | 2
drivers/md/dm-table.c | 45 +++++++++-------------
drivers/md/dm-zoned-metadata.c | 7 +--
drivers/md/dm-zoned-target.c | 4 -
drivers/nvme/host/zns.c | 2
drivers/scsi/scsi_debug.c | 27 ++++++-------
drivers/scsi/sd.c | 50 +++++++++++-------------
drivers/scsi/sd_zbc.c | 16 -------
fs/btrfs/zoned.c | 23 +----------
fs/btrfs/zoned.h | 2
fs/f2fs/data.c | 2
fs/f2fs/super.c | 17 +++-----
include/linux/blkdev.h | 38 +-----------------
21 files changed, 124 insertions(+), 302 deletions(-)


2023-12-17 16:55:23

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/5] virtio_blk: remove the broken zone revalidation support

virtblk_revalidate_zones is called unconditionally from
virtblk_config_changed_work from the virtio config_changed callback.

virtblk_revalidate_zones is a bit odd in that it re-clears the zoned
state for host aware or non-zoned devices, which isn't needed unless the
zoned mode changed - but a zone mode change to a host managed model isn't
handled at all, and virtio_blk also doesn't handle any other config
change except for a capacity change is handled (and even if it was
the upper layers above virtio_blk wouldn't handle it very well).

But even the useful case of a size change that would add or remove
zones isn't handled properly as blk_revalidate_disk_zones expects the
device capacity to cover all zones, but the capacity is only updated
after virtblk_revalidate_zones.

As this code appears to be entirely untested and is getting in the way
remove it for now, but it can be readded in a fixed version with
proper test coverage if needed.

Fixes: 95bfec41bd3d ("virtio-blk: add support for zoned block devices")
Fixes: f1ba4e674feb ("virtio-blk: fix to match virtio spec")
Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/virtio_blk.c | 26 --------------------------
1 file changed, 26 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index aeead732a24dc9..a28f1687066bb4 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -722,27 +722,6 @@ static int virtblk_report_zones(struct gendisk *disk, sector_t sector,
return ret;
}

-static void virtblk_revalidate_zones(struct virtio_blk *vblk)
-{
- u8 model;
-
- virtio_cread(vblk->vdev, struct virtio_blk_config,
- zoned.model, &model);
- switch (model) {
- default:
- dev_err(&vblk->vdev->dev, "unknown zone model %d\n", model);
- fallthrough;
- case VIRTIO_BLK_Z_NONE:
- case VIRTIO_BLK_Z_HA:
- disk_set_zoned(vblk->disk, BLK_ZONED_NONE);
- return;
- case VIRTIO_BLK_Z_HM:
- WARN_ON_ONCE(!vblk->zone_sectors);
- if (!blk_revalidate_disk_zones(vblk->disk, NULL))
- set_capacity_and_notify(vblk->disk, 0);
- }
-}
-
static int virtblk_probe_zoned_device(struct virtio_device *vdev,
struct virtio_blk *vblk,
struct request_queue *q)
@@ -823,10 +802,6 @@ static int virtblk_probe_zoned_device(struct virtio_device *vdev,
*/
#define virtblk_report_zones NULL

-static inline void virtblk_revalidate_zones(struct virtio_blk *vblk)
-{
-}
-
static inline int virtblk_probe_zoned_device(struct virtio_device *vdev,
struct virtio_blk *vblk, struct request_queue *q)
{
@@ -982,7 +957,6 @@ static void virtblk_config_changed_work(struct work_struct *work)
struct virtio_blk *vblk =
container_of(work, struct virtio_blk, config_work);

- virtblk_revalidate_zones(vblk);
virtblk_update_capacity(vblk, true);
}

--
2.39.2


2023-12-17 16:55:27

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/5] virtio_blk: cleanup zoned device probing

Move reading and checking the zoned model from virtblk_probe_zoned_device
into the caller, leaving only the code to perform the actual setup for
host managed zoned devices in virtblk_probe_zoned_device.

This allows to share the model reading and sharing between builds with
and without CONFIG_BLK_DEV_ZONED, and improve it for the
!CONFIG_BLK_DEV_ZONED case.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/block/virtio_blk.c | 50 +++++++++++++++++---------------------
1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index d53d6aa8ee69a4..aeead732a24dc9 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -748,22 +748,6 @@ static int virtblk_probe_zoned_device(struct virtio_device *vdev,
struct request_queue *q)
{
u32 v, wg;
- u8 model;
-
- virtio_cread(vdev, struct virtio_blk_config,
- zoned.model, &model);
-
- switch (model) {
- case VIRTIO_BLK_Z_NONE:
- case VIRTIO_BLK_Z_HA:
- /* Present the host-aware device as non-zoned */
- return 0;
- case VIRTIO_BLK_Z_HM:
- break;
- default:
- dev_err(&vdev->dev, "unsupported zone model %d\n", model);
- return -EINVAL;
- }

dev_dbg(&vdev->dev, "probing host-managed zoned device\n");

@@ -846,16 +830,9 @@ static inline void virtblk_revalidate_zones(struct virtio_blk *vblk)
static inline int virtblk_probe_zoned_device(struct virtio_device *vdev,
struct virtio_blk *vblk, struct request_queue *q)
{
- u8 model;
-
- virtio_cread(vdev, struct virtio_blk_config, zoned.model, &model);
- if (model == VIRTIO_BLK_Z_HM) {
- dev_err(&vdev->dev,
- "virtio_blk: zoned devices are not supported");
- return -EOPNOTSUPP;
- }
-
- return 0;
+ dev_err(&vdev->dev,
+ "virtio_blk: zoned devices are not supported");
+ return -EOPNOTSUPP;
}
#endif /* CONFIG_BLK_DEV_ZONED */

@@ -1570,9 +1547,26 @@ static int virtblk_probe(struct virtio_device *vdev)
* placed after the virtio_device_ready() call above.
*/
if (virtio_has_feature(vdev, VIRTIO_BLK_F_ZONED)) {
- err = virtblk_probe_zoned_device(vdev, vblk, q);
- if (err)
+ u8 model;
+
+ virtio_cread(vdev, struct virtio_blk_config, zoned.model,
+ &model);
+ switch (model) {
+ case VIRTIO_BLK_Z_NONE:
+ case VIRTIO_BLK_Z_HA:
+ /* Present the host-aware device as non-zoned */
+ break;
+ case VIRTIO_BLK_Z_HM:
+ err = virtblk_probe_zoned_device(vdev, vblk, q);
+ if (err)
+ goto out_cleanup_disk;
+ break;
+ default:
+ dev_err(&vdev->dev, "unsupported zone model %d\n",
+ model);
+ err = -EINVAL;
goto out_cleanup_disk;
+ }
}

err = device_add_disk(&vdev->dev, vblk->disk, virtblk_attr_groups);
--
2.39.2


2023-12-17 16:56:15

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 3/5] block: remove support for the host aware zone model

When zones were first added the SCSI and ATA specs, two different
models were supported (in addition to the drive managed one that
is invisible to the host):

- host managed where non-conventional zones there is strict requirement
to write at the write pointer, or else an error is returned
- host aware where a write point is maintained if writes always happen
at it, otherwise it is left in an under-defined state and the
sequential write preferred zones behave like conventional zones
(probably very badly performing ones, though)

Not surprisingly this lukewarm model didn't prove to be very useful and
was finally removed from the ZBC and SBC specs (NVMe never implemented
it). Due to to the easily disappearing write pointer host software
could never rely on the write pointer to actually be useful for say
recovery.

Fortunately only a few HDD prototypes shipped using this model which
never made it to mass production. Drop the support before it is too
late. Note that any such host aware prototype HDD can still be used
with Linux as we'll now treat it as a conventional HDD.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/blk-settings.c | 67 +++++-----------------------------
block/blk-sysfs.c | 9 +----
block/partitions/core.c | 12 +-----
drivers/block/null_blk/zoned.c | 2 +-
drivers/block/ublk_drv.c | 2 +-
drivers/block/virtio_blk.c | 2 +-
drivers/md/dm-kcopyd.c | 2 +-
drivers/md/dm-table.c | 45 ++++++++++-------------
drivers/md/dm-zoned-metadata.c | 7 ++--
drivers/md/dm-zoned-target.c | 4 +-
drivers/nvme/host/zns.c | 2 +-
drivers/scsi/scsi_debug.c | 27 +++++++-------
drivers/scsi/sd.c | 45 ++++++++++-------------
drivers/scsi/sd_zbc.c | 16 +-------
fs/btrfs/zoned.c | 23 ++----------
fs/btrfs/zoned.h | 2 +-
fs/f2fs/data.c | 2 +-
fs/f2fs/super.c | 17 ++++-----
include/linux/blkdev.h | 37 ++-----------------
19 files changed, 92 insertions(+), 231 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 09e3a4d5e4d20f..50e9efb59f67fd 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -56,7 +56,7 @@ void blk_set_default_limits(struct queue_limits *lim)
lim->alignment_offset = 0;
lim->io_opt = 0;
lim->misaligned = 0;
- lim->zoned = BLK_ZONED_NONE;
+ lim->zoned = false;
lim->zone_write_granularity = 0;
lim->dma_alignment = 511;
}
@@ -880,79 +880,30 @@ bool blk_queue_can_use_dma_map_merging(struct request_queue *q,
}
EXPORT_SYMBOL_GPL(blk_queue_can_use_dma_map_merging);

-static bool disk_has_partitions(struct gendisk *disk)
-{
- unsigned long idx;
- struct block_device *part;
- bool ret = false;
-
- rcu_read_lock();
- xa_for_each(&disk->part_tbl, idx, part) {
- if (bdev_is_partition(part)) {
- ret = true;
- break;
- }
- }
- rcu_read_unlock();
-
- return ret;
-}
-
/**
* disk_set_zoned - configure the zoned model for a disk
* @disk: the gendisk of the queue to configure
- * @model: the zoned model to set
- *
- * Set the zoned model of @disk to @model.
+ * @zoned: zoned or not.
*
- * When @model is BLK_ZONED_HM (host managed), this should be called only
- * if zoned block device support is enabled (CONFIG_BLK_DEV_ZONED option).
- * If @model specifies BLK_ZONED_HA (host aware), the effective model used
- * depends on CONFIG_BLK_DEV_ZONED settings and on the existence of partitions
- * on the disk.
+ * When @zoned is %true, this should be called only if zoned block device
+ * support is enabled (CONFIG_BLK_DEV_ZONED option).
*/
-void disk_set_zoned(struct gendisk *disk, enum blk_zoned_model model)
+void disk_set_zoned(struct gendisk *disk, bool zoned)
{
struct request_queue *q = disk->queue;
- unsigned int old_model = q->limits.zoned;

- switch (model) {
- case BLK_ZONED_HM:
- /*
- * Host managed devices are supported only if
- * CONFIG_BLK_DEV_ZONED is enabled.
- */
+ if (zoned) {
WARN_ON_ONCE(!IS_ENABLED(CONFIG_BLK_DEV_ZONED));
- break;
- case BLK_ZONED_HA:
- /*
- * Host aware devices can be treated either as regular block
- * devices (similar to drive managed devices) or as zoned block
- * devices to take advantage of the zone command set, similarly
- * to host managed devices. We try the latter if there are no
- * partitions and zoned block device support is enabled, else
- * we do nothing special as far as the block layer is concerned.
- */
- if (!IS_ENABLED(CONFIG_BLK_DEV_ZONED) ||
- disk_has_partitions(disk))
- model = BLK_ZONED_NONE;
- break;
- case BLK_ZONED_NONE:
- default:
- if (WARN_ON_ONCE(model != BLK_ZONED_NONE))
- model = BLK_ZONED_NONE;
- break;
- }

- q->limits.zoned = model;
- if (model != BLK_ZONED_NONE) {
/*
* Set the zone write granularity to the device logical block
* size by default. The driver can change this value if needed.
*/
+ q->limits.zoned = true;
blk_queue_zone_write_granularity(q,
queue_logical_block_size(q));
- } else if (old_model != BLK_ZONED_NONE) {
+ } else if (q->limits.zoned) {
+ q->limits.zoned = false;
disk_clear_zone_settings(disk);
}
}
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 63e4812623361d..d5e669a401b0cc 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -309,14 +309,9 @@ QUEUE_SYSFS_BIT_FNS(stable_writes, STABLE_WRITES, 0);

static ssize_t queue_zoned_show(struct request_queue *q, char *page)
{
- switch (blk_queue_zoned_model(q)) {
- case BLK_ZONED_HA:
- return sprintf(page, "host-aware\n");
- case BLK_ZONED_HM:
+ if (blk_queue_is_zoned(q))
return sprintf(page, "host-managed\n");
- default:
- return sprintf(page, "none\n");
- }
+ return sprintf(page, "none\n");
}

static ssize_t queue_nr_zones_show(struct request_queue *q, char *page)
diff --git a/block/partitions/core.c b/block/partitions/core.c
index f47ffcfdfcec22..e6ac73617f3e12 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -305,18 +305,10 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
* Partitions are not supported on zoned block devices that are used as
* such.
*/
- switch (disk->queue->limits.zoned) {
- case BLK_ZONED_HM:
+ if (bdev_is_zoned(disk->part0)) {
pr_warn("%s: partitions not supported on host managed zoned block device\n",
disk->disk_name);
return ERR_PTR(-ENXIO);
- case BLK_ZONED_HA:
- pr_info("%s: disabling host aware zoned block device support due to partitions\n",
- disk->disk_name);
- disk_set_zoned(disk, BLK_ZONED_NONE);
- break;
- case BLK_ZONED_NONE:
- break;
}

if (xa_load(&disk->part_tbl, partno))
@@ -613,7 +605,7 @@ static int blk_add_partitions(struct gendisk *disk)
/*
* Partitions are not supported on host managed zoned block devices.
*/
- if (disk->queue->limits.zoned == BLK_ZONED_HM) {
+ if (bdev_is_zoned(disk->part0)) {
pr_warn("%s: ignoring partition table on host managed zoned block device\n",
disk->disk_name);
ret = 0;
diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 55c5b48bc276fe..369eb1e78bb579 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -159,7 +159,7 @@ int null_register_zoned_dev(struct nullb *nullb)
struct nullb_device *dev = nullb->dev;
struct request_queue *q = nullb->q;

- disk_set_zoned(nullb->disk, BLK_ZONED_HM);
+ disk_set_zoned(nullb->disk, true);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
blk_queue_chunk_sectors(q, dev->zone_size_sects);
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 83600b45e12a28..24fb95f19d5284 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -250,7 +250,7 @@ static int ublk_dev_param_zoned_apply(struct ublk_device *ub)
{
const struct ublk_param_zoned *p = &ub->params.zoned;

- disk_set_zoned(ub->ub_disk, BLK_ZONED_HM);
+ disk_set_zoned(ub->ub_disk, true);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, ub->ub_disk->queue);
blk_queue_required_elevator_features(ub->ub_disk->queue,
ELEVATOR_F_ZBD_SEQ_WRITE);
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index a28f1687066bb4..19a4f20bd1c2f8 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -730,7 +730,7 @@ static int virtblk_probe_zoned_device(struct virtio_device *vdev,

dev_dbg(&vdev->dev, "probing host-managed zoned device\n");

- disk_set_zoned(vblk->disk, BLK_ZONED_HM);
+ disk_set_zoned(vblk->disk, true);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);

virtio_cread(vdev, struct virtio_blk_config,
diff --git a/drivers/md/dm-kcopyd.c b/drivers/md/dm-kcopyd.c
index d01807c50f20b9..36bcfdccae0461 100644
--- a/drivers/md/dm-kcopyd.c
+++ b/drivers/md/dm-kcopyd.c
@@ -807,7 +807,7 @@ void dm_kcopyd_copy(struct dm_kcopyd_client *kc, struct dm_io_region *from,
*/
if (!(job->flags & BIT(DM_KCOPYD_WRITE_SEQ))) {
for (i = 0; i < job->num_dests; i++) {
- if (bdev_zoned_model(dests[i].bdev) == BLK_ZONED_HM) {
+ if (bdev_is_zoned(dests[i].bdev)) {
job->flags |= BIT(DM_KCOPYD_WRITE_SEQ);
break;
}
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 198d38b53322c1..260b5b8f2b0d7e 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1579,21 +1579,18 @@ bool dm_table_has_no_data_devices(struct dm_table *t)
return true;
}

-static int device_not_zoned_model(struct dm_target *ti, struct dm_dev *dev,
- sector_t start, sector_t len, void *data)
+static int device_not_zoned(struct dm_target *ti, struct dm_dev *dev,
+ sector_t start, sector_t len, void *data)
{
- struct request_queue *q = bdev_get_queue(dev->bdev);
- enum blk_zoned_model *zoned_model = data;
+ bool *zoned = data;

- return blk_queue_zoned_model(q) != *zoned_model;
+ return bdev_is_zoned(dev->bdev) != *zoned;
}

static int device_is_zoned_model(struct dm_target *ti, struct dm_dev *dev,
sector_t start, sector_t len, void *data)
{
- struct request_queue *q = bdev_get_queue(dev->bdev);
-
- return blk_queue_zoned_model(q) != BLK_ZONED_NONE;
+ return bdev_is_zoned(dev->bdev);
}

/*
@@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct dm_target *ti, struct dm_dev *dev,
* has the DM_TARGET_MIXED_ZONED_MODEL feature set, the devices can have any
* zoned model with all zoned devices having the same zone size.
*/
-static bool dm_table_supports_zoned_model(struct dm_table *t,
- enum blk_zoned_model zoned_model)
+static bool dm_table_supports_zoned(struct dm_table *t, bool zoned)
{
for (unsigned int i = 0; i < t->num_targets; i++) {
struct dm_target *ti = dm_table_get_target(t, i);
@@ -1623,11 +1619,11 @@ static bool dm_table_supports_zoned_model(struct dm_table *t,

if (dm_target_supports_zoned_hm(ti->type)) {
if (!ti->type->iterate_devices ||
- ti->type->iterate_devices(ti, device_not_zoned_model,
- &zoned_model))
+ ti->type->iterate_devices(ti, device_not_zoned,
+ &zoned))
return false;
} else if (!dm_target_supports_mixed_zoned_model(ti->type)) {
- if (zoned_model == BLK_ZONED_HM)
+ if (zoned)
return false;
}
}
@@ -1650,14 +1646,13 @@ static int device_not_matches_zone_sectors(struct dm_target *ti, struct dm_dev *
* zone sectors, if the destination device is a zoned block device, it shall
* have the specified zone_sectors.
*/
-static int validate_hardware_zoned_model(struct dm_table *t,
- enum blk_zoned_model zoned_model,
- unsigned int zone_sectors)
+static int validate_hardware_zoned(struct dm_table *t, bool zoned,
+ unsigned int zone_sectors)
{
- if (zoned_model == BLK_ZONED_NONE)
+ if (!zoned)
return 0;

- if (!dm_table_supports_zoned_model(t, zoned_model)) {
+ if (!dm_table_supports_zoned(t, zoned)) {
DMERR("%s: zoned model is not consistent across all devices",
dm_device_name(t->md));
return -EINVAL;
@@ -1683,8 +1678,8 @@ int dm_calculate_queue_limits(struct dm_table *t,
struct queue_limits *limits)
{
struct queue_limits ti_limits;
- enum blk_zoned_model zoned_model = BLK_ZONED_NONE;
unsigned int zone_sectors = 0;
+ bool zoned = false;

blk_set_stacking_limits(limits);

@@ -1706,12 +1701,12 @@ int dm_calculate_queue_limits(struct dm_table *t,
ti->type->iterate_devices(ti, dm_set_device_limits,
&ti_limits);

- if (zoned_model == BLK_ZONED_NONE && ti_limits.zoned != BLK_ZONED_NONE) {
+ if (!zoned && ti_limits.zoned) {
/*
* After stacking all limits, validate all devices
* in table support this zoned model and zone sectors.
*/
- zoned_model = ti_limits.zoned;
+ zoned = ti_limits.zoned;
zone_sectors = ti_limits.chunk_sectors;
}

@@ -1744,18 +1739,18 @@ int dm_calculate_queue_limits(struct dm_table *t,
* Verify that the zoned model and zone sectors, as determined before
* any .io_hints override, are the same across all devices in the table.
* - this is especially relevant if .io_hints is emulating a disk-managed
- * zoned model (aka BLK_ZONED_NONE) on host-managed zoned block devices.
+ * zoned model on host-managed zoned block devices.
* BUT...
*/
- if (limits->zoned != BLK_ZONED_NONE) {
+ if (limits->zoned) {
/*
* ...IF the above limits stacking determined a zoned model
* validate that all of the table's devices conform to it.
*/
- zoned_model = limits->zoned;
+ zoned = limits->zoned;
zone_sectors = limits->chunk_sectors;
}
- if (validate_hardware_zoned_model(t, zoned_model, zone_sectors))
+ if (validate_hardware_zoned(t, zoned, zone_sectors))
return -EINVAL;

return validate_hardware_logical_block_alignment(t, limits);
diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
index 60a4dc01ea187d..fdfe30f7b6973d 100644
--- a/drivers/md/dm-zoned-metadata.c
+++ b/drivers/md/dm-zoned-metadata.c
@@ -2836,12 +2836,11 @@ static void dmz_print_dev(struct dmz_metadata *zmd, int num)
{
struct dmz_dev *dev = &zmd->dev[num];

- if (bdev_zoned_model(dev->bdev) == BLK_ZONED_NONE)
+ if (!bdev_is_zoned(dev->bdev))
dmz_dev_info(dev, "Regular block device");
else
- dmz_dev_info(dev, "Host-%s zoned block device",
- bdev_zoned_model(dev->bdev) == BLK_ZONED_HA ?
- "aware" : "managed");
+ dmz_dev_info(dev, "Host-managed zoned block device");
+
if (zmd->sb_version > 1) {
sector_t sector_offset =
dev->zone_offset << zmd->zone_nr_sectors_shift;
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index b487f7acc860f7..621794a9edd65e 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -702,7 +702,7 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path,
}

bdev = ddev->bdev;
- if (bdev_zoned_model(bdev) == BLK_ZONED_NONE) {
+ if (!bdev_is_zoned(bdev)) {
if (nr_devs == 1) {
ti->error = "Invalid regular device";
goto err;
@@ -1010,7 +1010,7 @@ static void dmz_io_hints(struct dm_target *ti, struct queue_limits *limits)
limits->max_sectors = chunk_sectors;

/* We are exposing a drive-managed zoned block device */
- limits->zoned = BLK_ZONED_NONE;
+ limits->zoned = false;
}

/*
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index ec8557810c2102..6d4c440e97e2a5 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -108,7 +108,7 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
goto free_data;
}

- disk_set_zoned(ns->disk, BLK_ZONED_HM);
+ disk_set_zoned(ns->disk, true);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
disk_set_max_open_zones(ns->disk, le32_to_cpu(id->mor) + 1);
disk_set_max_active_zones(ns->disk, le32_to_cpu(id->mar) + 1);
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 6d8218a4412264..d03d66f1149301 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -339,7 +339,7 @@ struct sdebug_dev_info {
bool used;

/* For ZBC devices */
- enum blk_zoned_model zmodel;
+ bool zoned;
unsigned int zcap;
unsigned int zsize;
unsigned int zsize_shift;
@@ -844,8 +844,11 @@ static bool write_since_sync;
static bool sdebug_statistics = DEF_STATISTICS;
static bool sdebug_wp;
static bool sdebug_allow_restart;
-/* Following enum: 0: no zbc, def; 1: host aware; 2: host managed */
-static enum blk_zoned_model sdeb_zbc_model = BLK_ZONED_NONE;
+static enum {
+ BLK_ZONED_NONE = 0,
+ BLK_ZONED_HA = 1,
+ BLK_ZONED_HM = 2,
+} sdeb_zbc_model = BLK_ZONED_NONE;
static char *sdeb_zbc_model_s;

enum sam_lun_addr_method {SAM_LUN_AM_PERIPHERAL = 0x0,
@@ -1815,8 +1818,6 @@ static int inquiry_vpd_b1(struct sdebug_dev_info *devip, unsigned char *arr)
arr[1] = 1; /* non rotating medium (e.g. solid state) */
arr[2] = 0;
arr[3] = 5; /* less than 1.8" */
- if (devip->zmodel == BLK_ZONED_HA)
- arr[4] = 1 << 4; /* zoned field = 01b */

return 0x3c;
}
@@ -1883,7 +1884,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
if (! arr)
return DID_REQUEUE << 16;
is_disk = (sdebug_ptype == TYPE_DISK);
- is_zbc = (devip->zmodel != BLK_ZONED_NONE);
+ is_zbc = devip->zoned;
is_disk_zbc = (is_disk || is_zbc);
have_wlun = scsi_is_wlun(scp->device->lun);
if (have_wlun)
@@ -2195,7 +2196,7 @@ static int resp_readcap16(struct scsi_cmnd *scp,
* Since the scsi_debug READ CAPACITY implementation always reports the
* total disk capacity, set RC BASIS = 1 for host-managed ZBC devices.
*/
- if (devip->zmodel == BLK_ZONED_HM)
+ if (devip->zoned)
arr[12] |= 1 << 4;

arr[15] = sdebug_lowest_aligned & 0xff;
@@ -2648,7 +2649,7 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
msense_6 = (MODE_SENSE == cmd[0]);
llbaa = msense_6 ? false : !!(cmd[1] & 0x10);
is_disk = (sdebug_ptype == TYPE_DISK);
- is_zbc = (devip->zmodel != BLK_ZONED_NONE);
+ is_zbc = devip->zoned;
if ((is_disk || is_zbc) && !dbd)
bd_len = llbaa ? 16 : 8;
else
@@ -3194,8 +3195,6 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
struct sdeb_zone_state *zsp_end = zbc_zone(devip, lba + num - 1);

if (!write) {
- if (devip->zmodel == BLK_ZONED_HA)
- return 0;
/* For host-managed, reads cannot cross zone types boundaries */
if (zsp->z_type != zsp_end->z_type) {
mk_sense_buffer(scp, ILLEGAL_REQUEST,
@@ -5322,7 +5321,7 @@ static int sdebug_device_create_zones(struct sdebug_dev_info *devip)
if (devip->zcap < devip->zsize)
devip->nr_zones += devip->nr_seq_zones;

- if (devip->zmodel == BLK_ZONED_HM) {
+ if (devip->zoned) {
/* zbc_max_open_zones can be 0, meaning "not reported" */
if (sdeb_zbc_max_open >= devip->nr_zones - 1)
devip->max_open = (devip->nr_zones - 1) / 2;
@@ -5347,7 +5346,7 @@ static int sdebug_device_create_zones(struct sdebug_dev_info *devip)
zsp->z_size =
min_t(u64, devip->zsize, capacity - zstart);
} else if ((zstart & (devip->zsize - 1)) == 0) {
- if (devip->zmodel == BLK_ZONED_HM)
+ if (devip->zoned)
zsp->z_type = ZBC_ZTYPE_SWR;
else
zsp->z_type = ZBC_ZTYPE_SWP;
@@ -5390,13 +5389,13 @@ static struct sdebug_dev_info *sdebug_device_create(
}
devip->sdbg_host = sdbg_host;
if (sdeb_zbc_in_use) {
- devip->zmodel = sdeb_zbc_model;
+ devip->zoned = sdeb_zbc_model == BLK_ZONED_HM;
if (sdebug_device_create_zones(devip)) {
kfree(devip);
return NULL;
}
} else {
- devip->zmodel = BLK_ZONED_NONE;
+ devip->zoned = false;
}
devip->create_ts = ktime_get_boottime();
atomic_set(&devip->stopped, (sdeb_tur_ms_to_ready > 0 ? 2 : 0));
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index fa00dd503cbf61..19a19eb277f57d 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3117,7 +3117,6 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
struct request_queue *q = sdkp->disk->queue;
struct scsi_vpd *vpd;
u16 rot;
- u8 zoned;

rcu_read_lock();
vpd = rcu_dereference(sdkp->device->vpd_pgb1);
@@ -3128,7 +3127,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
}

rot = get_unaligned_be16(&vpd->data[4]);
- zoned = (vpd->data[8] >> 4) & 3;
+ sdkp->zoned = (vpd->data[8] >> 4) & 3;
rcu_read_unlock();

if (rot == 1) {
@@ -3138,37 +3137,33 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)

if (sdkp->device->type == TYPE_ZBC) {
/*
- * Host-managed: Per ZBC and ZAC specifications, writes in
- * sequential write required zones of host-managed devices must
- * be aligned to the device physical block size.
+ * Host-managed.
+ */
+ disk_set_zoned(sdkp->disk, true);
+
+ /*
+ * Per ZBC and ZAC specifications, writes in sequential write
+ * required zones of host-managed devices must be aligned to
+ * the device physical block size.
*/
- disk_set_zoned(sdkp->disk, BLK_ZONED_HM);
blk_queue_zone_write_granularity(q, sdkp->physical_block_size);
} else {
- sdkp->zoned = zoned;
- if (sdkp->zoned == 1) {
- /* Host-aware */
- disk_set_zoned(sdkp->disk, BLK_ZONED_HA);
- } else {
- /* Regular disk or drive managed disk */
- disk_set_zoned(sdkp->disk, BLK_ZONED_NONE);
- }
+ /*
+ * Anything else. This includes host-aware device that we treat
+ * as conventional.
+ */
+ disk_set_zoned(sdkp->disk, false);
}

if (!sdkp->first_scan)
return;

- if (blk_queue_is_zoned(q)) {
- sd_printk(KERN_NOTICE, sdkp, "Host-%s zoned block device\n",
- q->limits.zoned == BLK_ZONED_HM ? "managed" : "aware");
- } else {
- if (sdkp->zoned == 1)
- sd_printk(KERN_NOTICE, sdkp,
- "Host-aware SMR disk used as regular disk\n");
- else if (sdkp->zoned == 2)
- sd_printk(KERN_NOTICE, sdkp,
- "Drive-managed SMR disk\n");
- }
+ if (blk_queue_is_zoned(q))
+ sd_printk(KERN_NOTICE, sdkp, "Host-managed zoned block device\n");
+ else if (sdkp->zoned == 1)
+ sd_printk(KERN_NOTICE, sdkp, "Host-aware SMR disk used as regular disk\n");
+ else if (sdkp->zoned == 2)
+ sd_printk(KERN_NOTICE, sdkp, "Drive-managed SMR disk\n");
}

/**
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index a25215507668d8..26af5ab7d7c173 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -836,10 +836,7 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)

/*
* For all zoned disks, initialize zone append emulation data if not
- * already done. This is necessary also for host-aware disks used as
- * regular disks due to the presence of partitions as these partitions
- * may be deleted and the disk zoned model changed back from
- * BLK_ZONED_NONE to BLK_ZONED_HA.
+ * already done.
*/
if (sd_is_zoned(sdkp) && !sdkp->zone_wp_update_buf) {
ret = sd_zbc_init_disk(sdkp);
@@ -932,17 +929,6 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8 buf[SD_BUF_SIZE])
sdkp->device->use_10_for_rw = 0;
sdkp->device->use_16_for_sync = 1;

- if (!blk_queue_is_zoned(q)) {
- /*
- * This can happen for a host aware disk with partitions.
- * The block device zone model was already cleared by
- * disk_set_zoned(). Only free the scsi disk zone
- * information and exit early.
- */
- sd_zbc_free_zone_info(sdkp);
- return 0;
- }
-
/* Check zoned block device characteristics (unconstrained reads) */
ret = sd_zbc_check_zoned_characteristics(sdkp, buf);
if (ret)
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 188378ca19c7f6..23c1e6b19a653b 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -578,26 +578,12 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device, bool populate_cache)

kvfree(zones);

- switch (bdev_zoned_model(bdev)) {
- case BLK_ZONED_HM:
+ if (bdev_is_zoned(bdev)) {
model = "host-managed zoned";
emulated = "";
- break;
- case BLK_ZONED_HA:
- model = "host-aware zoned";
- emulated = "";
- break;
- case BLK_ZONED_NONE:
+ } else {
model = "regular";
emulated = "emulated ";
- break;
- default:
- /* Just in case */
- btrfs_err_in_rcu(fs_info, "zoned: unsupported model %d on %s",
- bdev_zoned_model(bdev),
- rcu_str_deref(device->name));
- ret = -EOPNOTSUPP;
- goto out_free_zone_info;
}

btrfs_info_in_rcu(fs_info,
@@ -609,9 +595,7 @@ int btrfs_get_dev_zone_info(struct btrfs_device *device, bool populate_cache)

out:
kvfree(zones);
-out_free_zone_info:
btrfs_destroy_dev_zone_info(device);
-
return ret;
}

@@ -688,8 +672,7 @@ static int btrfs_check_for_zoned_device(struct btrfs_fs_info *fs_info)
struct btrfs_device *device;

list_for_each_entry(device, &fs_info->fs_devices->devices, dev_list) {
- if (device->bdev &&
- bdev_zoned_model(device->bdev) == BLK_ZONED_HM) {
+ if (device->bdev && bdev_is_zoned(device->bdev)) {
btrfs_err(fs_info,
"zoned: mode not enabled but zoned device found: %pg",
device->bdev);
diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
index b9cec523b77842..bc1b540c1597ab 100644
--- a/fs/btrfs/zoned.h
+++ b/fs/btrfs/zoned.h
@@ -324,7 +324,7 @@ static inline bool btrfs_check_device_zone_type(const struct btrfs_fs_info *fs_i
}

/* Do not allow Host Manged zoned device */
- return bdev_zoned_model(bdev) != BLK_ZONED_HM;
+ return !bdev_is_zoned(bdev);
}

static inline bool btrfs_check_super_location(struct btrfs_device *device, u64 pos)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 4e42b5f24debec..9b62549a29ced0 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -995,7 +995,7 @@ static bool is_end_zone_blkaddr(struct f2fs_sb_info *sbi, block_t blkaddr)
}
blkaddr -= FDEV(devi).start_blk;
}
- return bdev_zoned_model(FDEV(devi).bdev) == BLK_ZONED_HM &&
+ return bdev_is_zoned(FDEV(devi).bdev) &&
f2fs_blkz_is_seq(sbi, devi, blkaddr) &&
(blkaddr % sbi->blocks_per_blkz == sbi->blocks_per_blkz - 1);
}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 033af907c3b1dc..850c87ae7d98ad 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4282,24 +4282,21 @@ static int f2fs_scan_devices(struct f2fs_sb_info *sbi)
sbi->aligned_blksize = false;

#ifdef CONFIG_BLK_DEV_ZONED
- if (bdev_zoned_model(FDEV(i).bdev) == BLK_ZONED_HM &&
- !f2fs_sb_has_blkzoned(sbi)) {
- f2fs_err(sbi, "Zoned block device feature not enabled");
- return -EINVAL;
- }
- if (bdev_zoned_model(FDEV(i).bdev) != BLK_ZONED_NONE) {
+ if (bdev_is_zoned(FDEV(i).bdev)) {
+ if (!f2fs_sb_has_blkzoned(sbi)) {
+ f2fs_err(sbi, "Zoned block device feature not enabled");
+ return -EINVAL;
+ }
if (init_blkz_info(sbi, i)) {
f2fs_err(sbi, "Failed to initialize F2FS blkzone information");
return -EINVAL;
}
if (max_devices == 1)
break;
- f2fs_info(sbi, "Mount Device [%2d]: %20s, %8u, %8x - %8x (zone: %s)",
+ f2fs_info(sbi, "Mount Device [%2d]: %20s, %8u, %8x - %8x (zone: Host-managed)",
i, FDEV(i).path,
FDEV(i).total_segments,
- FDEV(i).start_blk, FDEV(i).end_blk,
- bdev_zoned_model(FDEV(i).bdev) == BLK_ZONED_HA ?
- "Host-aware" : "Host-managed");
+ FDEV(i).start_blk, FDEV(i).end_blk);
continue;
}
#endif
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 185ed3770e3a96..28cda9fb239eb6 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -263,18 +263,6 @@ static inline bool blk_op_is_passthrough(blk_opf_t op)
return op == REQ_OP_DRV_IN || op == REQ_OP_DRV_OUT;
}

-/*
- * Zoned block device models (zoned limit).
- *
- * Note: This needs to be ordered from the least to the most severe
- * restrictions for the inheritance in blk_stack_limits() to work.
- */
-enum blk_zoned_model {
- BLK_ZONED_NONE = 0, /* Regular block device */
- BLK_ZONED_HA, /* Host-aware zoned block device */
- BLK_ZONED_HM, /* Host-managed zoned block device */
-};
-
/*
* BLK_BOUNCE_NONE: never bounce (default)
* BLK_BOUNCE_HIGH: bounce all highmem pages
@@ -316,7 +304,7 @@ struct queue_limits {
unsigned char misaligned;
unsigned char discard_misaligned;
unsigned char raid_partial_stripes_expensive;
- enum blk_zoned_model zoned;
+ bool zoned;

/*
* Drivers that set dma_alignment to less than 511 must be prepared to
@@ -329,7 +317,7 @@ struct queue_limits {
typedef int (*report_zones_cb)(struct blk_zone *zone, unsigned int idx,
void *data);

-void disk_set_zoned(struct gendisk *disk, enum blk_zoned_model model);
+void disk_set_zoned(struct gendisk *disk, bool zoned);

#define BLK_ALL_ZONES ((unsigned int)-1)
int blkdev_report_zones(struct block_device *bdev, sector_t sector,
@@ -617,23 +605,9 @@ static inline enum rpm_status queue_rpm_status(struct request_queue *q)
}
#endif

-static inline enum blk_zoned_model
-blk_queue_zoned_model(struct request_queue *q)
-{
- if (IS_ENABLED(CONFIG_BLK_DEV_ZONED))
- return q->limits.zoned;
- return BLK_ZONED_NONE;
-}
-
static inline bool blk_queue_is_zoned(struct request_queue *q)
{
- switch (blk_queue_zoned_model(q)) {
- case BLK_ZONED_HA:
- case BLK_ZONED_HM:
- return true;
- default:
- return false;
- }
+ return IS_ENABLED(CONFIG_BLK_DEV_ZONED) && q->limits.zoned;
}

#ifdef CONFIG_BLK_DEV_ZONED
@@ -1260,11 +1234,6 @@ static inline bool bdev_nowait(struct block_device *bdev)
return test_bit(QUEUE_FLAG_NOWAIT, &bdev_get_queue(bdev)->queue_flags);
}

-static inline enum blk_zoned_model bdev_zoned_model(struct block_device *bdev)
-{
- return blk_queue_zoned_model(bdev_get_queue(bdev));
-}
-
static inline bool bdev_is_zoned(struct block_device *bdev)
{
return blk_queue_is_zoned(bdev_get_queue(bdev));
--
2.39.2


2023-12-17 16:56:40

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 4/5] block: simplify disk_set_zoned

Only use disk_set_zoned to actually enable zoned device support.
For clearing it, call disk_clear_zoned, which is renamed from
disk_clear_zone_settings and now directly clears the zoned flag as
well.

Signed-off-by: Christoph Hellwig <[email protected]>
---
block/blk-settings.c | 32 +++++++++++---------------------
block/blk-zoned.c | 3 ++-
block/blk.h | 2 --
drivers/block/null_blk/zoned.c | 2 +-
drivers/block/ublk_drv.c | 2 +-
drivers/block/virtio_blk.c | 2 +-
drivers/nvme/host/zns.c | 2 +-
drivers/scsi/sd.c | 7 +++++--
include/linux/blkdev.h | 3 ++-
9 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 50e9efb59f67fd..bb94a3d471f4fb 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -881,31 +881,21 @@ bool blk_queue_can_use_dma_map_merging(struct request_queue *q,
EXPORT_SYMBOL_GPL(blk_queue_can_use_dma_map_merging);

/**
- * disk_set_zoned - configure the zoned model for a disk
- * @disk: the gendisk of the queue to configure
- * @zoned: zoned or not.
- *
- * When @zoned is %true, this should be called only if zoned block device
- * support is enabled (CONFIG_BLK_DEV_ZONED option).
+ * disk_set_zoned - inidicate a zoned device
+ * @disk: gendisk to configure
*/
-void disk_set_zoned(struct gendisk *disk, bool zoned)
+void disk_set_zoned(struct gendisk *disk)
{
struct request_queue *q = disk->queue;

- if (zoned) {
- WARN_ON_ONCE(!IS_ENABLED(CONFIG_BLK_DEV_ZONED));
-
- /*
- * Set the zone write granularity to the device logical block
- * size by default. The driver can change this value if needed.
- */
- q->limits.zoned = true;
- blk_queue_zone_write_granularity(q,
- queue_logical_block_size(q));
- } else if (q->limits.zoned) {
- q->limits.zoned = false;
- disk_clear_zone_settings(disk);
- }
+ WARN_ON_ONCE(!IS_ENABLED(CONFIG_BLK_DEV_ZONED));
+
+ /*
+ * Set the zone write granularity to the device logical block
+ * size by default. The driver can change this value if needed.
+ */
+ q->limits.zoned = true;
+ blk_queue_zone_write_granularity(q, queue_logical_block_size(q));
}
EXPORT_SYMBOL_GPL(disk_set_zoned);

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 619ee41a51cc8c..580a58e53efd77 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -616,12 +616,13 @@ int blk_revalidate_disk_zones(struct gendisk *disk,
}
EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones);

-void disk_clear_zone_settings(struct gendisk *disk)
+void disk_clear_zoned(struct gendisk *disk)
{
struct request_queue *q = disk->queue;

blk_mq_freeze_queue(q);

+ q->limits.zoned = false;
disk_free_zone_bitmaps(disk);
blk_queue_flag_clear(QUEUE_FLAG_ZONE_RESETALL, q);
q->required_elevator_features &= ~ELEVATOR_F_ZBD_SEQ_WRITE;
diff --git a/block/blk.h b/block/blk.h
index 08a358bc0919e2..1ef920f72e0f87 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -395,14 +395,12 @@ static inline struct bio *blk_queue_bounce(struct bio *bio,

#ifdef CONFIG_BLK_DEV_ZONED
void disk_free_zone_bitmaps(struct gendisk *disk);
-void disk_clear_zone_settings(struct gendisk *disk);
int blkdev_report_zones_ioctl(struct block_device *bdev, unsigned int cmd,
unsigned long arg);
int blkdev_zone_mgmt_ioctl(struct block_device *bdev, blk_mode_t mode,
unsigned int cmd, unsigned long arg);
#else /* CONFIG_BLK_DEV_ZONED */
static inline void disk_free_zone_bitmaps(struct gendisk *disk) {}
-static inline void disk_clear_zone_settings(struct gendisk *disk) {}
static inline int blkdev_report_zones_ioctl(struct block_device *bdev,
unsigned int cmd, unsigned long arg)
{
diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 369eb1e78bb579..6f5e0994862eae 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -159,7 +159,7 @@ int null_register_zoned_dev(struct nullb *nullb)
struct nullb_device *dev = nullb->dev;
struct request_queue *q = nullb->q;

- disk_set_zoned(nullb->disk, true);
+ disk_set_zoned(nullb->disk);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
blk_queue_chunk_sectors(q, dev->zone_size_sects);
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 24fb95f19d5284..d50d69b2c023de 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -250,7 +250,7 @@ static int ublk_dev_param_zoned_apply(struct ublk_device *ub)
{
const struct ublk_param_zoned *p = &ub->params.zoned;

- disk_set_zoned(ub->ub_disk, true);
+ disk_set_zoned(ub->ub_disk);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, ub->ub_disk->queue);
blk_queue_required_elevator_features(ub->ub_disk->queue,
ELEVATOR_F_ZBD_SEQ_WRITE);
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 19a4f20bd1c2f8..7d7a19b2b9a8eb 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -730,7 +730,7 @@ static int virtblk_probe_zoned_device(struct virtio_device *vdev,

dev_dbg(&vdev->dev, "probing host-managed zoned device\n");

- disk_set_zoned(vblk->disk, true);
+ disk_set_zoned(vblk->disk);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);

virtio_cread(vdev, struct virtio_blk_config,
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 6d4c440e97e2a5..3d98e435821e4a 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -108,7 +108,7 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
goto free_data;
}

- disk_set_zoned(ns->disk, true);
+ disk_set_zoned(ns->disk);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
disk_set_max_open_zones(ns->disk, le32_to_cpu(id->mor) + 1);
disk_set_max_active_zones(ns->disk, le32_to_cpu(id->mar) + 1);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 19a19eb277f57d..dbed075cdb981a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3135,11 +3135,13 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, q);
}

+
+#ifdef CONFIG_BLK_DEV_ZONED /* sd_probe rejects ZBD devices early otherwise */
if (sdkp->device->type == TYPE_ZBC) {
/*
* Host-managed.
*/
- disk_set_zoned(sdkp->disk, true);
+ disk_set_zoned(sdkp->disk);

/*
* Per ZBC and ZAC specifications, writes in sequential write
@@ -3152,8 +3154,9 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
* Anything else. This includes host-aware device that we treat
* as conventional.
*/
- disk_set_zoned(sdkp->disk, false);
+ disk_clear_zoned(sdkp->disk);
}
+#endif /* CONFIG_BLK_DEV_ZONED */

if (!sdkp->first_scan)
return;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 28cda9fb239eb6..bc236e77d85e1c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -317,7 +317,8 @@ struct queue_limits {
typedef int (*report_zones_cb)(struct blk_zone *zone, unsigned int idx,
void *data);

-void disk_set_zoned(struct gendisk *disk, bool zoned);
+void disk_set_zoned(struct gendisk *disk);
+void disk_clear_zoned(struct gendisk *disk);

#define BLK_ALL_ZONES ((unsigned int)-1)
int blkdev_report_zones(struct block_device *bdev, sector_t sector,
--
2.39.2


2023-12-17 16:57:24

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 5/5] sd: only call disk_clear_zoned when needed

disk_clear_zoned only needs to be called when a device reported zone
managed mode first and we clear it. Add a check so that disk_clear_zoned
isn't called on devices that were never zoned.

This avoids a fairly expensive queue freezing when revalidating
conventional devices.

Signed-off-by: Christoph Hellwig <[email protected]>
---
drivers/scsi/sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index dbed075cdb981a..8c8ac5cd1833b4 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3149,7 +3149,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
* the device physical block size.
*/
blk_queue_zone_write_granularity(q, sdkp->physical_block_size);
- } else {
+ } else if (blk_queue_is_zoned(q)) {
/*
* Anything else. This includes host-aware device that we treat
* as conventional.
--
2.39.2


2023-12-18 06:16:51

by Ed Tsai (蔡宗軒)

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

Hi Christoph,

some minor suggestions:

On Sun, 2023-12-17 at 17:53 +0100, Christoph Hellwig wrote:
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 198d38b53322c1..260b5b8f2b0d7e 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -1579,21 +1579,18 @@ bool dm_table_has_no_data_devices(struct
> dm_table *t)
> return true;
> }
>
> -static int device_not_zoned_model(struct dm_target *ti, struct
> dm_dev *dev,
> - sector_t start, sector_t len, void
> *data)
> +static int device_not_zoned(struct dm_target *ti, struct dm_dev
> *dev,
> + sector_t start, sector_t len, void *data)
> {
> - struct request_queue *q = bdev_get_queue(dev->bdev);
> - enum blk_zoned_model *zoned_model = data;
> + bool *zoned = data;
>
> - return blk_queue_zoned_model(q) != *zoned_model;
> + return bdev_is_zoned(dev->bdev) != *zoned;
> }
>
> static int device_is_zoned_model(struct dm_target *ti, struct dm_dev
> *dev,
> sector_t start, sector_t len, void
> *data)

Seems like the word "model" should also be remove here.

> {
> - struct request_queue *q = bdev_get_queue(dev->bdev);
> -
> - return blk_queue_zoned_model(q) != BLK_ZONED_NONE;
> + return bdev_is_zoned(dev->bdev);
> }
>
> /*
> @@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct
> dm_target *ti, struct dm_dev *dev,
> * has the DM_TARGET_MIXED_ZONED_MODEL feature set, the devices can
> have any
> * zoned model with all zoned devices having the same zone size.
> */
> -static bool dm_table_supports_zoned_model(struct dm_table *t,
> - enum blk_zoned_model
> zoned_model)
> +static bool dm_table_supports_zoned(struct dm_table *t, bool zoned)
> {
> for (unsigned int i = 0; i < t->num_targets; i++) {
> struct dm_target *ti = dm_table_get_target(t, i);
> @@ -1623,11 +1619,11 @@ static bool
> dm_table_supports_zoned_model(struct dm_table *t,
>
> if (dm_target_supports_zoned_hm(ti->type)) {
> if (!ti->type->iterate_devices ||
> - ti->type->iterate_devices(ti,
> device_not_zoned_model,
> - &zoned_model))
> + ti->type->iterate_devices(ti,
> device_not_zoned,
> + &zoned))
> return false;
> } else if (!dm_target_supports_mixed_zoned_model(ti-
> >type)) {
> - if (zoned_model == BLK_ZONED_HM)
> + if (zoned)
> return false;
> }
> }

The parameter "bool zoned" is redundant. It should be removed from the
above 3 functions

Additionally, because we no longer need to distinguish the zoned model
here, DM_TARGET_MIXED_ZONED_MODEL is meaningless. We can also clean up
its related code.

2023-12-18 06:53:31

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On 2023/12/18 15:15, Ed Tsai (蔡宗軒) wrote:
> Hi Christoph,
>
> some minor suggestions:
>
> On Sun, 2023-12-17 at 17:53 +0100, Christoph Hellwig wrote:
>>
>> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
>> index 198d38b53322c1..260b5b8f2b0d7e 100644
>> --- a/drivers/md/dm-table.c
>> +++ b/drivers/md/dm-table.c
>> @@ -1579,21 +1579,18 @@ bool dm_table_has_no_data_devices(struct
>> dm_table *t)
>> return true;
>> }
>>
>> -static int device_not_zoned_model(struct dm_target *ti, struct
>> dm_dev *dev,
>> - sector_t start, sector_t len, void
>> *data)
>> +static int device_not_zoned(struct dm_target *ti, struct dm_dev
>> *dev,
>> + sector_t start, sector_t len, void *data)
>> {
>> - struct request_queue *q = bdev_get_queue(dev->bdev);
>> - enum blk_zoned_model *zoned_model = data;
>> + bool *zoned = data;
>>
>> - return blk_queue_zoned_model(q) != *zoned_model;
>> + return bdev_is_zoned(dev->bdev) != *zoned;
>> }
>>
>> static int device_is_zoned_model(struct dm_target *ti, struct dm_dev
>> *dev,
>> sector_t start, sector_t len, void
>> *data)
>
> Seems like the word "model" should also be remove here.
>
>> {
>> - struct request_queue *q = bdev_get_queue(dev->bdev);
>> -
>> - return blk_queue_zoned_model(q) != BLK_ZONED_NONE;
>> + return bdev_is_zoned(dev->bdev);
>> }
>>
>> /*
>> @@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct
>> dm_target *ti, struct dm_dev *dev,
>> * has the DM_TARGET_MIXED_ZONED_MODEL feature set, the devices can
>> have any
>> * zoned model with all zoned devices having the same zone size.
>> */
>> -static bool dm_table_supports_zoned_model(struct dm_table *t,
>> - enum blk_zoned_model
>> zoned_model)
>> +static bool dm_table_supports_zoned(struct dm_table *t, bool zoned)
>> {
>> for (unsigned int i = 0; i < t->num_targets; i++) {
>> struct dm_target *ti = dm_table_get_target(t, i);
>> @@ -1623,11 +1619,11 @@ static bool
>> dm_table_supports_zoned_model(struct dm_table *t,
>>
>> if (dm_target_supports_zoned_hm(ti->type)) {
>> if (!ti->type->iterate_devices ||
>> - ti->type->iterate_devices(ti,
>> device_not_zoned_model,
>> - &zoned_model))
>> + ti->type->iterate_devices(ti,
>> device_not_zoned,
>> + &zoned))
>> return false;
>> } else if (!dm_target_supports_mixed_zoned_model(ti-
>>> type)) {
>> - if (zoned_model == BLK_ZONED_HM)
>> + if (zoned)
>> return false;
>> }
>> }
>
> The parameter "bool zoned" is redundant. It should be removed from the
> above 3 functions
>
> Additionally, because we no longer need to distinguish the zoned model
> here, DM_TARGET_MIXED_ZONED_MODEL is meaningless. We can also clean up
> its related code.

Nope. The mixed thing is for mixing up non-zoned with zoned models.
For the entire DM code, HM and HA are both treated as HM-like zoned.

--
Damien Le Moal
Western Digital Research


2023-12-18 08:21:56

by Ed Tsai (蔡宗軒)

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On Mon, 2023-12-18 at 15:53 +0900, Damien Le Moal wrote:
> On 2023/12/18 15:15, Ed Tsai (蔡宗軒) wrote:
> > Hi Christoph,
> >
> > some minor suggestions:
> >
> > On Sun, 2023-12-17 at 17:53 +0100, Christoph Hellwig wrote:
> >>
> >> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> >> index 198d38b53322c1..260b5b8f2b0d7e 100644
> >> --- a/drivers/md/dm-table.c
> >> +++ b/drivers/md/dm-table.c
> >> @@ -1579,21 +1579,18 @@ bool dm_table_has_no_data_devices(struct
> >> dm_table *t)
> >> return true;
> >> }
> >>
> >> -static int device_not_zoned_model(struct dm_target *ti, struct
> >> dm_dev *dev,
> >> - sector_t start, sector_t len, void
> >> *data)
> >> +static int device_not_zoned(struct dm_target *ti, struct dm_dev
> >> *dev,
> >> + sector_t start, sector_t len, void *data)
> >> {
> >> -struct request_queue *q = bdev_get_queue(dev->bdev);
> >> -enum blk_zoned_model *zoned_model = data;
> >> +bool *zoned = data;
> >>
> >> -return blk_queue_zoned_model(q) != *zoned_model;
> >> +return bdev_is_zoned(dev->bdev) != *zoned;
> >> }
> >>
> >> static int device_is_zoned_model(struct dm_target *ti, struct
> dm_dev
> >> *dev,
> >> sector_t start, sector_t len, void
> >> *data)
> >
> > Seems like the word "model" should also be remove here.
> >
> >> {
> >> -struct request_queue *q = bdev_get_queue(dev->bdev);
> >> -
> >> -return blk_queue_zoned_model(q) != BLK_ZONED_NONE;
> >> +return bdev_is_zoned(dev->bdev);
> >> }
> >>
> >> /*
> >> @@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct
> >> dm_target *ti, struct dm_dev *dev,
> >> * has the DM_TARGET_MIXED_ZONED_MODEL feature set, the devices
> can
> >> have any
> >> * zoned model with all zoned devices having the same zone size.
> >> */
> >> -static bool dm_table_supports_zoned_model(struct dm_table *t,
> >> - enum blk_zoned_model
> >> zoned_model)
> >> +static bool dm_table_supports_zoned(struct dm_table *t, bool
> zoned)
> >> {
> >> for (unsigned int i = 0; i < t->num_targets; i++) {
> >> struct dm_target *ti = dm_table_get_target(t, i);
> >> @@ -1623,11 +1619,11 @@ static bool
> >> dm_table_supports_zoned_model(struct dm_table *t,
> >>
> >> if (dm_target_supports_zoned_hm(ti->type)) {
> >> if (!ti->type->iterate_devices ||
> >> - ti->type->iterate_devices(ti,
> >> device_not_zoned_model,
> >> - &zoned_model))
> >> + ti->type->iterate_devices(ti,
> >> device_not_zoned,
> >> + &zoned))
> >> return false;
> >> } else if (!dm_target_supports_mixed_zoned_model(ti-
> >>> type)) {
> >> -if (zoned_model == BLK_ZONED_HM)
> >> +if (zoned)
> >> return false;
> >> }
> >> }
> >
> > The parameter "bool zoned" is redundant. It should be removed from
> the
> > above 3 functions

The two func, is zoned and not zoned, are essentially the same. They
can be simplified into one function.

> >
> > Additionally, because we no longer need to distinguish the zoned
> model
> > here, DM_TARGET_MIXED_ZONED_MODEL is meaningless. We can also clean
> up
> > its related code.
>
> Nope. The mixed thing is for mixing up non-zoned with zoned models.
> For the entire DM code, HM and HA are both treated as HM-like zoned.
>
> --
> Damien Le Moal
> Western Digital Research

Thank you. I have some misunderstanding. Please disregard it.

2023-12-18 09:33:55

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On 12/18/23 17:21, Ed Tsai (蔡宗軒) wrote:
> On Mon, 2023-12-18 at 15:53 +0900, Damien Le Moal wrote:
>> On 2023/12/18 15:15, Ed Tsai (蔡宗軒) wrote:
>>> Hi Christoph,
>>>
>>> some minor suggestions:
>>>
>>> On Sun, 2023-12-17 at 17:53 +0100, Christoph Hellwig wrote:
>>>>
>>>> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
>>>> index 198d38b53322c1..260b5b8f2b0d7e 100644
>>>> --- a/drivers/md/dm-table.c
>>>> +++ b/drivers/md/dm-table.c
>>>> @@ -1579,21 +1579,18 @@ bool dm_table_has_no_data_devices(struct
>>>> dm_table *t)
>>>> return true;
>>>> }
>>>>
>>>> -static int device_not_zoned_model(struct dm_target *ti, struct
>>>> dm_dev *dev,
>>>> - sector_t start, sector_t len, void
>>>> *data)
>>>> +static int device_not_zoned(struct dm_target *ti, struct dm_dev
>>>> *dev,
>>>> + sector_t start, sector_t len, void *data)
>>>> {
>>>> -struct request_queue *q = bdev_get_queue(dev->bdev);
>>>> -enum blk_zoned_model *zoned_model = data;
>>>> +bool *zoned = data;
>>>>
>>>> -return blk_queue_zoned_model(q) != *zoned_model;
>>>> +return bdev_is_zoned(dev->bdev) != *zoned;
>>>> }
>>>>
>>>> static int device_is_zoned_model(struct dm_target *ti, struct
>> dm_dev
>>>> *dev,
>>>> sector_t start, sector_t len, void
>>>> *data)
>>>
>>> Seems like the word "model" should also be remove here.
>>>
>>>> {
>>>> -struct request_queue *q = bdev_get_queue(dev->bdev);
>>>> -
>>>> -return blk_queue_zoned_model(q) != BLK_ZONED_NONE;
>>>> +return bdev_is_zoned(dev->bdev);
>>>> }
>>>>
>>>> /*
>>>> @@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct
>>>> dm_target *ti, struct dm_dev *dev,
>>>> * has the DM_TARGET_MIXED_ZONED_MODEL feature set, the devices
>> can
>>>> have any
>>>> * zoned model with all zoned devices having the same zone size.
>>>> */
>>>> -static bool dm_table_supports_zoned_model(struct dm_table *t,
>>>> - enum blk_zoned_model
>>>> zoned_model)
>>>> +static bool dm_table_supports_zoned(struct dm_table *t, bool
>> zoned)
>>>> {
>>>> for (unsigned int i = 0; i < t->num_targets; i++) {
>>>> struct dm_target *ti = dm_table_get_target(t, i);
>>>> @@ -1623,11 +1619,11 @@ static bool
>>>> dm_table_supports_zoned_model(struct dm_table *t,
>>>>
>>>> if (dm_target_supports_zoned_hm(ti->type)) {
>>>> if (!ti->type->iterate_devices ||
>>>> - ti->type->iterate_devices(ti,
>>>> device_not_zoned_model,
>>>> - &zoned_model))
>>>> + ti->type->iterate_devices(ti,
>>>> device_not_zoned,
>>>> + &zoned))
>>>> return false;
>>>> } else if (!dm_target_supports_mixed_zoned_model(ti-
>>>>> type)) {
>>>> -if (zoned_model == BLK_ZONED_HM)
>>>> +if (zoned)
>>>> return false;
>>>> }
>>>> }
>>>
>>> The parameter "bool zoned" is redundant. It should be removed from
>> the
>>> above 3 functions
>
> The two func, is zoned and not zoned, are essentially the same. They
> can be simplified into one function.

Maybe... But that needs testing/checking. I added the one because I could not
reuse the other given what is being tested.

>
>>>
>>> Additionally, because we no longer need to distinguish the zoned
>> model
>>> here, DM_TARGET_MIXED_ZONED_MODEL is meaningless. We can also clean
>> up
>>> its related code.
>>
>> Nope. The mixed thing is for mixing up non-zoned with zoned models.
>> For the entire DM code, HM and HA are both treated as HM-like zoned.
>>
>> --
>> Damien Le Moal
>> Western Digital Research
>
> Thank you. I have some misunderstanding. Please disregard it.

--
Damien Le Moal
Western Digital Research


2023-12-18 09:35:50

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 1/5] virtio_blk: cleanup zoned device probing

On 2023/12/18 1:53, Christoph Hellwig wrote:
> Move reading and checking the zoned model from virtblk_probe_zoned_device
> into the caller, leaving only the code to perform the actual setup for
> host managed zoned devices in virtblk_probe_zoned_device.
>
> This allows to share the model reading and sharing between builds with
> and without CONFIG_BLK_DEV_ZONED, and improve it for the
> !CONFIG_BLK_DEV_ZONED case.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks good.
Reviewed-by: Damien Le Moal <[email protected]>

> ---
> drivers/block/virtio_blk.c | 50 +++++++++++++++++---------------------
> 1 file changed, 22 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index d53d6aa8ee69a4..aeead732a24dc9 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -748,22 +748,6 @@ static int virtblk_probe_zoned_device(struct virtio_device *vdev,
> struct request_queue *q)
> {
> u32 v, wg;
> - u8 model;
> -
> - virtio_cread(vdev, struct virtio_blk_config,
> - zoned.model, &model);
> -
> - switch (model) {
> - case VIRTIO_BLK_Z_NONE:
> - case VIRTIO_BLK_Z_HA:
> - /* Present the host-aware device as non-zoned */
> - return 0;
> - case VIRTIO_BLK_Z_HM:
> - break;
> - default:
> - dev_err(&vdev->dev, "unsupported zone model %d\n", model);
> - return -EINVAL;
> - }
>
> dev_dbg(&vdev->dev, "probing host-managed zoned device\n");
>
> @@ -846,16 +830,9 @@ static inline void virtblk_revalidate_zones(struct virtio_blk *vblk)
> static inline int virtblk_probe_zoned_device(struct virtio_device *vdev,
> struct virtio_blk *vblk, struct request_queue *q)
> {
> - u8 model;
> -
> - virtio_cread(vdev, struct virtio_blk_config, zoned.model, &model);
> - if (model == VIRTIO_BLK_Z_HM) {
> - dev_err(&vdev->dev,
> - "virtio_blk: zoned devices are not supported");
> - return -EOPNOTSUPP;
> - }
> -
> - return 0;
> + dev_err(&vdev->dev,
> + "virtio_blk: zoned devices are not supported");
> + return -EOPNOTSUPP;
> }
> #endif /* CONFIG_BLK_DEV_ZONED */
>
> @@ -1570,9 +1547,26 @@ static int virtblk_probe(struct virtio_device *vdev)
> * placed after the virtio_device_ready() call above.
> */
> if (virtio_has_feature(vdev, VIRTIO_BLK_F_ZONED)) {
> - err = virtblk_probe_zoned_device(vdev, vblk, q);
> - if (err)
> + u8 model;
> +
> + virtio_cread(vdev, struct virtio_blk_config, zoned.model,
> + &model);
> + switch (model) {
> + case VIRTIO_BLK_Z_NONE:
> + case VIRTIO_BLK_Z_HA:
> + /* Present the host-aware device as non-zoned */
> + break;
> + case VIRTIO_BLK_Z_HM:
> + err = virtblk_probe_zoned_device(vdev, vblk, q);
> + if (err)
> + goto out_cleanup_disk;
> + break;
> + default:
> + dev_err(&vdev->dev, "unsupported zone model %d\n",
> + model);
> + err = -EINVAL;
> goto out_cleanup_disk;
> + }
> }
>
> err = device_add_disk(&vdev->dev, vblk->disk, virtblk_attr_groups);

--
Damien Le Moal
Western Digital Research


2023-12-18 09:37:23

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 2/5] virtio_blk: remove the broken zone revalidation support

On 2023/12/18 1:53, Christoph Hellwig wrote:
> virtblk_revalidate_zones is called unconditionally from
> virtblk_config_changed_work from the virtio config_changed callback.
>
> virtblk_revalidate_zones is a bit odd in that it re-clears the zoned
> state for host aware or non-zoned devices, which isn't needed unless the
> zoned mode changed - but a zone mode change to a host managed model isn't
> handled at all, and virtio_blk also doesn't handle any other config
> change except for a capacity change is handled (and even if it was
> the upper layers above virtio_blk wouldn't handle it very well).
>
> But even the useful case of a size change that would add or remove
> zones isn't handled properly as blk_revalidate_disk_zones expects the
> device capacity to cover all zones, but the capacity is only updated
> after virtblk_revalidate_zones.
>
> As this code appears to be entirely untested and is getting in the way
> remove it for now, but it can be readded in a fixed version with
> proper test coverage if needed.
>
> Fixes: 95bfec41bd3d ("virtio-blk: add support for zoned block devices")
> Fixes: f1ba4e674feb ("virtio-blk: fix to match virtio spec")
> Signed-off-by: Christoph Hellwig <[email protected]>

Not ideal... But I think this is OK for now given that as you say, the upper
layer will not be able to handle zone changes anyway.

Reviewed-by: Damien Le Moal <[email protected]>

> ---
> drivers/block/virtio_blk.c | 26 --------------------------
> 1 file changed, 26 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index aeead732a24dc9..a28f1687066bb4 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -722,27 +722,6 @@ static int virtblk_report_zones(struct gendisk *disk, sector_t sector,
> return ret;
> }
>
> -static void virtblk_revalidate_zones(struct virtio_blk *vblk)
> -{
> - u8 model;
> -
> - virtio_cread(vblk->vdev, struct virtio_blk_config,
> - zoned.model, &model);
> - switch (model) {
> - default:
> - dev_err(&vblk->vdev->dev, "unknown zone model %d\n", model);
> - fallthrough;
> - case VIRTIO_BLK_Z_NONE:
> - case VIRTIO_BLK_Z_HA:
> - disk_set_zoned(vblk->disk, BLK_ZONED_NONE);
> - return;
> - case VIRTIO_BLK_Z_HM:
> - WARN_ON_ONCE(!vblk->zone_sectors);
> - if (!blk_revalidate_disk_zones(vblk->disk, NULL))
> - set_capacity_and_notify(vblk->disk, 0);
> - }
> -}
> -
> static int virtblk_probe_zoned_device(struct virtio_device *vdev,
> struct virtio_blk *vblk,
> struct request_queue *q)
> @@ -823,10 +802,6 @@ static int virtblk_probe_zoned_device(struct virtio_device *vdev,
> */
> #define virtblk_report_zones NULL
>
> -static inline void virtblk_revalidate_zones(struct virtio_blk *vblk)
> -{
> -}
> -
> static inline int virtblk_probe_zoned_device(struct virtio_device *vdev,
> struct virtio_blk *vblk, struct request_queue *q)
> {
> @@ -982,7 +957,6 @@ static void virtblk_config_changed_work(struct work_struct *work)
> struct virtio_blk *vblk =
> container_of(work, struct virtio_blk, config_work);
>
> - virtblk_revalidate_zones(vblk);
> virtblk_update_capacity(vblk, true);
> }
>

--
Damien Le Moal
Western Digital Research


2023-12-18 09:49:02

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On 2023/12/18 1:53, Christoph Hellwig wrote:
> When zones were first added the SCSI and ATA specs, two different
> models were supported (in addition to the drive managed one that
> is invisible to the host):
>
> - host managed where non-conventional zones there is strict requirement
> to write at the write pointer, or else an error is returned
> - host aware where a write point is maintained if writes always happen
> at it, otherwise it is left in an under-defined state and the
> sequential write preferred zones behave like conventional zones
> (probably very badly performing ones, though)
>
> Not surprisingly this lukewarm model didn't prove to be very useful and
> was finally removed from the ZBC and SBC specs (NVMe never implemented
> it). Due to to the easily disappearing write pointer host software
> could never rely on the write pointer to actually be useful for say
> recovery.
>
> Fortunately only a few HDD prototypes shipped using this model which
> never made it to mass production. Drop the support before it is too
> late. Note that any such host aware prototype HDD can still be used
> with Linux as we'll now treat it as a conventional HDD.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

[...]

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 6d8218a4412264..d03d66f1149301 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -339,7 +339,7 @@ struct sdebug_dev_info {
> bool used;
>
> /* For ZBC devices */
> - enum blk_zoned_model zmodel;
> + bool zoned;
> unsigned int zcap;
> unsigned int zsize;
> unsigned int zsize_shift;
> @@ -844,8 +844,11 @@ static bool write_since_sync;
> static bool sdebug_statistics = DEF_STATISTICS;
> static bool sdebug_wp;
> static bool sdebug_allow_restart;
> -/* Following enum: 0: no zbc, def; 1: host aware; 2: host managed */
> -static enum blk_zoned_model sdeb_zbc_model = BLK_ZONED_NONE;
> +static enum {
> + BLK_ZONED_NONE = 0,
> + BLK_ZONED_HA = 1,
> + BLK_ZONED_HM = 2,
> +} sdeb_zbc_model = BLK_ZONED_NONE;
> static char *sdeb_zbc_model_s;
>
> enum sam_lun_addr_method {SAM_LUN_AM_PERIPHERAL = 0x0,
> @@ -1815,8 +1818,6 @@ static int inquiry_vpd_b1(struct sdebug_dev_info *devip, unsigned char *arr)
> arr[1] = 1; /* non rotating medium (e.g. solid state) */
> arr[2] = 0;
> arr[3] = 5; /* less than 1.8" */
> - if (devip->zmodel == BLK_ZONED_HA)
> - arr[4] = 1 << 4; /* zoned field = 01b */

I think we should keep everything related to HA in scsi debug as that is an easy
way to test the block layer and scsi. no ?

Other than this, very nice cleanup !

--
Damien Le Moal
Western Digital Research


2023-12-18 09:50:27

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 4/5] block: simplify disk_set_zoned

On 2023/12/18 1:53, Christoph Hellwig wrote:
> Only use disk_set_zoned to actually enable zoned device support.
> For clearing it, call disk_clear_zoned, which is renamed from
> disk_clear_zone_settings and now directly clears the zoned flag as
> well.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by: Damien Le Moal <[email protected]>

--
Damien Le Moal
Western Digital Research


2023-12-18 09:51:57

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 5/5] sd: only call disk_clear_zoned when needed

On 2023/12/18 1:53, Christoph Hellwig wrote:
> disk_clear_zoned only needs to be called when a device reported zone
> managed mode first and we clear it. Add a check so that disk_clear_zoned
> isn't called on devices that were never zoned.
>
> This avoids a fairly expensive queue freezing when revalidating
> conventional devices.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks good.

Reviewed-by: Damien Le Moal <[email protected]>


--
Damien Le Moal
Western Digital Research


2023-12-18 14:33:33

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On Mon, Dec 18, 2023 at 06:48:43PM +0900, Damien Le Moal wrote:
> > - if (devip->zmodel == BLK_ZONED_HA)
> > - arr[4] = 1 << 4; /* zoned field = 01b */
>
> I think we should keep everything related to HA in scsi debug as that is an easy
> way to test the block layer and scsi. no ?

Yes.

2023-12-18 15:16:37

by Stefan Hajnoczi

[permalink] [raw]
Subject: Re: [PATCH 1/5] virtio_blk: cleanup zoned device probing

On Sun, Dec 17, 2023 at 05:53:55PM +0100, Christoph Hellwig wrote:
> Move reading and checking the zoned model from virtblk_probe_zoned_device
> into the caller, leaving only the code to perform the actual setup for
> host managed zoned devices in virtblk_probe_zoned_device.
>
> This allows to share the model reading and sharing between builds with
> and without CONFIG_BLK_DEV_ZONED, and improve it for the
> !CONFIG_BLK_DEV_ZONED case.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/block/virtio_blk.c | 50 +++++++++++++++++---------------------
> 1 file changed, 22 insertions(+), 28 deletions(-)

Reviewed-by: Stefan Hajnoczi <[email protected]>


Attachments:
(No filename) (692.00 B)
signature.asc (499.00 B)
Download all attachments

2023-12-18 15:17:36

by Stefan Hajnoczi

[permalink] [raw]
Subject: Re: [PATCH 2/5] virtio_blk: remove the broken zone revalidation support

On Sun, Dec 17, 2023 at 05:53:56PM +0100, Christoph Hellwig wrote:
> virtblk_revalidate_zones is called unconditionally from
> virtblk_config_changed_work from the virtio config_changed callback.
>
> virtblk_revalidate_zones is a bit odd in that it re-clears the zoned
> state for host aware or non-zoned devices, which isn't needed unless the
> zoned mode changed - but a zone mode change to a host managed model isn't
> handled at all, and virtio_blk also doesn't handle any other config
> change except for a capacity change is handled (and even if it was
> the upper layers above virtio_blk wouldn't handle it very well).
>
> But even the useful case of a size change that would add or remove
> zones isn't handled properly as blk_revalidate_disk_zones expects the
> device capacity to cover all zones, but the capacity is only updated
> after virtblk_revalidate_zones.
>
> As this code appears to be entirely untested and is getting in the way
> remove it for now, but it can be readded in a fixed version with
> proper test coverage if needed.
>
> Fixes: 95bfec41bd3d ("virtio-blk: add support for zoned block devices")
> Fixes: f1ba4e674feb ("virtio-blk: fix to match virtio spec")
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> drivers/block/virtio_blk.c | 26 --------------------------
> 1 file changed, 26 deletions(-)

Fair enough.

Reviewed-by: Stefan Hajnoczi <[email protected]>


Attachments:
(No filename) (1.41 kB)
signature.asc (499.00 B)
Download all attachments

2023-12-19 02:17:41

by Martin K. Petersen

[permalink] [raw]
Subject: Re: remove support for the host aware zoned model


Christoph,

> Fortunately only a few HDD prototypes shipped using this model which
> never made it to mass production. Drop the support before it is too
> late. Note that any such host aware prototype HDD can still be used
> with Linux as we'll now treat it as a conventional HDD.

Looks good to me!

Reviewed-by: Martin K. Petersen <[email protected]>

--
Martin K. Petersen Oracle Linux Engineering

2023-12-19 07:26:46

by Naohiro Aota

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On Mon, Dec 18, 2023 at 08:21:22AM +0000, Ed Tsai (蔡宗軒) wrote:
> On Mon, 2023-12-18 at 15:53 +0900, Damien Le Moal wrote:
> > On 2023/12/18 15:15, Ed Tsai (蔡宗軒) wrote:
> > > Hi Christoph,
> > >
> > > some minor suggestions:
> > >
> > > On Sun, 2023-12-17 at 17:53 +0100, Christoph Hellwig wrote:
> > >>
> > >> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> > >> index 198d38b53322c1..260b5b8f2b0d7e 100644
> > >> --- a/drivers/md/dm-table.c
> > >> +++ b/drivers/md/dm-table.c
> > >> @@ -1579,21 +1579,18 @@ bool dm_table_has_no_data_devices(struct
> > >> dm_table *t)
> > >> return true;
> > >> }
> > >>
> > >> -static int device_not_zoned_model(struct dm_target *ti, struct
> > >> dm_dev *dev,
> > >> - sector_t start, sector_t len, void
> > >> *data)
> > >> +static int device_not_zoned(struct dm_target *ti, struct dm_dev
> > >> *dev,
> > >> + sector_t start, sector_t len, void *data)
> > >> {
> > >> -struct request_queue *q = bdev_get_queue(dev->bdev);
> > >> -enum blk_zoned_model *zoned_model = data;
> > >> +bool *zoned = data;
> > >>
> > >> -return blk_queue_zoned_model(q) != *zoned_model;
> > >> +return bdev_is_zoned(dev->bdev) != *zoned;
> > >> }
> > >>
> > >> static int device_is_zoned_model(struct dm_target *ti, struct
> > dm_dev
> > >> *dev,
> > >> sector_t start, sector_t len, void
> > >> *data)
> > >
> > > Seems like the word "model" should also be remove here.
> > >
> > >> {
> > >> -struct request_queue *q = bdev_get_queue(dev->bdev);
> > >> -
> > >> -return blk_queue_zoned_model(q) != BLK_ZONED_NONE;
> > >> +return bdev_is_zoned(dev->bdev);
> > >> }
> > >>
> > >> /*
> > >> @@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct
> > >> dm_target *ti, struct dm_dev *dev,
> > >> * has the DM_TARGET_MIXED_ZONED_MODEL feature set, the devices
> > can
> > >> have any
> > >> * zoned model with all zoned devices having the same zone size.
> > >> */
> > >> -static bool dm_table_supports_zoned_model(struct dm_table *t,
> > >> - enum blk_zoned_model
> > >> zoned_model)
> > >> +static bool dm_table_supports_zoned(struct dm_table *t, bool
> > zoned)
> > >> {
> > >> for (unsigned int i = 0; i < t->num_targets; i++) {
> > >> struct dm_target *ti = dm_table_get_target(t, i);
> > >> @@ -1623,11 +1619,11 @@ static bool
> > >> dm_table_supports_zoned_model(struct dm_table *t,
> > >>
> > >> if (dm_target_supports_zoned_hm(ti->type)) {
> > >> if (!ti->type->iterate_devices ||
> > >> - ti->type->iterate_devices(ti,
> > >> device_not_zoned_model,
> > >> - &zoned_model))
> > >> + ti->type->iterate_devices(ti,
> > >> device_not_zoned,
> > >> + &zoned))
> > >> return false;
> > >> } else if (!dm_target_supports_mixed_zoned_model(ti-
> > >>> type)) {
> > >> -if (zoned_model == BLK_ZONED_HM)
> > >> +if (zoned)
> > >> return false;
> > >> }
> > >> }
> > >
> > > The parameter "bool zoned" is redundant. It should be removed from
> > the
> > > above 3 functions
>
> The two func, is zoned and not zoned, are essentially the same. They
> can be simplified into one function.

Both functions are used for iterate_devices's callback in
dm_table_supports_zoned_model(). As shown in raid_iterate_devices(),
iterate_devices() returns 0 if the callback func calls on all the devices
returns 0, or returns a non-zero result early otherwise. So, the
iterate_devices() call returns "true" if any one of the underlying devices
is (zoned|not zoned).

Since we cannot create lambda as in other fancy languages, we need two
functions...

2023-12-19 08:08:36

by Ed Tsai (蔡宗軒)

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On Tue, 2023-12-19 at 07:16 +0000, Naohiro Aota wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On Mon, Dec 18, 2023 at 08:21:22AM +0000, Ed Tsai (蔡宗軒) wrote:
> > On Mon, 2023-12-18 at 15:53 +0900, Damien Le Moal wrote:
> > > On 2023/12/18 15:15, Ed Tsai (蔡宗軒) wrote:
> > > > Hi Christoph,
> > > >
> > > > some minor suggestions:
> > > >
> > > > On Sun, 2023-12-17 at 17:53 +0100, Christoph Hellwig wrote:
> > > >>
> > > >> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> > > >> index 198d38b53322c1..260b5b8f2b0d7e 100644
> > > >> --- a/drivers/md/dm-table.c
> > > >> +++ b/drivers/md/dm-table.c
> > > >> @@ -1579,21 +1579,18 @@ bool
> dm_table_has_no_data_devices(struct
> > > >> dm_table *t)
> > > >> return true;
> > > >> }
> > > >>
> > > >> -static int device_not_zoned_model(struct dm_target *ti,
> struct
> > > >> dm_dev *dev,
> > > >> - sector_t start, sector_t len, void
> > > >> *data)
> > > >> +static int device_not_zoned(struct dm_target *ti, struct
> dm_dev
> > > >> *dev,
> > > >> + sector_t start, sector_t len, void *data)
> > > >> {
> > > >> -struct request_queue *q = bdev_get_queue(dev->bdev);
> > > >> -enum blk_zoned_model *zoned_model = data;
> > > >> +bool *zoned = data;
> > > >>
> > > >> -return blk_queue_zoned_model(q) != *zoned_model;
> > > >> +return bdev_is_zoned(dev->bdev) != *zoned;
> > > >> }
> > > >>
> > > >> static int device_is_zoned_model(struct dm_target *ti, struct
> > > dm_dev
> > > >> *dev,
> > > >> sector_t start, sector_t len, void
> > > >> *data)
> > > >
> > > > Seems like the word "model" should also be remove here.
> > > >
> > > >> {
> > > >> -struct request_queue *q = bdev_get_queue(dev->bdev);
> > > >> -
> > > >> -return blk_queue_zoned_model(q) != BLK_ZONED_NONE;
> > > >> +return bdev_is_zoned(dev->bdev);
> > > >> }
> > > >>
> > > >> /*
> > > >> @@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct
> > > >> dm_target *ti, struct dm_dev *dev,
> > > >> * has the DM_TARGET_MIXED_ZONED_MODEL feature set, the
> devices
> > > can
> > > >> have any
> > > >> * zoned model with all zoned devices having the same zone
> size.
> > > >> */
> > > >> -static bool dm_table_supports_zoned_model(struct dm_table *t,
> > > >> - enum blk_zoned_model
> > > >> zoned_model)
> > > >> +static bool dm_table_supports_zoned(struct dm_table *t, bool
> > > zoned)
> > > >> {
> > > >> for (unsigned int i = 0; i < t->num_targets; i++) {
> > > >> struct dm_target *ti = dm_table_get_target(t, i);
> > > >> @@ -1623,11 +1619,11 @@ static bool
> > > >> dm_table_supports_zoned_model(struct dm_table *t,
> > > >>
> > > >> if (dm_target_supports_zoned_hm(ti->type)) {
> > > >> if (!ti->type->iterate_devices ||
> > > >> - ti->type->iterate_devices(ti,
> > > >> device_not_zoned_model,
> > > >> - &zoned_model))
> > > >> + ti->type->iterate_devices(ti,
> > > >> device_not_zoned,
> > > >> + &zoned))
> > > >> return false;
> > > >> } else if (!dm_target_supports_mixed_zoned_model(ti-
> > > >>> type)) {
> > > >> -if (zoned_model == BLK_ZONED_HM)
> > > >> +if (zoned)
> > > >> return false;
> > > >> }
> > > >> }
> > > >
> > > > The parameter "bool zoned" is redundant. It should be removed
> from
> > > the
> > > > above 3 functions
> >
> > The two func, is zoned and not zoned, are essentially the same.
> They
> > can be simplified into one function.
>
> Both functions are used for iterate_devices's callback in
> dm_table_supports_zoned_model(). As shown in raid_iterate_devices(),
> iterate_devices() returns 0 if the callback func calls on all the
> devices
> returns 0, or returns a non-zero result early otherwise. So, the
> iterate_devices() call returns "true" if any one of the underlying
> devices
> is (zoned|not zoned).
>
> Since we cannot create lambda as in other fancy languages, we need
> two
> functions...

Not really, there is a "void *data" can be used.

The device_is_zoned_model() is just the same as the device_not_zoned()
with (bool *)data = false.

It's very minor, so is okay to ignore my preference.

2023-12-19 08:13:05

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On 12/19/23 17:08, Ed Tsai (蔡宗軒) wrote:
> On Tue, 2023-12-19 at 07:16 +0000, Naohiro Aota wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>> On Mon, Dec 18, 2023 at 08:21:22AM +0000, Ed Tsai (蔡宗軒) wrote:
>>> On Mon, 2023-12-18 at 15:53 +0900, Damien Le Moal wrote:
>>>> On 2023/12/18 15:15, Ed Tsai (蔡宗軒) wrote:
>>>>> Hi Christoph,
>>>>>
>>>>> some minor suggestions:
>>>>>
>>>>> On Sun, 2023-12-17 at 17:53 +0100, Christoph Hellwig wrote:
>>>>>>
>>>>>> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
>>>>>> index 198d38b53322c1..260b5b8f2b0d7e 100644
>>>>>> --- a/drivers/md/dm-table.c
>>>>>> +++ b/drivers/md/dm-table.c
>>>>>> @@ -1579,21 +1579,18 @@ bool
>> dm_table_has_no_data_devices(struct
>>>>>> dm_table *t)
>>>>>> return true;
>>>>>> }
>>>>>>
>>>>>> -static int device_not_zoned_model(struct dm_target *ti,
>> struct
>>>>>> dm_dev *dev,
>>>>>> - sector_t start, sector_t len, void
>>>>>> *data)
>>>>>> +static int device_not_zoned(struct dm_target *ti, struct
>> dm_dev
>>>>>> *dev,
>>>>>> + sector_t start, sector_t len, void *data)
>>>>>> {
>>>>>> -struct request_queue *q = bdev_get_queue(dev->bdev);
>>>>>> -enum blk_zoned_model *zoned_model = data;
>>>>>> +bool *zoned = data;
>>>>>>
>>>>>> -return blk_queue_zoned_model(q) != *zoned_model;
>>>>>> +return bdev_is_zoned(dev->bdev) != *zoned;
>>>>>> }
>>>>>>
>>>>>> static int device_is_zoned_model(struct dm_target *ti, struct
>>>> dm_dev
>>>>>> *dev,
>>>>>> sector_t start, sector_t len, void
>>>>>> *data)
>>>>>
>>>>> Seems like the word "model" should also be remove here.
>>>>>
>>>>>> {
>>>>>> -struct request_queue *q = bdev_get_queue(dev->bdev);
>>>>>> -
>>>>>> -return blk_queue_zoned_model(q) != BLK_ZONED_NONE;
>>>>>> +return bdev_is_zoned(dev->bdev);
>>>>>> }
>>>>>>
>>>>>> /*
>>>>>> @@ -1603,8 +1600,7 @@ static int device_is_zoned_model(struct
>>>>>> dm_target *ti, struct dm_dev *dev,
>>>>>> * has the DM_TARGET_MIXED_ZONED_MODEL feature set, the
>> devices
>>>> can
>>>>>> have any
>>>>>> * zoned model with all zoned devices having the same zone
>> size.
>>>>>> */
>>>>>> -static bool dm_table_supports_zoned_model(struct dm_table *t,
>>>>>> - enum blk_zoned_model
>>>>>> zoned_model)
>>>>>> +static bool dm_table_supports_zoned(struct dm_table *t, bool
>>>> zoned)
>>>>>> {
>>>>>> for (unsigned int i = 0; i < t->num_targets; i++) {
>>>>>> struct dm_target *ti = dm_table_get_target(t, i);
>>>>>> @@ -1623,11 +1619,11 @@ static bool
>>>>>> dm_table_supports_zoned_model(struct dm_table *t,
>>>>>>
>>>>>> if (dm_target_supports_zoned_hm(ti->type)) {
>>>>>> if (!ti->type->iterate_devices ||
>>>>>> - ti->type->iterate_devices(ti,
>>>>>> device_not_zoned_model,
>>>>>> - &zoned_model))
>>>>>> + ti->type->iterate_devices(ti,
>>>>>> device_not_zoned,
>>>>>> + &zoned))
>>>>>> return false;
>>>>>> } else if (!dm_target_supports_mixed_zoned_model(ti-
>>>>>>> type)) {
>>>>>> -if (zoned_model == BLK_ZONED_HM)
>>>>>> +if (zoned)
>>>>>> return false;
>>>>>> }
>>>>>> }
>>>>>
>>>>> The parameter "bool zoned" is redundant. It should be removed
>> from
>>>> the
>>>>> above 3 functions
>>>
>>> The two func, is zoned and not zoned, are essentially the same.
>> They
>>> can be simplified into one function.
>>
>> Both functions are used for iterate_devices's callback in
>> dm_table_supports_zoned_model(). As shown in raid_iterate_devices(),
>> iterate_devices() returns 0 if the callback func calls on all the
>> devices
>> returns 0, or returns a non-zero result early otherwise. So, the
>> iterate_devices() call returns "true" if any one of the underlying
>> devices
>> is (zoned|not zoned).
>>
>> Since we cannot create lambda as in other fancy languages, we need
>> two
>> functions...
>
> Not really, there is a "void *data" can be used.
>
> The device_is_zoned_model() is just the same as the device_not_zoned()
> with (bool *)data = false.
>
> It's very minor, so is okay to ignore my preference.

Send a patch on top of Christoph's series if you want to clean this up.

--
Damien Le Moal
Western Digital Research


2023-12-19 10:38:50

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On Tue, Dec 19, 2023 at 05:12:41PM +0900, Damien Le Moal wrote:
> >> Since we cannot create lambda as in other fancy languages, we need
> >> two
> >> functions...
> >
> > Not really, there is a "void *data" can be used.
> >
> > The device_is_zoned_model() is just the same as the device_not_zoned()
> > with (bool *)data = false.
> >
> > It's very minor, so is okay to ignore my preference.
>
> Send a patch on top of Christoph's series if you want to clean this up.

I'll need to respin anyway, so I'll look into incorporating the
suggestion.

2023-12-19 12:17:30

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/5] block: remove support for the host aware zone model

On Tue, Dec 19, 2023 at 11:38:25AM +0100, [email protected] wrote:
> > > It's very minor, so is okay to ignore my preference.
> >
> > Send a patch on top of Christoph's series if you want to clean this up.
>
> I'll need to respin anyway, so I'll look into incorporating the
> suggestion.

I did look into it and the iterate callbacks confuse the heck out of me.
So I'm not going to touch them in non-trivial ways here and will leave
it for follow on cleanups as needed.

2023-12-20 03:18:26

by Jens Axboe

[permalink] [raw]
Subject: Re: remove support for the host aware zoned model


On Sun, 17 Dec 2023 17:53:54 +0100, Christoph Hellwig wrote:
> hen zones were first added the SCSI and ATA specs, two different
> models were supported (in addition to the drive managed one that
> is invisible to the host):
>
> - host managed where non-conventional zones there is strict requirement
> to write at the write pointer, or else an error is returned
> - host aware where a write point is maintained if writes always happen
> at it, otherwise it is left in an under-defined state and the
> sequential write preferred zones behave like conventional zones
> (probably very badly performing ones, though)
>
> [...]

Applied, thanks!

[1/5] virtio_blk: cleanup zoned device probing
commit: 77360cadaae562f437b3e98dc3af748d8d75bdc2
[2/5] virtio_blk: remove the broken zone revalidation support
commit: a971ed8002110f211899279cd7295756d263b771
[3/5] block: remove support for the host aware zone model
commit: 7437bb73f087e5f216f9c6603f5149d354e315af
[4/5] block: simplify disk_set_zoned
commit: d73e93b4dfab10c80688b061c30048df05585c7e
[5/5] sd: only call disk_clear_zoned when needed
commit: 5cc99b89785c55430a5674b32ad0d9e57a8ec251

Best regards,
--
Jens Axboe




2024-01-16 19:03:00

by patchwork-bot+f2fs

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH 1/5] virtio_blk: cleanup zoned device probing

Hello:

This series was applied to jaegeuk/f2fs.git (dev)
by Jens Axboe <[email protected]>:

On Sun, 17 Dec 2023 17:53:55 +0100 you wrote:
> Move reading and checking the zoned model from virtblk_probe_zoned_device
> into the caller, leaving only the code to perform the actual setup for
> host managed zoned devices in virtblk_probe_zoned_device.
>
> This allows to share the model reading and sharing between builds with
> and without CONFIG_BLK_DEV_ZONED, and improve it for the
> !CONFIG_BLK_DEV_ZONED case.
>
> [...]

Here is the summary with links:
- [f2fs-dev,1/5] virtio_blk: cleanup zoned device probing
https://git.kernel.org/jaegeuk/f2fs/c/77360cadaae5
- [f2fs-dev,2/5] virtio_blk: remove the broken zone revalidation support
https://git.kernel.org/jaegeuk/f2fs/c/a971ed800211
- [f2fs-dev,3/5] block: remove support for the host aware zone model
https://git.kernel.org/jaegeuk/f2fs/c/7437bb73f087
- [f2fs-dev,4/5] block: simplify disk_set_zoned
https://git.kernel.org/jaegeuk/f2fs/c/d73e93b4dfab
- [f2fs-dev,5/5] sd: only call disk_clear_zoned when needed
https://git.kernel.org/jaegeuk/f2fs/c/5cc99b89785c

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html