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

Bryan Gurney <[email protected]>
Newsgroups gmane.linux.kernel
Message-ID <CAHhmqcQigDdvfazttm_tFiXMy-3hUQQnQS0meJrzhkDC_ie0Xg@mail.gmail.com>
On Mon, Aug 3, 2026 at 10:30 AM 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=$(blockdev --getsz "$TEST_DEV")
+    # sz=$(blockdev --getsz "$TEST_DEV")
     bsz=$(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=/dev/zero
of=/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, sector_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 write",
>                                (unsigned long long)thisblock);
>                 }
> @@ -224,15 +223,16 @@ static int dust_map_write(struct dust_device *dd, sector_t thisblock,
>  static int dust_map(struct dm_target *ti, struct bio *bio)
>  {
>         struct dust_device *dd = ti->private;
> +       sector_t dust_sector = dm_target_offset(ti, bio->bi_iter.bi_sector);
>         int r;
>
>         bio_set_dev(bio, dd->dev->bdev);
> -       bio->bi_iter.bi_sector = dd->start + dm_target_offset(ti, bio->bi_iter.bi_sector);
> +       bio->bi_iter.bi_sector = dd->start + dust_sector;
>
>         if (bio_data_dir(bio) == READ)
> -               r = dust_map_read(dd, bio->bi_iter.bi_sector, dd->fail_read_on_bb);
> +               r = dust_map_read(dd, dust_sector, dd->fail_read_on_bb);
>         else
> -               r = dust_map_write(dd, bio->bi_iter.bi_sector, dd->fail_read_on_bb);
> +               r = 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, unsigned int argc, char **argv,
>                         char *result, unsigned int maxlen)
>  {
>         struct dust_device *dd = ti->private;
> -       sector_t size = bdev_nr_sectors(dd->dev->bdev);
> +       sector_t size = dm_sector_div_up(ti->len, dd->sect_per_block);
>         bool invalid_msg = false;
>         int r = -EINVAL;
>         unsigned long long tmp, block;
> @@ -462,8 +462,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv,
>                         return r;
>
>                 block = tmp;
> -               sector_div(size, dd->sect_per_block);
> -               if (block > size) {
> +               if (block >= size) {
>                         DMERR("selected block value out of range");
>                         return r;
>                 }
> @@ -490,8 +489,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv,
>                         return r;
>                 }
>                 wr_fail_cnt = tmp_ui;
> -               sector_div(size, dd->sect_per_block);
> -               if (block > size) {
> +               if (block >= size) {
>                         DMERR("selected block value out of range");
>                         return r;
>                 }
> --
> 2.43.0
>
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.