2009-09-18 15:26:11

by Junichi Nomura

[permalink] [raw]
Subject: [PATCH 1/2] dm: Set safe default max_sectors for targets with no underlying device

This is a preparation for the next patch, which changes
blk_set_default_limits() to set 0 for max_sectors/max_hw_sectors.
With the next patch, for dm targets like 'zero', where there's no
underlying device, those values remain 0 and the dm device becomes
unusable.

So check the max_sectors and set to SAFE_MAX_SECTORS if 0.

Check this thread for further background:
https://www.redhat.com/archives/dm-devel/2009-September/msg00176.html


Signed-off-by: Kiyoshi Ueda <[email protected]>
Signed-off-by: Jun'ichi Nomura <[email protected]>
Cc: David Strand <[email protected]>
Cc: Mike Snitzer <[email protected]>
Cc: Alasdair G Kergon <[email protected]>
Cc: Martin K. Petersen <[email protected]>
Cc: Jens Axboe <[email protected]>
---
drivers/md/dm-table.c | 11 +++++++++++
1 file changed, 11 insertions(+)

Index: linux-2.6.31.work/drivers/md/dm-table.c
===================================================================
--- linux-2.6.31.work.orig/drivers/md/dm-table.c
+++ linux-2.6.31.work/drivers/md/dm-table.c
@@ -707,6 +707,17 @@ static int validate_hardware_logical_blo
device_logical_block_size_sects - next_target_start : 0;
}

+ /*
+ * blk_set_default_limits() sets max_sectors/max_hw_sectors to 0.
+ * When all targets have no underlying device, they are
+ * left unchanged from the default values and cause problems.
+ * Use SAFE_MAX_SECTORS for such cases.
+ */
+ if (limits->max_hw_sectors == 0)
+ limits->max_hw_sectors = SAFE_MAX_SECTORS;
+ if (limits->max_sectors == 0)
+ limits->max_sectors = SAFE_MAX_SECTORS;
+
if (remaining) {
DMWARN("%s: table line %u (start sect %llu len %llu) "
"not aligned to h/w logical block size %u",


2009-09-18 15:52:26

by Mike Snitzer

[permalink] [raw]
Subject: Re: [PATCH 1/2] dm: Set safe default max_sectors for targets with no underlying device

On Fri, Sep 18 2009 at 11:18am -0400,
Jun'ichi Nomura <[email protected]> wrote:

> This is a preparation for the next patch, which changes
> blk_set_default_limits() to set 0 for max_sectors/max_hw_sectors.
> With the next patch, for dm targets like 'zero', where there's no
> underlying device, those values remain 0 and the dm device becomes
> unusable.
>
> So check the max_sectors and set to SAFE_MAX_SECTORS if 0.
>
> Check this thread for further background:
> https://www.redhat.com/archives/dm-devel/2009-September/msg00176.html
>
>
> Signed-off-by: Kiyoshi Ueda <[email protected]>
> Signed-off-by: Jun'ichi Nomura <[email protected]>
> Cc: David Strand <[email protected]>
> Cc: Mike Snitzer <[email protected]>
> Cc: Alasdair G Kergon <[email protected]>
> Cc: Martin K. Petersen <[email protected]>
> Cc: Jens Axboe <[email protected]>
> ---
> drivers/md/dm-table.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> Index: linux-2.6.31.work/drivers/md/dm-table.c
> ===================================================================
> --- linux-2.6.31.work.orig/drivers/md/dm-table.c
> +++ linux-2.6.31.work/drivers/md/dm-table.c
> @@ -707,6 +707,17 @@ static int validate_hardware_logical_blo
> device_logical_block_size_sects - next_target_start : 0;
> }
>
> + /*
> + * blk_set_default_limits() sets max_sectors/max_hw_sectors to 0.
> + * When all targets have no underlying device, they are
> + * left unchanged from the default values and cause problems.
> + * Use SAFE_MAX_SECTORS for such cases.
> + */
> + if (limits->max_hw_sectors == 0)
> + limits->max_hw_sectors = SAFE_MAX_SECTORS;
> + if (limits->max_sectors == 0)
> + limits->max_sectors = SAFE_MAX_SECTORS;
> +
> if (remaining) {
> DMWARN("%s: table line %u (start sect %llu len %llu) "
> "not aligned to h/w logical block size %u",

The blk_set_default_limits() that matters, as referenced in your comment
block, is actually in dm_calculate_queue_limits().

It would be better to put these checks in dm_calculate_queue_limits()
just before the call to validate_hardware_logical_block_alignment().

Either way works because the checks are effectively at the end of
dm_calculate_queue_limits() -- which is where they need to be.

But I think you're polluting validate_hardware_logical_block_alignment()
with unrelated checks.

Thanks,
Mike