On Tue, Oct 24, 2023 at 07:45:13AM +0900, Damien Le Moal wrote:
> dm-error is used in several test cases in the xfstests test suite to
> check the handling of IO errors in file syatems. However, with several
^ systems
> file systems getting native support for zoned block devices (e.g. btrfs
> and f2fs), dm-error lack of zoned block device support creates problems
> as the file system attempt executing zone commands (e.g. a zone append
> operation) against a dm-error non-zoned block device, which causes
> various issues in the block layer (e.g. WARN_ON triggers).
>
> This patch adds supports for zoned block devices to dm-error, allowing
> an error table to be exposed as a zoned block device. This is done by
> relying on the first argument passed to dmsetup when creating the device
> table: if that first argument is a path to a backing block device, the
> dm-error device is created by copying the limits of the backing device,
> thus also copying its zone model. This is consistent with how xfstests
> creates dm-error devices (always passing the path to the backing device
> as the first argument).
>
> The zone support for dm-error requires the definition of the
> report_zones target type method, which is done by introducing the
> function io_err_report_zones(). Given that this function fails report
> zones operations (similarly to any other command issued to the dm-error
> device), dm_set_zones_restrictions() is tweaked to do nothing for a
> wildcard target to avoid failing zone revalidation. As the dm-error
> target does not implemt the iterate_devices method,
^implement
> dm_table_supports_zoned_model() is also changed to return true.
>
> Signed-off-by: Damien Le Moal <dlem...@kernel.org>
> ---
> drivers/md/dm-table.c | 3 +++
> drivers/md/dm-target.c | 42 ++++++++++++++++++++++++++++++++++++++++--
> drivers/md/dm-zone.c | 9 +++++++++
> 3 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 37b48f63ae6a..5e4d887063d3 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -1600,6 +1600,9 @@ static bool dm_table_supports_zoned_model(struct
> dm_table *t,
> for (unsigned int i = 0; i < t->num_targets; i++) {
> struct dm_target *ti = dm_table_get_target(t, i);
>
> + if (dm_target_is_wildcard(ti->type))
> + continue;
> +
This seems tricky to me. Currently, dm-error is the only dm target having
DM_TARGET_WILDCARD. But, can we expect that will be so forever?
Also, I considered what happens if the backing device is non-zoned
one. dm_table_supports_zoned_model() returns true in that case. But, it is
still reported as non-zoned as we copy non-zoned queue limits. So, it is
OK ... but it is a bit tricky.
Instead, how about implementing the .iterate_devices just like
linear_iterate_devices?
> if (dm_target_supports_zoned_hm(ti->type)) {
> if (!ti->type->iterate_devices ||
> ti->type->iterate_devices(ti,
> device_not_zoned_model,
> diff --git a/drivers/md/dm-target.c b/drivers/md/dm-target.c
> index 27e2992ff249..1bf4ecda3012 100644
> --- a/drivers/md/dm-target.c
> +++ b/drivers/md/dm-target.c
> @@ -118,6 +118,21 @@ EXPORT_SYMBOL(dm_unregister_target);
> */
> static int io_err_ctr(struct dm_target *tt, unsigned int argc, char **args)
> {
> + struct dm_dev *ddev;
> + int ret;
> +
> + /*
> + * If we have an argument, assume it is the path to the target
> + * block device we are replacing. In this case, get the device
> + * so that we can copy its limits in io_err_io_hints().
> + */
> + if (argc) {
> + ret = dm_get_device(tt, args[0], dm_table_get_mode(tt->table),
> + &ddev);
> + if (ret == 0)
> + tt->private = ddev;
No need to handle an error case here? Or, I guess it ignores an error for
compatibility when non-device argument is specified. Then, I'd like to add
a note here.
> + }
> +
> /*
> * Return error for discards instead of -EOPNOTSUPP
> */
> @@ -129,7 +144,10 @@ static int io_err_ctr(struct dm_target *tt, unsigned int
> argc, char **args)
>
> static void io_err_dtr(struct dm_target *tt)
> {
> - /* empty */
> + struct dm_dev *ddev = tt->private;
> +
> + if (ddev)
> + dm_put_device(tt, ddev);
> }
>
> static int io_err_map(struct dm_target *tt, struct bio *bio)
> @@ -149,8 +167,27 @@ static void io_err_release_clone_rq(struct request
> *clone,
> {
> }
>
> +#ifdef CONFIG_BLK_DEV_ZONED
> +static int io_err_report_zones(struct dm_target *ti,
> + struct dm_report_zones_args *args, unsigned int nr_zones)
> +{
> + return -EIO;
> +}
> +#else
> +#define io_err_report_zones NULL
> +#endif
> +
> static void io_err_io_hints(struct dm_target *ti, struct queue_limits
> *limits)
> {
> + struct dm_dev *ddev = ti->private;
> +
> + /* If we have a target device, copy its limits */
> + if (ddev) {
> + struct request_queue *q = bdev_get_queue(ddev->bdev);
> +
> + memcpy(limits, &q->limits, sizeof(*limits));
> + }
> +
> limits->max_discard_sectors = UINT_MAX;
> limits->max_hw_discard_sectors = UINT_MAX;
> limits->discard_granularity = 512;
> @@ -166,7 +203,7 @@ static long io_err_dax_direct_access(struct dm_target
> *ti, pgoff_t pgoff,
> static struct target_type error_target = {
> .name = "error",
> .version = {1, 6, 0},
> - .features = DM_TARGET_WILDCARD,
> + .features = DM_TARGET_WILDCARD | DM_TARGET_ZONED_HM,
> .ctr = io_err_ctr,
> .dtr = io_err_dtr,
> .map = io_err_map,
> @@ -174,6 +211,7 @@ static struct target_type error_target = {
> .release_clone_rq = io_err_release_clone_rq,
> .io_hints = io_err_io_hints,
> .direct_access = io_err_dax_direct_access,
> + .report_zones = io_err_report_zones,
> };
>
> int __init dm_target_init(void)
> diff --git a/drivers/md/dm-zone.c b/drivers/md/dm-zone.c
> index eb9832b22b14..9b77ee05e8dd 100644
> --- a/drivers/md/dm-zone.c
> +++ b/drivers/md/dm-zone.c
> @@ -297,6 +297,15 @@ int dm_set_zones_restrictions(struct dm_table *t, struct
> request_queue *q)
> WARN_ON_ONCE(queue_is_mq(q));
> md->disk->nr_zones = bdev_nr_zones(md->disk->part0);
>
> + /*
> + * With dm-error (wildcard target), report zones will fail, so do
> + * nothing. dm-error will copy the zones limits itself.
> + */
> + if (dm_table_get_wildcard_target(t)) {
> + md->nr_zones = md->disk->nr_zones;
> + return 0;
> + }
> +
> /* Check if zone append is natively supported */
> if (dm_table_supports_zone_append(t)) {
> clear_bit(DMF_EMULATE_ZONE_APPEND, &md->flags);
> --
> 2.41.0
>
