[dm-devel] [bug report] BUG for REQ_OP_WRITE_ZEROES to dm-zoned

[Shinichiro Kawasaki]

Hello Mike,

Let me share a BUG I observed with v5.18-rcX and ask comments for the fix.

BUG_ON(dm_tio_flagged(tio, DM_TIO_IS_DUPLICATE_BIO)) in dm_accept_partial_bio()
was triggered for dm-zoned. It happens when a bio with REQ_OP_WRITE_ZEROES and
sector range which goes across zone boundaries of the zoned devices that
dm-zoned maps. For such bios, dm-zoned calls dm_accept_partial_bio() to trim the
bio to fit in a zone. And dm core sets the flag DM_TIO_IS_DUPLICATE_BIO to the
tio of the bio.

    The BUG_ON symptom can be recreated with command as follows:

    # xfs_io -C "fzero 4096 $((512 * $(</sys/block/sdf/queue/chunk_sectors)))" /dev/dm-0

    In this command, /dev/dm-0 is the dm-zoned device. /dev/sdf is the zoned
    block device. Its zone size is obtained from sysfs chunk_sectors attribute.

The trigger commit is e6fc9f62ce6e ("dm: flag clones created by
__send_duplicate_bios") which introduced the new flag (it was named
is_duplicated_bio, and following commit renamed it to DM_TIO_IS_DUPLICATE_BIO).
I understand that the flag is set to the bios cloned in __send_duplicate_bios()
to guard tio->len_ptr shared among the cloned bios from updates in
dm_accept_partial_bio().

One point I can not understand is that the flag is set even when
__send_duplicate_bios() clones only single bio. I think bio is not duplicated in
this case, and there is no need to guard tio->len_ptr. Dm-zoned sets 1 to
ti->num_write_zeroes_bios (and ti->num_discard_bios), then I think
__send_duplicate_bios() always clones single bio for dm-zoned. I tried
following patch below, which removes the flag set for the single bio clone case.

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index f2397546b93f..d886c57e49ed 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1363,7 +1363,6 @@ static void __send_duplicate_bios(struct clone_info *ci, struct dm_target *ti,
                break;
        case 1:
                clone = alloc_tio(ci, ti, 0, len, GFP_NOIO);
-               dm_tio_set_flag(clone_to_tio(clone), DM_TIO_IS_DUPLICATE_BIO);
                __map_bio(clone);
                break;
        default:

With this patch, the BUG is no longer triggered. Is this a right fix approach?
It looks for me the DM_TIO_IS_DUPLICATE_BIO check is too tight and I think we
can relax it for the single clone case.

If I miss anything and the len_ptr guard by DM_TIO_IS_DUPLICATE_BIO is required
even for the single bio clone case, I will think about dm-zoned change to avoid
dm_accept_partial_bio() call, which will need bio split within dm-zoned.

-- 
Best Regards,
Shin'ichiro Kawasaki