On 2022-06-15 13:01, Damien Le Moal wrote: > On 6/15/22 19:19, Pankaj Raghav wrote: >> dm_zone_endio() updates the bi_sector of orig bio for zoned devices that >> uses either native append or append emulation and it is called before the >> endio of the target. But target endio can still update the clone bio >> after dm_zone_endio is called, thereby, the orig bio does not contain >> the updated information anymore. Call dm_zone_endio for zoned devices >> after calling the target's endio function >> >> Signed-off-by: Pankaj Raghav <p.raghav@xxxxxxxxxxx> >> --- >> @Damien and @Hannes: I couldn't come up with a testcase that uses endio callback and >> zone append or append emulation for zoned devices to test for >> regression in this change. It would be great if you can suggest >> something. This change is required for the npo2 target as we update the >> clone bio sector in the endio callback function and the orig bio should >> be updated only after the endio callback for zone appends. > > Running zonefs tests on top of dm-crypt will exercise DM zone append > emulation. > Thanks. However, I am facing issues creating a dm-crypt target with a zoned device. Steps: - cryptsetup luksFormat <zns-device> is throwing a bunch of IO errors with the following error message: Device wipe error, offset 32768. Cannot wipe header on device <zns-device>. I can observe the same behavior in both v5.18 and next-20220615 with cryptsetup 2.4.3.The same step is working correctly on a normal NVMe device. Am I doing something wrong? ZNS info: zsze 128M and zcap 128M with 50 zones >> >> drivers/md/dm.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/md/dm.c b/drivers/md/dm.c >> index 3f17fe1de..3a74e1038 100644 >> --- a/drivers/md/dm.c >> +++ b/drivers/md/dm.c >> @@ -1025,10 +1025,6 @@ static void clone_endio(struct bio *bio) >> disable_write_zeroes(md); >> } >> >> - if (static_branch_unlikely(&zoned_enabled) && >> - unlikely(blk_queue_is_zoned(bdev_get_queue(bio->bi_bdev)))) >> - dm_zone_endio(io, bio); >> - >> if (endio) { >> int r = endio(ti, bio, &error); >> switch (r) { >> @@ -1057,6 +1053,10 @@ static void clone_endio(struct bio *bio) >> } >> } >> >> + if (static_branch_unlikely(&zoned_enabled) && >> + unlikely(blk_queue_is_zoned(bdev_get_queue(bio->bi_bdev)))) > > blk_queue_is_zoned(bdev_get_queue(bio->bi_bdev))) -> > bdev_is_zoned(bio->bi_bdev) > Ok. Even though I just moved the statements, I think this is trivial enough to take it along. >> + dm_zone_endio(io, bio); >> + >> if (static_branch_unlikely(&swap_bios_enabled) && >> unlikely(swap_bios_limit(ti, bio))) >> up(&md->swap_bios_semaphore); > > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel