On Wed, Jun 10 2009 at 7:08pm -0400,
Alasdair G Kergon <agk@xxxxxxxxxx> wrote:
> On Wed, Jun 10, 2009 at 06:06:36PM -0400, Martin K. Petersen wrote:
> > The default limits should all be set by the block layer when setting up
> > the request queue. So my reason for inquiring was to figure out whether
> > check_for_valid_limits() actually makes any difference?
>
> I renamed that badly-named function earlier to:
> init_valid_queue_limits()
>
> It should also really be shared with block/.
>
> What's going on is that LVM prepares the new tables it wants to
> build up a device stack in advance, then if everything has worked,
> makes them all go live.
>
> The validation has to happen during the first phase - backing out
> the change to the device stack upon a failure is easier then as
> we have not yet reached the commit point of the transaction.
> The operation making the new stack live if at all possible must not
> fail, because that comes within the commit logic and would make recovery
> much trickier.
>
> In dm terms, this means we have two tables - called 'live' and
> 'inactive'. The first phase sets up inactive tables on all the stacked
> devices involved in the transaction and that is when all the memory
> needed is allocated and the validation occurs. The second phase then
> makes the inactive table live and discards the previously-live table.
> The two tables are independent: the old queue limits on the dm device
> are discarded and replaced by the newly-calculated ones.
>
> Currently those limits are calculated in phase one, but we should see
> about delaying this limit combination code (which should alway succeed)
> until phase two (which gives userspace code more freedom in its use of
> the interface).
Alasdair,
I've attempted to implement your suggested change of moving the
combining of limits from stage1 (dmsetup load) to stage2 (dmsetup
resume).
- moved combining the limits for the DM table into stage2
(dm_table_set_restrictions).
- init_valid_queue_limits() was removed in favor of Martin's
blk_set_default_limits()
But the following still establishes each target device's limits during
stage1 (dm_set_device_limits). I don't see a way to avoid this given
that we only know the table's target devices (and associated bdev and
request_queue) through each target's ctr():
stage1 (dmsetup load)
---------------------
all necessary validation is at table load time
=> dm-ioctl.c:table_load
=> dm-table.c:dm_table_create
=> dm-ioctl.c:populate_table
-> dm-table.c:dm_table_add_target
-> tgt->type->ctr()
-> dm-table.c:dm_get_device
-> dm-table.c:dm_set_device_limits
-> blk_stack_limits(ti, bdev->q->limits)
[NOTE: changed to: ti->limits = q->limits below]
[NOTE: this copy can't be delayed, need
access to target's bdev; only available through
ctr's params]
-> BEFORE: blk_stack_limits(table, ti->limits)
[NOTE: now delayed, moved to dm_table_set_restrictions]
-> dm-table.c:dm_table_complete
-> BEFORE: init_valid_queue_limits(table->limits)
[NOTE: now delayed, moved to dm_table_set_restrictions]
[NOTE: changed call to blk_set_default_limits]
stage2 (dmsetup resume)
-----------------------
swap_table: (use dm_table_set_restrictions hook)
1) inits table limits
2) iterates all target devices, stack limits
3) copies table limits to queue limits
=> dm.c:dm_swap_table
-> dm.c:__bind
-> dm-table.c:dm_table_set_restrictions
---
drivers/md/dm-table.c | 60 ++++++++++++++++----------------------------------
1 file changed, 20 insertions(+), 40 deletions(-)
Index: linux-2.6/drivers/md/dm-table.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-table.c
+++ linux-2.6/drivers/md/dm-table.c
@@ -492,9 +492,8 @@ void dm_set_device_limits(struct dm_targ
return;
}
- if (blk_stack_limits(&ti->limits, &q->limits, 0) < 0)
- DMWARN("%s: target device %s is misaligned",
- dm_device_name(ti->table->md), bdevname(bdev, b));
+ /* Copy queue_limits from underlying device */
+ ti->limits = q->limits;
/*
* Check if merge fn is supported.
@@ -643,34 +642,6 @@ int dm_split_args(int *argc, char ***arg
return 0;
}
-static void init_valid_queue_limits(struct queue_limits *limits)
-{
- if (!limits->max_sectors)
- limits->max_sectors = SAFE_MAX_SECTORS;
- if (!limits->max_hw_sectors)
- limits->max_hw_sectors = SAFE_MAX_SECTORS;
- if (!limits->max_phys_segments)
- limits->max_phys_segments = MAX_PHYS_SEGMENTS;
- if (!limits->max_hw_segments)
- limits->max_hw_segments = MAX_HW_SEGMENTS;
- if (!limits->logical_block_size)
- limits->logical_block_size = 1 << SECTOR_SHIFT;
- if (!limits->physical_block_size)
- limits->physical_block_size = 1 << SECTOR_SHIFT;
- if (!limits->io_min)
- limits->io_min = 1 << SECTOR_SHIFT;
- if (!limits->max_segment_size)
- limits->max_segment_size = MAX_SEGMENT_SIZE;
- if (!limits->seg_boundary_mask)
- limits->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
- if (!limits->bounce_pfn)
- limits->bounce_pfn = -1;
- /*
- * The other fields (alignment_offset, io_opt, misaligned)
- * hold 0 from the kzalloc().
- */
-}
-
/*
* Impose necessary and sufficient conditions on a devices's table such
* that any incoming bio which respects its logical_block_size can be
@@ -788,12 +759,6 @@ int dm_table_add_target(struct dm_table
t->highs[t->num_targets++] = tgt->begin + tgt->len - 1;
- if (blk_stack_limits(&t->limits, &tgt->limits, 0) < 0)
- DMWARN("%s: target device (start sect %llu len %llu) "
- "is misaligned",
- dm_device_name(t->md),
- (unsigned long long) tgt->begin,
- (unsigned long long) tgt->len);
return 0;
bad:
@@ -836,8 +801,6 @@ int dm_table_complete(struct dm_table *t
int r = 0;
unsigned int leaf_nodes;
- init_valid_queue_limits(&t->limits);
-
r = validate_hardware_logical_block_alignment(t);
if (r)
return r;
@@ -958,8 +921,25 @@ no_integrity:
void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
{
/*
- * Copy table's limits to the DM device's request_queue
+ * Initialize table's queue_limits and merge each underlying
+ * device's queue_limits with it
+ */
+ struct dm_target *uninitialized_var(ti);
+ unsigned i = 0;
+
+ blk_set_default_limits(&t->limits);
+ while (i < dm_table_get_num_targets(t)) {
+ ti = dm_table_get_target(t, i++);
+ (void)blk_stack_limits(&t->limits, &ti->limits, 0);
+ }
+
+ /*
+ * Each target device in the table has a data area that is aligned
+ * (via LVM2) so the DM device's alignment_offset should be 0.
*/
+ t->limits.alignment_offset = 0;
+
+ /* Copy table's queue_limits to the DM device's request_queue */
q->limits = t->limits;
if (t->limits.no_cluster)
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
