On Fri, Feb 02 2024 at 2:30P -0500,
Damien Le Moal <dlemoal@xxxxxxxxxx> wrote:
> For targets requiring zone append operation emulation with regular
> writes (e.g. dm-crypt), we can use the block layer emulation provided by
> zone write plugging. Remove DM implemented zone append emulation and
> enable the block layer one.
>
> This is done by setting the max_zone_append_sectors limit of the
> mapped device queue to 0 for mapped devices that have a target table
> that cannot support native zone append operations. These includes
> mixed zoned and non-zoned targets, or targets that explicitly requested
> emulation of zone append (e.g. dm-crypt). For these mapped devices, the
> new field emulate_zone_append is set to true. dm_split_and_process_bio()
> is modified to call blk_zone_write_plug_bio() for such device to let the
> block layer transform zone append operations into regular writes. This
> is done after ensuring that the submitted BIO is split if it straddles
> zone boundaries.
>
> dm_revalidate_zones() is also modified to use the block layer provided
> function blk_revalidate_disk_zones() so that all zone resources needed
> for zone append emulation are allocated and initialized by the block
> layer without DM core needing to do anything. Since the device table is
> not yet live when dm_revalidate_zones() is executed, enabling the use of
> blk_revalidate_disk_zones() requires adding a pointer to the device
> table in struct mapped_device. This avoids errors in
> dm_blk_report_zones() trying to get the table with dm_get_live_table().
> The mapped device table pointer is set to the table passed as argument
> to dm_revalidate_zones() before calling blk_revalidate_disk_zones() and
> reset to NULL after this function returns to restore the live table
> handling for user call of report zones.
>
> All the code related to zone append emulation is removed from
> dm-zone.c. This leads to simplifications of the functions __map_bio()
> and dm_zone_endio(). This later function now only needs to deal with
> completions of real zone append operations for targets that support it.
>
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
Love the overall improvement to the DM core code and the broader block
layer by switching to this bio-based ZWP approach.
Reviewed-by: Mike Snitzer <snitzer@xxxxxxxxxx>
But one incremental suggestion inlined below.
> ---
> drivers/md/dm-core.h | 11 +-
> drivers/md/dm-zone.c | 470 ++++---------------------------------------
> drivers/md/dm.c | 44 ++--
> drivers/md/dm.h | 7 -
> 4 files changed, 68 insertions(+), 464 deletions(-)
>
<snip>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 8dcabf84d866..92ce3b2eb4ae 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1419,25 +1419,12 @@ static void __map_bio(struct bio *clone)
> down(&md->swap_bios_semaphore);
> }
>
> - if (static_branch_unlikely(&zoned_enabled)) {
> - /*
> - * Check if the IO needs a special mapping due to zone append
> - * emulation on zoned target. In this case, dm_zone_map_bio()
> - * calls the target map operation.
> - */
> - if (unlikely(dm_emulate_zone_append(md)))
> - r = dm_zone_map_bio(tio);
> - else
> - goto do_map;
> - } else {
> -do_map:
> - if (likely(ti->type->map == linear_map))
> - r = linear_map(ti, clone);
> - else if (ti->type->map == stripe_map)
> - r = stripe_map(ti, clone);
> - else
> - r = ti->type->map(ti, clone);
> - }
> + if (likely(ti->type->map == linear_map))
> + r = linear_map(ti, clone);
> + else if (ti->type->map == stripe_map)
> + r = stripe_map(ti, clone);
> + else
> + r = ti->type->map(ti, clone);
>
> switch (r) {
> case DM_MAPIO_SUBMITTED:
> @@ -1774,19 +1761,33 @@ static void dm_split_and_process_bio(struct mapped_device *md,
> struct clone_info ci;
> struct dm_io *io;
> blk_status_t error = BLK_STS_OK;
> - bool is_abnormal;
> + bool is_abnormal, need_split;
>
> is_abnormal = is_abnormal_io(bio);
> - if (unlikely(is_abnormal)) {
> + if (likely(!md->emulate_zone_append))
> + need_split = is_abnormal;
> + else
> + need_split = is_abnormal || bio_straddle_zones(bio);
> + if (unlikely(need_split)) {
> /*
> * Use bio_split_to_limits() for abnormal IO (e.g. discard, etc)
> * otherwise associated queue_limits won't be imposed.
> + * Also split the BIO for mapped devices needing zone append
> + * emulation to ensure that the BIO does not cross zone
> + * boundaries.
> */
> bio = bio_split_to_limits(bio);
> if (!bio)
> return;
> }
>
> + /*
> + * Use the block layer zone write plugging for mapped devices that
> + * need zone append emulation (e.g. dm-crypt).
> + */
> + if (md->emulate_zone_append && blk_zone_write_plug_bio(bio, 0))
> + return;
> +
> /* Only support nowait for normal IO */
> if (unlikely(bio->bi_opf & REQ_NOWAIT) && !is_abnormal) {
> io = alloc_io(md, bio, GFP_NOWAIT);
Would prefer to see this incremental change included from the start:
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 92ce3b2eb4ae..1fd9bbf35db3 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1763,11 +1763,10 @@ static void dm_split_and_process_bio(struct mapped_device *md,
blk_status_t error = BLK_STS_OK;
bool is_abnormal, need_split;
- is_abnormal = is_abnormal_io(bio);
- if (likely(!md->emulate_zone_append))
- need_split = is_abnormal;
- else
+ need_split = is_abnormal = is_abnormal_io(bio);
+ if (static_branch_unlikely(&zoned_enabled) && unlikely(md->emulate_zone_append))
need_split = is_abnormal || bio_straddle_zones(bio);
+
if (unlikely(need_split)) {
/*
* Use bio_split_to_limits() for abnormal IO (e.g. discard, etc)
@@ -1781,12 +1780,14 @@ static void dm_split_and_process_bio(struct mapped_device *md,
return;
}
- /*
- * Use the block layer zone write plugging for mapped devices that
- * need zone append emulation (e.g. dm-crypt).
- */
- if (md->emulate_zone_append && blk_zone_write_plug_bio(bio, 0))
- return;
+ if (static_branch_unlikely(&zoned_enabled)) {
+ /*
+ * Use the block layer zone write plugging for mapped devices that
+ * need zone append emulation (e.g. dm-crypt).
+ */
+ if (unlikely(md->emulate_zone_append) && blk_zone_write_plug_bio(bio, 0))
+ return;
+ }
/* Only support nowait for normal IO */
if (unlikely(bio->bi_opf & REQ_NOWAIT) && !is_abnormal) {
