* [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