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
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
