* [dm-devel] [PATCH] dm crypt: set bdev to clone bio
@ 2022-06-09 11:43 Shin'ichiro Kawasaki
2022-06-09 12:02 ` Damien Le Moal
2022-06-10 16:26 ` [dm-devel] " Mike Snitzer
0 siblings, 2 replies; 5+ messages in thread
From: Shin'ichiro Kawasaki @ 2022-06-09 11:43 UTC (permalink / raw)
To: dm-devel, Mike Snitzer; +Cc: Shin'ichiro Kawasaki, Damien Le Moal
After the commit ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone"),
bdev is no longer set to clone bio for ->map function. Instead, each DM
targets shall set bdev to the clone bio by calling bio_set_dev() before
issuing IO. Also the commit ensured that dm_zone_endio() is called from
clone_endio() only when DM targets set bdev to the clone bio.
However, crypt_map() of dm-crypt does not call bio_set_dev() for every
clone bio. Then dm_zone_endio() is not called at completion of the bios
and zone locks are not properly unlocked. This triggers a hang when
blktests block/004 is run for dm-crypt on zoned block devices [1]. To
avoid the hang, call bio_set_dev() for every bio in crypt_map().
[1]
[ 6596.702977][T55017] run blktests block/004 at 2022-06-07 20:18:01
[ 6805.654531][ T41] INFO: task fio:55089 blocked for more than 122 seconds.
[ 6805.664287][ T41] Not tainted 5.19.0-rc1+ #1
[ 6805.671040][ T41] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 6805.682059][ T41] task:fio state:D stack: 0 pid:55089 ppid: 55042 flags:0x00000002
[ 6805.693538][ T41] Call Trace:
[ 6805.697563][ T41] <TASK>
[ 6805.700855][ T41] __schedule+0xd5d/0x4b70
[ 6805.701035][ T41] ? lock_is_held_type+0xe3/0x140
[ 6805.701094][ T41] ? lock_release+0x365/0x730
[ 6805.701153][ T41] ? io_schedule_timeout+0x150/0x150
[ 6805.701182][ T41] ? blk_start_plug_nr_ios+0x270/0x270
[ 6805.701208][ T41] schedule+0xe0/0x200
[ 6805.701225][ T41] io_schedule+0xbf/0x130
[ 6805.701238][ T41] bit_wait_io+0x17/0xe0
[ 6805.701251][ T41] __wait_on_bit_lock+0x11e/0x1b0
[ 6805.701266][ T41] ? out_of_line_wait_on_bit_lock+0xe0/0xe0
[ 6805.701289][ T41] out_of_line_wait_on_bit_lock+0xc6/0xe0
[ 6805.701302][ T41] ? __wait_on_bit_lock+0x1b0/0x1b0
[ 6805.701311][ T41] ? lock_is_held_type+0xe3/0x140
[ 6805.701330][ T41] ? cpuacct_css_alloc+0x150/0x150
[ 6805.701355][ T41] dm_zone_map_bio+0x4e3/0x15d0
[ 6805.701423][ T41] ? dm_set_zones_restrictions+0x930/0x930
[ 6805.701443][ T41] ? bvec_alloc+0x1a0/0x1a0
[ 6805.701457][ T41] ? lockdep_init_map_type+0x169/0x7a0
[ 6805.701478][ T41] __map_bio+0x4bc/0x6f0
[ 6805.701500][ T41] dm_submit_bio+0x635/0x1440
[ 6805.701531][ T41] ? dm_dax_direct_access+0x1c0/0x1c0
[ 6805.701542][ T41] ? lock_release+0x365/0x730
[ 6805.701559][ T41] ? reacquire_held_locks+0x4e0/0x4e0
[ 6805.701609][ T41] __submit_bio+0x1c0/0x2c0
[ 6805.701638][ T41] ? __bio_queue_enter+0x5b0/0x5b0
[ 6805.701683][ T41] ? lockdep_hardirqs_on_prepare+0x17b/0x410
[ 6805.701703][ T41] submit_bio_noacct_nocheck+0x2f8/0x810
[ 6805.701727][ T41] ? should_fail_request+0x70/0x70
[ 6805.701737][ T41] ? submit_bio_noacct+0x1079/0x1650
[ 6805.701776][ T41] submit_bio+0x92/0x250
[ 6805.701829][ T41] ? submit_bio_noacct+0x1650/0x1650
[ 6805.701860][ T41] submit_bio_wait+0xf2/0x1d0
[ 6805.701873][ T41] ? submit_bio_wait_endio+0x40/0x40
[ 6805.701963][ T41] ? bio_init+0x365/0x5e0
[ 6805.701985][ T41] __blkdev_direct_IO_simple+0x326/0x550
[ 6805.702010][ T41] ? blkdev_llseek+0xd0/0xd0
[ 6805.702019][ T41] ? truncate_folio_batch_exceptionals.part.0+0x540/0x540
[ 6805.702044][ T41] ? lock_is_held_type+0xe3/0x140
[ 6805.702058][ T41] ? find_held_lock+0x2d/0x110
[ 6805.702087][ T41] ? __bio_clone+0x350/0x350
[ 6805.702124][ T41] ? generic_update_time+0x195/0x2b0
[ 6805.702156][ T41] generic_file_direct_write+0x1a9/0x490
[ 6805.702193][ T41] __generic_file_write_iter+0x161/0x430
[ 6805.702221][ T41] blkdev_write_iter+0x32c/0x5a0
[ 6805.702242][ T41] ? blkdev_open+0x240/0x240
[ 6805.702266][ T41] ? do_fault+0x4bd/0xed0
[ 6805.702278][ T41] ? restore_exclusive_pte+0x3b0/0x3b0
[ 6805.702303][ T41] new_sync_write+0x2cd/0x500
[ 6805.702320][ T41] ? new_sync_read+0x500/0x500
[ 6805.702355][ T41] ? inode_security+0x54/0xf0
[ 6805.702400][ T41] vfs_write+0x62c/0x980
[ 6805.702425][ T41] __x64_sys_pwrite64+0x17a/0x1c0
[ 6805.702439][ T41] ? vfs_write+0x980/0x980
[ 6805.702453][ T41] ? syscall_enter_from_user_mode+0x20/0x70
[ 6805.702477][ T41] do_syscall_64+0x3a/0x80
[ 6805.702493][ T41] entry_SYSCALL_64_after_hwframe+0x46/0xb0
[ 6805.702507][ T41] RIP: 0033:0x7fdec0affc6f
[ 6805.702538][ T41] RSP: 002b:00007ffd9c193620 EFLAGS: 00000293 ORIG_RAX: 0000000000000012
[ 6805.702554][ T41] RAX: ffffffffffffffda RBX: 0000000001170540 RCX: 00007fdec0affc6f
[ 6805.702562][ T41] RDX: 0000000000001000 RSI: 00000000011de000 RDI: 0000000000000007
[ 6805.702569][ T41] RBP: 00007fdea150a6c8 R08: 0000000000000000 R09: 0000000000000001
[ 6805.702576][ T41] R10: 0000000000c00000 R11: 0000000000000293 R12: 0000000000000001
[ 6805.702583][ T41] R13: 0000000000000000 R14: 0000000000001000 R15: 00007fdea150a6c8
[ 6805.702634][ T41] </TASK>
[ 6805.702643][ T41] INFO: task fio:55091 blocked for more than 122 seconds.
[ 6805.702651][ T41] Not tainted 5.19.0-rc1+ #1
[ 6805.702657][ T41] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 6805.702663][ T41] task:fio state:D stack: 0 pid:55091 ppid: 55042 flags:0x00000002
[ 6805.702679][ T41] Call Trace:
[ 6805.702685][ T41] <TASK>
[ 6805.702703][ T41] __schedule+0xd5d/0x4b70
[ 6805.702719][ T41] ? lock_is_held_type+0xe3/0x140
[ 6805.702747][ T41] ? lock_release+0x365/0x730
[ 6805.702777][ T41] ? io_schedule_timeout+0x150/0x150
[ 6805.702829][ T41] ? blk_start_plug_nr_ios+0x270/0x270
[ 6805.702853][ T41] schedule+0xe0/0x200
[ 6805.702870][ T41] io_schedule+0xbf/0x130
[ 6805.702925][ T41] bit_wait_io+0x17/0xe0
[ 6805.702939][ T41] __wait_on_bit_lock+0x11e/0x1b0
[ 6805.702954][ T41] ? out_of_line_wait_on_bit_lock+0xe0/0xe0
[ 6805.702977][ T41] out_of_line_wait_on_bit_lock+0xc6/0xe0
[ 6805.702990][ T41] ? __wait_on_bit_lock+0x1b0/0x1b0
[ 6805.702998][ T41] ? lock_is_held_type+0xe3/0x140
[ 6805.703018][ T41] ? cpuacct_css_alloc+0x150/0x150
[ 6805.703042][ T41] dm_zone_map_bio+0x4e3/0x15d0
[ 6805.703082][ T41] ? dm_set_zones_restrictions+0x930/0x930
[ 6805.703101][ T41] ? bvec_alloc+0x1a0/0x1a0
[ 6805.703114][ T41] ? lockdep_init_map_type+0x169/0x7a0
[ 6805.703136][ T41] __map_bio+0x4bc/0x6f0
[ 6805.703158][ T41] dm_submit_bio+0x635/0x1440
[ 6805.703188][ T41] ? dm_dax_direct_access+0x1c0/0x1c0
[ 6805.703199][ T41] ? lock_release+0x365/0x730
[ 6805.703216][ T41] ? reacquire_held_locks+0x4e0/0x4e0
[ 6805.703267][ T41] __submit_bio+0x1c0/0x2c0
[ 6805.703281][ T41] ? __bio_queue_enter+0x5b0/0x5b0
[ 6805.703300][ T41] ? lockdep_hardirqs_on_prepare+0x17b/0x410
[ 6805.703320][ T41] submit_bio_noacct_nocheck+0x2f8/0x810
[ 6805.703343][ T41] ? should_fail_request+0x70/0x70
[ 6805.703353][ T41] ? submit_bio_noacct+0x1079/0x1650
[ 6805.703392][ T41] submit_bio+0x92/0x250
[ 6805.703407][ T41] ? submit_bio_noacct+0x1650/0x1650
[ 6805.703438][ T41] submit_bio_wait+0xf2/0x1d0
[ 6805.703451][ T41] ? submit_bio_wait_endio+0x40/0x40
[ 6805.703488][ T41] ? bio_init+0x365/0x5e0
[ 6805.703509][ T41] __blkdev_direct_IO_simple+0x326/0x550
[ 6805.703534][ T41] ? blkdev_llseek+0xd0/0xd0
[ 6805.703542][ T41] ? truncate_folio_batch_exceptionals.part.0+0x540/0x540
[ 6805.703566][ T41] ? lock_is_held_type+0xe3/0x140
[ 6805.703579][ T41] ? find_held_lock+0x2d/0x110
[ 6805.703608][ T41] ? __bio_clone+0x350/0x350
[ 6805.703645][ T41] ? generic_update_time+0x195/0x2b0
[ 6805.703675][ T41] generic_file_direct_write+0x1a9/0x490
[ 6805.703711][ T41] __generic_file_write_iter+0x161/0x430
[ 6805.703738][ T41] blkdev_write_iter+0x32c/0x5a0
[ 6805.703759][ T41] ? blkdev_open+0x240/0x240
[ 6805.703822][ T41] ? do_fault+0x4bd/0xed0
[ 6805.703835][ T41] ? restore_exclusive_pte+0x3b0/0x3b0
[ 6805.703860][ T41] new_sync_write+0x2cd/0x500
[ 6805.703915][ T41] ? new_sync_read+0x500/0x500
[ 6805.703951][ T41] ? inode_security+0x54/0xf0
[ 6805.703995][ T41] vfs_write+0x62c/0x980
[ 6805.704021][ T41] __x64_sys_pwrite64+0x17a/0x1c0
[ 6805.704034][ T41] ? vfs_write+0x980/0x980
[ 6805.704048][ T41] ? syscall_enter_from_user_mode+0x20/0x70
[ 6805.704073][ T41] do_syscall_64+0x3a/0x80
[ 6805.704100][ T41] entry_SYSCALL_64_after_hwframe+0x46/0xb0
[ 6805.704111][ T41] RIP: 0033:0x7fdec0affc6f
[ 6805.704120][ T41] RSP: 002b:00007ffd9c193620 EFLAGS: 00000293 ORIG_RAX: 0000000000000012
[ 6805.704133][ T41] RAX: ffffffffffffffda RBX: 0000000001170540 RCX: 00007fdec0affc6f
[ 6805.704140][ T41] RDX: 0000000000001000 RSI: 00000000011de000 RDI: 0000000000000007
[ 6805.704146][ T41] RBP: 00007fdea1568318 R08: 0000000000000000 R09: 0000000000000001
[ 6805.704152][ T41] R10: 0000000000801000 R11: 0000000000000293 R12: 0000000000000001
[ 6805.704158][ T41] R13: 0000000000000000 R14: 0000000000001000 R15: 00007fdea1568318
[ 6805.704205][ T41] </TASK>
[ 6805.704236][ T41]
[ 6805.704236][ T41] Showing all locks held in the system:
[ 6805.704262][ T41] 2 locks held by pr/ttyS0/16:
[ 6805.704285][ T41] 1 lock held by khungtaskd/41:
[ 6805.704292][ T41] #0: ffffffffb163b380 (rcu_read_lock){....}-{1:2}, at: debug_show_all_locks+0x53/0x260
[ 6805.704343][ T41] 3 locks held by systemd-journal/541:
[ 6805.705034][ T41] 1 lock held by fio/[ 6805.705051][ T41] #0: ffff88811a04d110 (&md->io_barrier){....}-{0:0}, at: dm_get_live_table+0x5/0x110
[ 6805.705118][ T41] 1 lock held by fio/55091:
[ 6805.705125][ T41] #0: ffff88811a04d110 (&md->io_barrier){....}-{0:0}, at: dm_get_live_table+0x5/0x110
[ 6805.705162][ T41]
[ 6805.705191][ T41] =============================================
[ 6805.705191][ T41]
Fixes: ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone")
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
drivers/md/dm-crypt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 159c6806c19b..c68523a89428 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -3378,6 +3378,8 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
struct dm_crypt_io *io;
struct crypt_config *cc = ti->private;
+ bio_set_dev(bio, cc->dev->bdev);
+
/*
* If bio is REQ_PREFLUSH or REQ_OP_DISCARD, just bypass crypt queues.
* - for REQ_PREFLUSH device-mapper core ensures that no IO is in-flight
@@ -3385,7 +3387,6 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
*/
if (unlikely(bio->bi_opf & REQ_PREFLUSH ||
bio_op(bio) == REQ_OP_DISCARD)) {
- bio_set_dev(bio, cc->dev->bdev);
if (bio_sectors(bio))
bio->bi_iter.bi_sector = cc->start +
dm_target_offset(ti, bio->bi_iter.bi_sector);
--
2.36.1
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [dm-devel] [PATCH] dm crypt: set bdev to clone bio
2022-06-09 11:43 [dm-devel] [PATCH] dm crypt: set bdev to clone bio Shin'ichiro Kawasaki
@ 2022-06-09 12:02 ` Damien Le Moal
2022-06-10 16:26 ` [dm-devel] " Mike Snitzer
1 sibling, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2022-06-09 12:02 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, dm-devel, Mike Snitzer
On 6/9/22 20:43, Shin'ichiro Kawasaki wrote:
> After the commit ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone"),
> bdev is no longer set to clone bio for ->map function. Instead, each DM
> targets shall set bdev to the clone bio by calling bio_set_dev() before
> issuing IO. Also the commit ensured that dm_zone_endio() is called from
> clone_endio() only when DM targets set bdev to the clone bio.
>
> However, crypt_map() of dm-crypt does not call bio_set_dev() for every
> clone bio. Then dm_zone_endio() is not called at completion of the bios
> and zone locks are not properly unlocked. This triggers a hang when
> blktests block/004 is run for dm-crypt on zoned block devices [1]. To
> avoid the hang, call bio_set_dev() for every bio in crypt_map().
>
> [1]
>
> [ 6596.702977][T55017] run blktests block/004 at 2022-06-07 20:18:01
> [ 6805.654531][ T41] INFO: task fio:55089 blocked for more than 122 seconds.
> [ 6805.664287][ T41] Not tainted 5.19.0-rc1+ #1
> [ 6805.671040][ T41] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 6805.682059][ T41] task:fio state:D stack: 0 pid:55089 ppid: 55042 flags:0x00000002
> [ 6805.693538][ T41] Call Trace:
> [ 6805.697563][ T41] <TASK>
> [ 6805.700855][ T41] __schedule+0xd5d/0x4b70
> [ 6805.701035][ T41] ? lock_is_held_type+0xe3/0x140
> [ 6805.701094][ T41] ? lock_release+0x365/0x730
> [ 6805.701153][ T41] ? io_schedule_timeout+0x150/0x150
> [ 6805.701182][ T41] ? blk_start_plug_nr_ios+0x270/0x270
> [ 6805.701208][ T41] schedule+0xe0/0x200
> [ 6805.701225][ T41] io_schedule+0xbf/0x130
> [ 6805.701238][ T41] bit_wait_io+0x17/0xe0
> [ 6805.701251][ T41] __wait_on_bit_lock+0x11e/0x1b0
> [ 6805.701266][ T41] ? out_of_line_wait_on_bit_lock+0xe0/0xe0
> [ 6805.701289][ T41] out_of_line_wait_on_bit_lock+0xc6/0xe0
> [ 6805.701302][ T41] ? __wait_on_bit_lock+0x1b0/0x1b0
> [ 6805.701311][ T41] ? lock_is_held_type+0xe3/0x140
> [ 6805.701330][ T41] ? cpuacct_css_alloc+0x150/0x150
> [ 6805.701355][ T41] dm_zone_map_bio+0x4e3/0x15d0
> [ 6805.701423][ T41] ? dm_set_zones_restrictions+0x930/0x930
> [ 6805.701443][ T41] ? bvec_alloc+0x1a0/0x1a0
> [ 6805.701457][ T41] ? lockdep_init_map_type+0x169/0x7a0
> [ 6805.701478][ T41] __map_bio+0x4bc/0x6f0
> [ 6805.701500][ T41] dm_submit_bio+0x635/0x1440
> [ 6805.701531][ T41] ? dm_dax_direct_access+0x1c0/0x1c0
> [ 6805.701542][ T41] ? lock_release+0x365/0x730
> [ 6805.701559][ T41] ? reacquire_held_locks+0x4e0/0x4e0
> [ 6805.701609][ T41] __submit_bio+0x1c0/0x2c0
> [ 6805.701638][ T41] ? __bio_queue_enter+0x5b0/0x5b0
> [ 6805.701683][ T41] ? lockdep_hardirqs_on_prepare+0x17b/0x410
> [ 6805.701703][ T41] submit_bio_noacct_nocheck+0x2f8/0x810
> [ 6805.701727][ T41] ? should_fail_request+0x70/0x70
> [ 6805.701737][ T41] ? submit_bio_noacct+0x1079/0x1650
> [ 6805.701776][ T41] submit_bio+0x92/0x250
> [ 6805.701829][ T41] ? submit_bio_noacct+0x1650/0x1650
> [ 6805.701860][ T41] submit_bio_wait+0xf2/0x1d0
> [ 6805.701873][ T41] ? submit_bio_wait_endio+0x40/0x40
> [ 6805.701963][ T41] ? bio_init+0x365/0x5e0
> [ 6805.701985][ T41] __blkdev_direct_IO_simple+0x326/0x550
> [ 6805.702010][ T41] ? blkdev_llseek+0xd0/0xd0
> [ 6805.702019][ T41] ? truncate_folio_batch_exceptionals.part.0+0x540/0x540
> [ 6805.702044][ T41] ? lock_is_held_type+0xe3/0x140
> [ 6805.702058][ T41] ? find_held_lock+0x2d/0x110
> [ 6805.702087][ T41] ? __bio_clone+0x350/0x350
> [ 6805.702124][ T41] ? generic_update_time+0x195/0x2b0
> [ 6805.702156][ T41] generic_file_direct_write+0x1a9/0x490
> [ 6805.702193][ T41] __generic_file_write_iter+0x161/0x430
> [ 6805.702221][ T41] blkdev_write_iter+0x32c/0x5a0
> [ 6805.702242][ T41] ? blkdev_open+0x240/0x240
> [ 6805.702266][ T41] ? do_fault+0x4bd/0xed0
> [ 6805.702278][ T41] ? restore_exclusive_pte+0x3b0/0x3b0
> [ 6805.702303][ T41] new_sync_write+0x2cd/0x500
> [ 6805.702320][ T41] ? new_sync_read+0x500/0x500
> [ 6805.702355][ T41] ? inode_security+0x54/0xf0
> [ 6805.702400][ T41] vfs_write+0x62c/0x980
> [ 6805.702425][ T41] __x64_sys_pwrite64+0x17a/0x1c0
> [ 6805.702439][ T41] ? vfs_write+0x980/0x980
> [ 6805.702453][ T41] ? syscall_enter_from_user_mode+0x20/0x70
> [ 6805.702477][ T41] do_syscall_64+0x3a/0x80
> [ 6805.702493][ T41] entry_SYSCALL_64_after_hwframe+0x46/0xb0
> [ 6805.702507][ T41] RIP: 0033:0x7fdec0affc6f
> [ 6805.702538][ T41] RSP: 002b:00007ffd9c193620 EFLAGS: 00000293 ORIG_RAX: 0000000000000012
> [ 6805.702554][ T41] RAX: ffffffffffffffda RBX: 0000000001170540 RCX: 00007fdec0affc6f
> [ 6805.702562][ T41] RDX: 0000000000001000 RSI: 00000000011de000 RDI: 0000000000000007
> [ 6805.702569][ T41] RBP: 00007fdea150a6c8 R08: 0000000000000000 R09: 0000000000000001
> [ 6805.702576][ T41] R10: 0000000000c00000 R11: 0000000000000293 R12: 0000000000000001
> [ 6805.702583][ T41] R13: 0000000000000000 R14: 0000000000001000 R15: 00007fdea150a6c8
> [ 6805.702634][ T41] </TASK>
> [ 6805.702643][ T41] INFO: task fio:55091 blocked for more than 122 seconds.
> [ 6805.702651][ T41] Not tainted 5.19.0-rc1+ #1
> [ 6805.702657][ T41] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 6805.702663][ T41] task:fio state:D stack: 0 pid:55091 ppid: 55042 flags:0x00000002
> [ 6805.702679][ T41] Call Trace:
> [ 6805.702685][ T41] <TASK>
> [ 6805.702703][ T41] __schedule+0xd5d/0x4b70
> [ 6805.702719][ T41] ? lock_is_held_type+0xe3/0x140
> [ 6805.702747][ T41] ? lock_release+0x365/0x730
> [ 6805.702777][ T41] ? io_schedule_timeout+0x150/0x150
> [ 6805.702829][ T41] ? blk_start_plug_nr_ios+0x270/0x270
> [ 6805.702853][ T41] schedule+0xe0/0x200
> [ 6805.702870][ T41] io_schedule+0xbf/0x130
> [ 6805.702925][ T41] bit_wait_io+0x17/0xe0
> [ 6805.702939][ T41] __wait_on_bit_lock+0x11e/0x1b0
> [ 6805.702954][ T41] ? out_of_line_wait_on_bit_lock+0xe0/0xe0
> [ 6805.702977][ T41] out_of_line_wait_on_bit_lock+0xc6/0xe0
> [ 6805.702990][ T41] ? __wait_on_bit_lock+0x1b0/0x1b0
> [ 6805.702998][ T41] ? lock_is_held_type+0xe3/0x140
> [ 6805.703018][ T41] ? cpuacct_css_alloc+0x150/0x150
> [ 6805.703042][ T41] dm_zone_map_bio+0x4e3/0x15d0
> [ 6805.703082][ T41] ? dm_set_zones_restrictions+0x930/0x930
> [ 6805.703101][ T41] ? bvec_alloc+0x1a0/0x1a0
> [ 6805.703114][ T41] ? lockdep_init_map_type+0x169/0x7a0
> [ 6805.703136][ T41] __map_bio+0x4bc/0x6f0
> [ 6805.703158][ T41] dm_submit_bio+0x635/0x1440
> [ 6805.703188][ T41] ? dm_dax_direct_access+0x1c0/0x1c0
> [ 6805.703199][ T41] ? lock_release+0x365/0x730
> [ 6805.703216][ T41] ? reacquire_held_locks+0x4e0/0x4e0
> [ 6805.703267][ T41] __submit_bio+0x1c0/0x2c0
> [ 6805.703281][ T41] ? __bio_queue_enter+0x5b0/0x5b0
> [ 6805.703300][ T41] ? lockdep_hardirqs_on_prepare+0x17b/0x410
> [ 6805.703320][ T41] submit_bio_noacct_nocheck+0x2f8/0x810
> [ 6805.703343][ T41] ? should_fail_request+0x70/0x70
> [ 6805.703353][ T41] ? submit_bio_noacct+0x1079/0x1650
> [ 6805.703392][ T41] submit_bio+0x92/0x250
> [ 6805.703407][ T41] ? submit_bio_noacct+0x1650/0x1650
> [ 6805.703438][ T41] submit_bio_wait+0xf2/0x1d0
> [ 6805.703451][ T41] ? submit_bio_wait_endio+0x40/0x40
> [ 6805.703488][ T41] ? bio_init+0x365/0x5e0
> [ 6805.703509][ T41] __blkdev_direct_IO_simple+0x326/0x550
> [ 6805.703534][ T41] ? blkdev_llseek+0xd0/0xd0
> [ 6805.703542][ T41] ? truncate_folio_batch_exceptionals.part.0+0x540/0x540
> [ 6805.703566][ T41] ? lock_is_held_type+0xe3/0x140
> [ 6805.703579][ T41] ? find_held_lock+0x2d/0x110
> [ 6805.703608][ T41] ? __bio_clone+0x350/0x350
> [ 6805.703645][ T41] ? generic_update_time+0x195/0x2b0
> [ 6805.703675][ T41] generic_file_direct_write+0x1a9/0x490
> [ 6805.703711][ T41] __generic_file_write_iter+0x161/0x430
> [ 6805.703738][ T41] blkdev_write_iter+0x32c/0x5a0
> [ 6805.703759][ T41] ? blkdev_open+0x240/0x240
> [ 6805.703822][ T41] ? do_fault+0x4bd/0xed0
> [ 6805.703835][ T41] ? restore_exclusive_pte+0x3b0/0x3b0
> [ 6805.703860][ T41] new_sync_write+0x2cd/0x500
> [ 6805.703915][ T41] ? new_sync_read+0x500/0x500
> [ 6805.703951][ T41] ? inode_security+0x54/0xf0
> [ 6805.703995][ T41] vfs_write+0x62c/0x980
> [ 6805.704021][ T41] __x64_sys_pwrite64+0x17a/0x1c0
> [ 6805.704034][ T41] ? vfs_write+0x980/0x980
> [ 6805.704048][ T41] ? syscall_enter_from_user_mode+0x20/0x70
> [ 6805.704073][ T41] do_syscall_64+0x3a/0x80
> [ 6805.704100][ T41] entry_SYSCALL_64_after_hwframe+0x46/0xb0
> [ 6805.704111][ T41] RIP: 0033:0x7fdec0affc6f
> [ 6805.704120][ T41] RSP: 002b:00007ffd9c193620 EFLAGS: 00000293 ORIG_RAX: 0000000000000012
> [ 6805.704133][ T41] RAX: ffffffffffffffda RBX: 0000000001170540 RCX: 00007fdec0affc6f
> [ 6805.704140][ T41] RDX: 0000000000001000 RSI: 00000000011de000 RDI: 0000000000000007
> [ 6805.704146][ T41] RBP: 00007fdea1568318 R08: 0000000000000000 R09: 0000000000000001
> [ 6805.704152][ T41] R10: 0000000000801000 R11: 0000000000000293 R12: 0000000000000001
> [ 6805.704158][ T41] R13: 0000000000000000 R14: 0000000000001000 R15: 00007fdea1568318
> [ 6805.704205][ T41] </TASK>
> [ 6805.704236][ T41]
> [ 6805.704236][ T41] Showing all locks held in the system:
> [ 6805.704262][ T41] 2 locks held by pr/ttyS0/16:
> [ 6805.704285][ T41] 1 lock held by khungtaskd/41:
> [ 6805.704292][ T41] #0: ffffffffb163b380 (rcu_read_lock){....}-{1:2}, at: debug_show_all_locks+0x53/0x260
> [ 6805.704343][ T41] 3 locks held by systemd-journal/541:
> [ 6805.705034][ T41] 1 lock held by fio/[ 6805.705051][ T41] #0: ffff88811a04d110 (&md->io_barrier){....}-{0:0}, at: dm_get_live_table+0x5/0x110
> [ 6805.705118][ T41] 1 lock held by fio/55091:
> [ 6805.705125][ T41] #0: ffff88811a04d110 (&md->io_barrier){....}-{0:0}, at: dm_get_live_table+0x5/0x110
> [ 6805.705162][ T41]
> [ 6805.705191][ T41] =============================================
> [ 6805.705191][ T41]
>
> Fixes: ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone")
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
> drivers/md/dm-crypt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 159c6806c19b..c68523a89428 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -3378,6 +3378,8 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
> struct dm_crypt_io *io;
> struct crypt_config *cc = ti->private;
>
> + bio_set_dev(bio, cc->dev->bdev);
> +
> /*
> * If bio is REQ_PREFLUSH or REQ_OP_DISCARD, just bypass crypt queues.
> * - for REQ_PREFLUSH device-mapper core ensures that no IO is in-flight
> @@ -3385,7 +3387,6 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
> */
> if (unlikely(bio->bi_opf & REQ_PREFLUSH ||
> bio_op(bio) == REQ_OP_DISCARD)) {
> - bio_set_dev(bio, cc->dev->bdev);
> if (bio_sectors(bio))
> bio->bi_iter.bi_sector = cc->start +
> dm_target_offset(ti, bio->bi_iter.bi_sector);
Looks ok to me.
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
--
Damien Le Moal
Western Digital Research
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dm-devel] dm crypt: set bdev to clone bio
2022-06-09 11:43 [dm-devel] [PATCH] dm crypt: set bdev to clone bio Shin'ichiro Kawasaki
2022-06-09 12:02 ` Damien Le Moal
@ 2022-06-10 16:26 ` Mike Snitzer
2022-06-10 18:56 ` Mike Snitzer
1 sibling, 1 reply; 5+ messages in thread
From: Mike Snitzer @ 2022-06-10 16:26 UTC (permalink / raw)
To: Shin'ichiro Kawasaki; +Cc: dm-devel, Damien Le Moal
On Thu, Jun 09 2022 at 7:43P -0400,
Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> wrote:
> After the commit ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone"),
> bdev is no longer set to clone bio for ->map function. Instead, each DM
> targets shall set bdev to the clone bio by calling bio_set_dev() before
> issuing IO. Also the commit ensured that dm_zone_endio() is called from
> clone_endio() only when DM targets set bdev to the clone bio.
>
> However, crypt_map() of dm-crypt does not call bio_set_dev() for every
> clone bio. Then dm_zone_endio() is not called at completion of the bios
> and zone locks are not properly unlocked. This triggers a hang when
> blktests block/004 is run for dm-crypt on zoned block devices [1]. To
> avoid the hang, call bio_set_dev() for every bio in crypt_map().
>
> [1]
>
> [ 6596.702977][T55017] run blktests block/004 at 2022-06-07 20:18:01
<snip>
Please refrain from putting stack traces in patch headers whenever
possible. Really no need for this, especially given how long this one
is!
I revised the header as follows:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.19&id=cae0053631cd4b02bb882b53c7da20652b038049
> Fixes: ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone")
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
> drivers/md/dm-crypt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 159c6806c19b..c68523a89428 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -3378,6 +3378,8 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
> struct dm_crypt_io *io;
> struct crypt_config *cc = ti->private;
>
> + bio_set_dev(bio, cc->dev->bdev);
> +
> /*
> * If bio is REQ_PREFLUSH or REQ_OP_DISCARD, just bypass crypt queues.
> * - for REQ_PREFLUSH device-mapper core ensures that no IO is in-flight
> @@ -3385,7 +3387,6 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
> */
> if (unlikely(bio->bi_opf & REQ_PREFLUSH ||
> bio_op(bio) == REQ_OP_DISCARD)) {
> - bio_set_dev(bio, cc->dev->bdev);
> if (bio_sectors(bio))
> bio->bi_iter.bi_sector = cc->start +
> dm_target_offset(ti, bio->bi_iter.bi_sector);
> --
> 2.36.1
>
BUT something isn't quite adding up with why you need this change
given commit ca522482e3ea has this compatibility code:
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 9650ba2075b8..d62f1354ecbf 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -581,7 +581,9 @@ static struct dm_io *alloc_io(struct mapped_device *md, struct bio *bio)
struct dm_target_io *tio;
struct bio *clone;
- clone = bio_alloc_clone(bio->bi_bdev, bio, GFP_NOIO, &md->io_bs);
+ clone = bio_alloc_clone(NULL, bio, GFP_NOIO, &md->io_bs);
+ /* Set default bdev, but target must bio_set_dev() before issuing IO */
+ clone->bi_bdev = md->disk->part0;
tio = clone_to_tio(clone);
tio->flags = 0;
The clone bio passed to crypt_map() _should_ be the same as was passed
before commit ca522482e3ea (It gets set to md->disk->part0 rather than
bio->bi_bdev but they really should point to the same top-level DM
bdev).
So why is your extra override to have dm-crypt point to its underlying
data device important now?
Mike
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [dm-devel] dm crypt: set bdev to clone bio
2022-06-10 16:26 ` [dm-devel] " Mike Snitzer
@ 2022-06-10 18:56 ` Mike Snitzer
2022-06-13 12:04 ` Shinichiro Kawasaki
0 siblings, 1 reply; 5+ messages in thread
From: Mike Snitzer @ 2022-06-10 18:56 UTC (permalink / raw)
To: Shin'ichiro Kawasaki; +Cc: dm-devel, Damien Le Moal
On Fri, Jun 10 2022 at 12:26P -0400,
Mike Snitzer <snitzer@kernel.org> wrote:
> On Thu, Jun 09 2022 at 7:43P -0400,
> Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> wrote:
>
> > After the commit ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone"),
> > bdev is no longer set to clone bio for ->map function. Instead, each DM
> > targets shall set bdev to the clone bio by calling bio_set_dev() before
> > issuing IO. Also the commit ensured that dm_zone_endio() is called from
> > clone_endio() only when DM targets set bdev to the clone bio.
> >
> > However, crypt_map() of dm-crypt does not call bio_set_dev() for every
> > clone bio. Then dm_zone_endio() is not called at completion of the bios
> > and zone locks are not properly unlocked. This triggers a hang when
> > blktests block/004 is run for dm-crypt on zoned block devices [1]. To
> > avoid the hang, call bio_set_dev() for every bio in crypt_map().
> >
> > [1]
> >
> > [ 6596.702977][T55017] run blktests block/004 at 2022-06-07 20:18:01
>
> <snip>
>
> Please refrain from putting stack traces in patch headers whenever
> possible. Really no need for this, especially given how long this one
> is!
>
> I revised the header as follows:
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.19&id=cae0053631cd4b02bb882b53c7da20652b038049
>
> > Fixes: ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone")
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > ---
> > drivers/md/dm-crypt.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > index 159c6806c19b..c68523a89428 100644
> > --- a/drivers/md/dm-crypt.c
> > +++ b/drivers/md/dm-crypt.c
> > @@ -3378,6 +3378,8 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
> > struct dm_crypt_io *io;
> > struct crypt_config *cc = ti->private;
> >
> > + bio_set_dev(bio, cc->dev->bdev);
> > +
> > /*
> > * If bio is REQ_PREFLUSH or REQ_OP_DISCARD, just bypass crypt queues.
> > * - for REQ_PREFLUSH device-mapper core ensures that no IO is in-flight
> > @@ -3385,7 +3387,6 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
> > */
> > if (unlikely(bio->bi_opf & REQ_PREFLUSH ||
> > bio_op(bio) == REQ_OP_DISCARD)) {
> > - bio_set_dev(bio, cc->dev->bdev);
> > if (bio_sectors(bio))
> > bio->bi_iter.bi_sector = cc->start +
> > dm_target_offset(ti, bio->bi_iter.bi_sector);
> > --
> > 2.36.1
> >
>
> BUT something isn't quite adding up with why you need this change
> given commit ca522482e3ea has this compatibility code:
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 9650ba2075b8..d62f1354ecbf 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -581,7 +581,9 @@ static struct dm_io *alloc_io(struct mapped_device *md, struct bio *bio)
> struct dm_target_io *tio;
> struct bio *clone;
>
> - clone = bio_alloc_clone(bio->bi_bdev, bio, GFP_NOIO, &md->io_bs);
> + clone = bio_alloc_clone(NULL, bio, GFP_NOIO, &md->io_bs);
> + /* Set default bdev, but target must bio_set_dev() before issuing IO */
> + clone->bi_bdev = md->disk->part0;
>
> tio = clone_to_tio(clone);
> tio->flags = 0;
>
> The clone bio passed to crypt_map() _should_ be the same as was passed
> before commit ca522482e3ea (It gets set to md->disk->part0 rather than
> bio->bi_bdev but they really should point to the same top-level DM
> bdev).
>
> So why is your extra override to have dm-crypt point to its underlying
> data device important now?
Think the issue is not that the clone->bi_bdev isn't set. It is that
it is merely not changed from the md->disk->part0 default.
Commit ca522482e3ea added this to clone_endio:
if (likely(bio->bi_bdev != md->disk->part0)) {
...
if (static_branch_unlikely(&zoned_enabled) &&
unlikely(blk_queue_is_zoned(q)))
dm_zone_endio(io, bio);
}
So dm_zone_endio() isn't called unless you force dm-crypt (or any
other target) to remap the clone bio to a different bdev.
It is weird that a bio is completing without having been remapped by
dm-crypt; but there could be any number of reasons why that hasn't
happened.
Anyway, I think a correct fix needs to be to the logic in
clone_endio(). Could be that its best to just remove the gating
likely() conditional.
Can you please test this patch instead of your patch?
Thanks,
Mike
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8b21155d3c4f..d8f16183bf27 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1016,23 +1016,19 @@ static void clone_endio(struct bio *bio)
struct dm_io *io = tio->io;
struct mapped_device *md = io->md;
- if (likely(bio->bi_bdev != md->disk->part0)) {
- struct request_queue *q = bdev_get_queue(bio->bi_bdev);
-
- if (unlikely(error == BLK_STS_TARGET)) {
- if (bio_op(bio) == REQ_OP_DISCARD &&
- !bdev_max_discard_sectors(bio->bi_bdev))
- disable_discard(md);
- else if (bio_op(bio) == REQ_OP_WRITE_ZEROES &&
- !q->limits.max_write_zeroes_sectors)
- disable_write_zeroes(md);
- }
-
- if (static_branch_unlikely(&zoned_enabled) &&
- unlikely(blk_queue_is_zoned(q)))
- dm_zone_endio(io, bio);
+ if (unlikely(error == BLK_STS_TARGET)) {
+ if (bio_op(bio) == REQ_OP_DISCARD &&
+ !bdev_max_discard_sectors(bio->bi_bdev))
+ disable_discard(md);
+ else if (bio_op(bio) == REQ_OP_WRITE_ZEROES &&
+ !bdev_write_zeroes_sectors(bio->bi_bdev))
+ 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) {
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [dm-devel] dm crypt: set bdev to clone bio
2022-06-10 18:56 ` Mike Snitzer
@ 2022-06-13 12:04 ` Shinichiro Kawasaki
0 siblings, 0 replies; 5+ messages in thread
From: Shinichiro Kawasaki @ 2022-06-13 12:04 UTC (permalink / raw)
To: Mike Snitzer; +Cc: dm-devel, Damien Le Moal
On Jun 10, 2022 / 14:56, Mike Snitzer wrote:
> On Fri, Jun 10 2022 at 12:26P -0400,
> Mike Snitzer <snitzer@kernel.org> wrote:
>
> > On Thu, Jun 09 2022 at 7:43P -0400,
> > Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> wrote:
> >
> > > After the commit ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone"),
> > > bdev is no longer set to clone bio for ->map function. Instead, each DM
> > > targets shall set bdev to the clone bio by calling bio_set_dev() before
> > > issuing IO. Also the commit ensured that dm_zone_endio() is called from
> > > clone_endio() only when DM targets set bdev to the clone bio.
> > >
> > > However, crypt_map() of dm-crypt does not call bio_set_dev() for every
> > > clone bio. Then dm_zone_endio() is not called at completion of the bios
> > > and zone locks are not properly unlocked. This triggers a hang when
> > > blktests block/004 is run for dm-crypt on zoned block devices [1]. To
> > > avoid the hang, call bio_set_dev() for every bio in crypt_map().
> > >
> > > [1]
> > >
> > > [ 6596.702977][T55017] run blktests block/004 at 2022-06-07 20:18:01
> >
> > <snip>
> >
> > Please refrain from putting stack traces in patch headers whenever
> > possible. Really no need for this, especially given how long this one
> > is!
> >
> > I revised the header as follows:
> > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.19&id=cae0053631cd4b02bb882b53c7da20652b038049
I see. I will keep this in mind for my future patches. Thanks.
> >
> > > Fixes: ca522482e3ea ("dm: pass NULL bdev to bio_alloc_clone")
> > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > > ---
> > > drivers/md/dm-crypt.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > > index 159c6806c19b..c68523a89428 100644
> > > --- a/drivers/md/dm-crypt.c
> > > +++ b/drivers/md/dm-crypt.c
> > > @@ -3378,6 +3378,8 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
> > > struct dm_crypt_io *io;
> > > struct crypt_config *cc = ti->private;
> > >
> > > + bio_set_dev(bio, cc->dev->bdev);
> > > +
> > > /*
> > > * If bio is REQ_PREFLUSH or REQ_OP_DISCARD, just bypass crypt queues.
> > > * - for REQ_PREFLUSH device-mapper core ensures that no IO is in-flight
> > > @@ -3385,7 +3387,6 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
> > > */
> > > if (unlikely(bio->bi_opf & REQ_PREFLUSH ||
> > > bio_op(bio) == REQ_OP_DISCARD)) {
> > > - bio_set_dev(bio, cc->dev->bdev);
> > > if (bio_sectors(bio))
> > > bio->bi_iter.bi_sector = cc->start +
> > > dm_target_offset(ti, bio->bi_iter.bi_sector);
> > > --
> > > 2.36.1
> > >
> >
> > BUT something isn't quite adding up with why you need this change
> > given commit ca522482e3ea has this compatibility code:
> >
> > diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> > index 9650ba2075b8..d62f1354ecbf 100644
> > --- a/drivers/md/dm.c
> > +++ b/drivers/md/dm.c
> > @@ -581,7 +581,9 @@ static struct dm_io *alloc_io(struct mapped_device *md, struct bio *bio)
> > struct dm_target_io *tio;
> > struct bio *clone;
> >
> > - clone = bio_alloc_clone(bio->bi_bdev, bio, GFP_NOIO, &md->io_bs);
> > + clone = bio_alloc_clone(NULL, bio, GFP_NOIO, &md->io_bs);
> > + /* Set default bdev, but target must bio_set_dev() before issuing IO */
> > + clone->bi_bdev = md->disk->part0;
> >
> > tio = clone_to_tio(clone);
> > tio->flags = 0;
> >
> > The clone bio passed to crypt_map() _should_ be the same as was passed
> > before commit ca522482e3ea (It gets set to md->disk->part0 rather than
> > bio->bi_bdev but they really should point to the same top-level DM
> > bdev).
> >
> > So why is your extra override to have dm-crypt point to its underlying
> > data device important now?
>
> Think the issue is not that the clone->bi_bdev isn't set. It is that
> it is merely not changed from the md->disk->part0 default.
>
> Commit ca522482e3ea added this to clone_endio:
>
> if (likely(bio->bi_bdev != md->disk->part0)) {
> ...
> if (static_branch_unlikely(&zoned_enabled) &&
> unlikely(blk_queue_is_zoned(q)))
> dm_zone_endio(io, bio);
> }
>
> So dm_zone_endio() isn't called unless you force dm-crypt (or any
> other target) to remap the clone bio to a different bdev.
Yes, I have same understanding. I assumed the bio->bi_bdev != md->disk->part0
can not be removed, and suggested to dm-crypt to remap all clone bios.
>
> It is weird that a bio is completing without having been remapped by
> dm-crypt; but there could be any number of reasons why that hasn't
> happened.
>
> Anyway, I think a correct fix needs to be to the logic in
> clone_endio(). Could be that its best to just remove the gating
> likely() conditional.
>
> Can you please test this patch instead of your patch?
I tried this new patch and confirmed it avoids the hang. Looks good :)
Also, I ran my test set for the patch in same manner I did for the dm-5.19
branch, and observed no failure. Good. I found that this patch is already
pulled into v5.19-rc2, and a bit surprised. It looks too late to put my
Tested-by tag. Anyway, thanks for the fix.
--
Shin'ichiro Kawasaki
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-14 6:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-09 11:43 [dm-devel] [PATCH] dm crypt: set bdev to clone bio Shin'ichiro Kawasaki
2022-06-09 12:02 ` Damien Le Moal
2022-06-10 16:26 ` [dm-devel] " Mike Snitzer
2022-06-10 18:56 ` Mike Snitzer
2022-06-13 12:04 ` Shinichiro Kawasaki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.