On Thu, Jun 09 2022 at 7:43P -0400,
Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> 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@xxxxxxx>
> ---
> 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@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
