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