Re: [PATCH v2] dm dust: make badblock messages target-relative

On Mon, Aug 3, 2026 at 10:30=E2=80=AFAM Samuel Moelius
<[email protected]> wrote:
>
> dm-dust currently treats addbadblock, removebadblock and queryblock
> arguments as block numbers on the underlying device. That is surprising
> for a device-mapper target: a dm-dust table with a non-zero backing
> offset can add bad blocks that are outside the mapped target, and a
> badblock added for logical block 0 is missed because the I/O path checks
> the remapped backing-device block instead.
>
> Interpret badblock message arguments as blocks relative to the start of
> the dm-dust target instead. Bound the arguments by the target length and
> perform badblock lookup using target-relative sectors before remapping
> the bio to the underlying device.
>
> This intentionally changes the non-zero backing-offset behavior to make
> the badblock control interface match the mapped dm-dust device, rather
> than the underlying device.
>
> Assisted-by: Codex:gpt-5.5-cyber-preview
> Signed-off-by: Samuel Moelius <[email protected]>
> ---
> Changes in v2:
>   - Revise commit message
>   - Remove call to sector_div() in __dust_map_write()
>

Hi,

I'm Bryan Gurney, the original submitter of dm-dust.  It's a test
target, so its main value is in being able to instigate a certain kind
of failure.  It's been a while since I've worked on this, but I made a
modification to the blktests test that I created for it, to test this
patch:

--- tests/dm/002
+++ tests/dm/002
@@ -17,9 +17,9 @@ test_device() {
     local sz bsz
     echo "Running ${TEST_NAME}"

-    sz=3D$(blockdev --getsz "$TEST_DEV")
+    # sz=3D$(blockdev --getsz "$TEST_DEV")
     bsz=3D$(blockdev --getbsz "$TEST_DEV")
-    dmsetup create dust1 --table "0 $sz dust $TEST_DEV 0 $bsz"
+    dmsetup create dust1 --table "0 1048576 dust $TEST_DEV 1024 $bsz"
     dmsetup message dust1 0 addbadblock 60
     dmsetup message dust1 0 addbadblock 67
     dmsetup message dust1 0 addbadblock 72

...and I built it off of v7.2.0-rc6; in the non-zero offset test, the
3 badblocks fail to clear after the "dd if=3D/dev/zero
of=3D/dev/mapper/dust1" command, but with your patch (both v1 and v2),
both the zero-offset and non-zero offset tests successfully pass.

So with that, I will add:

Tested-by: Bryan Gurney <[email protected]>

...in the sense that it does what the original designer of the test
target feels should be the correct behavior.

I admit that I should have done more testing with non-zero offsets.  I
saw Ben Marzinski's review comment about dm-dust being able to "mark
bad blocks that aren't aligned with the dm-dust target"; that's an
oversight, and this patch fixes it.  (Or at least, as best as I can
test; Ben, Mikulas and Mike are more experienced with the
device-mapper code.)

To try and answer Ben's question about the expectations of existing
dm-dust users: from how I tried to design the target, a "bad block"
should be relative to the created device; e.g., "/dev/mapper/dust1".


Thanks,

Bryan Gurney
Senior Software Engineer - Enterprise Storage
Red Hat

>  drivers/md/dm-dust.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c
> index c7e3077fb1f5..954f4ec5a51c 100644
> --- a/drivers/md/dm-dust.c
> +++ b/drivers/md/dm-dust.c
> @@ -196,7 +196,6 @@ static int __dust_map_write(struct dust_device *dd, s=
ector_t thisblock)
>                 dd->badblock_count--;
>                 kfree(bblk);
>                 if (!dd->quiet_mode) {
> -                       sector_div(thisblock, dd->sect_per_block);
>                         DMINFO("block %llu removed from badblocklist by w=
rite",
>                                (unsigned long long)thisblock);
>                 }
> @@ -224,15 +223,16 @@ static int dust_map_write(struct dust_device *dd, s=
ector_t thisblock,
>  static int dust_map(struct dm_target *ti, struct bio *bio)
>  {
>         struct dust_device *dd =3D ti->private;
> +       sector_t dust_sector =3D dm_target_offset(ti, bio->bi_iter.bi_sec=
tor);
>         int r;
>
>         bio_set_dev(bio, dd->dev->bdev);
> -       bio->bi_iter.bi_sector =3D dd->start + dm_target_offset(ti, bio->=
bi_iter.bi_sector);
> +       bio->bi_iter.bi_sector =3D dd->start + dust_sector;
>
>         if (bio_data_dir(bio) =3D=3D READ)
> -               r =3D dust_map_read(dd, bio->bi_iter.bi_sector, dd->fail_=
read_on_bb);
> +               r =3D dust_map_read(dd, dust_sector, dd->fail_read_on_bb)=
;
>         else
> -               r =3D dust_map_write(dd, bio->bi_iter.bi_sector, dd->fail=
_read_on_bb);
> +               r =3D dust_map_write(dd, dust_sector, dd->fail_read_on_bb=
);
>
>         return r;
>  }
> @@ -415,7 +415,7 @@ static int dust_message(struct dm_target *ti, unsigne=
d int argc, char **argv,
>                         char *result, unsigned int maxlen)
>  {
>         struct dust_device *dd =3D ti->private;
> -       sector_t size =3D bdev_nr_sectors(dd->dev->bdev);
> +       sector_t size =3D dm_sector_div_up(ti->len, dd->sect_per_block);
>         bool invalid_msg =3D false;
>         int r =3D -EINVAL;
>         unsigned long long tmp, block;
> @@ -462,8 +462,7 @@ static int dust_message(struct dm_target *ti, unsigne=
d int argc, char **argv,
>                         return r;
>
>                 block =3D tmp;
> -               sector_div(size, dd->sect_per_block);
> -               if (block > size) {
> +               if (block >=3D size) {
>                         DMERR("selected block value out of range");
>                         return r;
>                 }
> @@ -490,8 +489,7 @@ static int dust_message(struct dm_target *ti, unsigne=
d int argc, char **argv,
>                         return r;
>                 }
>                 wr_fail_cnt =3D tmp_ui;
> -               sector_div(size, dd->sect_per_block);
> -               if (block > size) {
> +               if (block >=3D size) {
>                         DMERR("selected block value out of range");
>                         return r;
>                 }
> --
> 2.43.0
>
>