The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2] dm dust: make badblock messages target-relative
@ 2026-08-03 14:09 Samuel Moelius
  2026-08-04 15:18 ` Bryan Gurney
  2026-08-04 15:50 ` Benjamin Marzinski
  0 siblings, 2 replies; 3+ messages in thread
From: Samuel Moelius @ 2026-08-03 14:09 UTC (permalink / raw)
  To: Alasdair Kergon
  Cc: Samuel Moelius, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski,
	open list:DEVICE-MAPPER  (LVM), open list

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 <sam.moelius@trailofbits.com>
---
Changes in v2:
  - Revise commit message
  - Remove call to sector_div() in __dust_map_write()

 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] dm dust: make badblock messages target-relative
  2026-08-03 14:09 [PATCH v2] dm dust: make badblock messages target-relative Samuel Moelius
@ 2026-08-04 15:18 ` Bryan Gurney
  2026-08-04 15:50 ` Benjamin Marzinski
  1 sibling, 0 replies; 3+ messages in thread
From: Bryan Gurney @ 2026-08-04 15:18 UTC (permalink / raw)
  To: Samuel Moelius
  Cc: Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	Benjamin Marzinski, open list:DEVICE-MAPPER (LVM), open list

On Mon, Aug 3, 2026 at 10:30 AM Samuel Moelius
<sam.moelius@trailofbits.com> 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 <sam.moelius@trailofbits.com>
> ---
> 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 <bgurney@redhat.com>

...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
>
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] dm dust: make badblock messages target-relative
  2026-08-03 14:09 [PATCH v2] dm dust: make badblock messages target-relative Samuel Moelius
  2026-08-04 15:18 ` Bryan Gurney
@ 2026-08-04 15:50 ` Benjamin Marzinski
  1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Marzinski @ 2026-08-04 15:50 UTC (permalink / raw)
  To: Samuel Moelius
  Cc: Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	open list:DEVICE-MAPPER  (LVM), open list

On Mon, Aug 03, 2026 at 02:09:13PM +0000, Samuel Moelius 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 <sam.moelius@trailofbits.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> ---
> Changes in v2:
>   - Revise commit message
>   - Remove call to sector_div() in __dust_map_write()
> 
>  drivers/md/dm-dust.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-04 15:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03 14:09 [PATCH v2] dm dust: make badblock messages target-relative Samuel Moelius
2026-08-04 15:18 ` Bryan Gurney
2026-08-04 15:50 ` Benjamin Marzinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox