[PATCH v2] dm table: verify a DM device is H/W sector aligned

[PATCH v2] dm table: verify a DM device is H/W sector aligned

[Mike Snitzer]

Verify a table's targets can collectively accommodate the device's
hardsect_size aligned I/O in addition to their own hardsect_size
alignment; paying attention to target boundaries in the table.
- incoming I/O is aligned to the device's hardsect_size
- each target is aligned to the target's hardsect_size
- device's hardsect_size >= each targets' hardsect_size
- device and targets' hardsect_size are always powers of 2

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-table.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 61a05cd..ebb82b4 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -719,6 +719,65 @@ static void check_for_valid_limits(struct io_restrictions *rs)
 		rs->bounce_pfn = -1;
 }
 
+/*
+ * Verify a table's targets can collectively accommodate the device's
+ * hardsect_size aligned I/O in addition to their own hardsect_size
+ * alignment; paying attention to target boundaries in the table
+ * - incoming I/O is aligned to the device's hardsect_size
+ * - each target is aligned to the target's hardsect_size
+ * - device's hardsect_size >= each targets' hardsect_size
+ * - device and targets' hardsect_size are always powers of 2
+ */
+static int alignment_is_valid(struct dm_table *table)
+{
+	int r = 0;
+	unsigned int i, num_targets;
+	struct dm_target *ti = NULL;
+	unsigned short partial_sectors, remaining_sectors = 0;
+	unsigned short ti_hardsect_size_sectors = 0;
+	unsigned short device_hardsect_size_sectors =
+		table->limits.hardsect_size >> SECTOR_SHIFT;
+
+	num_targets = dm_table_get_num_targets(table);
+	for (i = 0; i < num_targets; i++) {
+		ti = dm_table_get_target(table, i);
+		ti_hardsect_size_sectors =
+			ti->limits.hardsect_size >> SECTOR_SHIFT;
+
+		/*
+		 * Check that remaining_sectors from the previous target
+		 * do not disturb this target's hardsect_size alignment
+		 * - if remaining_sectors is greater than this
+		 *   target's len: it is the next target's concern
+		 */
+		if ((remaining_sectors && remaining_sectors < ti->len) &&
+		    (remaining_sectors & (ti_hardsect_size_sectors - 1)))
+			break;
+
+		/*
+		 * If device's hardsect_size aligned I/O crosses target
+		 * boundaries any remaining_sectors must be handled by
+		 * the next target(s) -- otherwise the table is misaligned
+		 */
+		remaining_sectors = 0;
+		partial_sectors =
+			(unsigned short) ((ti->begin + ti->len) &
+					  (device_hardsect_size_sectors - 1));
+		if (partial_sectors)
+			remaining_sectors = device_hardsect_size_sectors -
+				partial_sectors;
+	}
+
+	if (remaining_sectors) {
+		DMWARN("%s: mapping with begin=%lu len=%lu "
+		       "not device h/w sector aligned",
+		       dm_device_name(table->md), ti->begin, ti->len);
+		r = -EINVAL;
+	}
+
+	return r;
+}
+
 int dm_table_add_target(struct dm_table *t, const char *type,
 			sector_t start, sector_t len, char *params)
 {
@@ -818,6 +877,10 @@ int dm_table_complete(struct dm_table *t)
 
 	check_for_valid_limits(&t->limits);
 
+	r = alignment_is_valid(t);
+	if (r)
+		return r;
+
 	/* how many indexes will the btree have ? */
 	leaf_nodes = dm_div_up(t->num_targets, KEYS_PER_NODE);
 	t->depth = 1 + int_log(leaf_nodes, CHILDREN_PER_NODE);
-- 
1.6.0.6

Re: [PATCH v2] dm table: verify a DM device is H/W sector aligned

[Alasdair G Kergon]

I've tried to make this easier to understand.
Please check I've not broken something in the process...


From: Mike Snitzer <snitzer@redhat.com>

Impose necessary and sufficient conditions on a devices's table such
that any incoming bio which respects its hardsect_size can be processed
successfully.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>

---
 drivers/md/dm-table.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

Index: linux-2.6.30-rc2/drivers/md/dm-table.c
===================================================================
--- linux-2.6.30-rc2.orig/drivers/md/dm-table.c
+++ linux-2.6.30-rc2/drivers/md/dm-table.c
@@ -719,6 +719,70 @@ static void check_for_valid_limits(struc
 		rs->bounce_pfn = -1;
 }
 
+/*
+ * Impose necessary and sufficient conditions on a devices's table such
+ * that any incoming bio which respects its hardsect_size can be
+ * processed successfully.  If it falls across the boundary between
+ * two or more targets, the size of each piece it gets split into must
+ * be compatible with the hardsect_size of the target processing it.
+ */
+static int validate_hardsect_alignment(struct dm_table *table)
+{
+	/*
+	 * This function uses arithmetic modulo the hardsect_size
+	 * (in units of 512-byte sectors).
+	 */
+	unsigned short device_hardsect_size_sects =
+			    table->limits.hardsect_size >> SECTOR_SHIFT;
+
+	/*
+	 * Offset of the start of the next table entry, mod hardsect_size.
+	 */
+	unsigned short next_target_start = 0;
+
+	/*
+	 * Given an aligned bio that extends beyond the end of a
+	 * target, how many sectors must the next target handle?
+	 */
+	unsigned short remaining = 0;
+
+	struct dm_target *ti;
+	unsigned i = 0;
+
+	/*
+	 * Check each entry in the table in turn.
+	 */
+	while (i < dm_table_get_num_targets(table)) {
+		ti = dm_table_get_target(table, i++);
+
+		/*
+		 * If the remaining sectors fall entirely within this
+		 * table entry are they compatible with its hardsect_size?
+		 */
+		if (remaining < ti->len &&
+		    remaining & (ti->limits.hardsect_size >> SECTOR_SHIFT) - 1)
+			break;	/* Error */
+
+		next_target_start =
+		    (unsigned short) ((next_target_start + ti->len) &
+				      (device_hardsect_size_sects - 1));
+		remaining = next_target_start ?
+		    device_hardsect_size_sects - next_target_start : 0;
+	}
+
+	if (remaining) {
+		DMWARN("%s: table line %u (start sect %llu len %llu) "
+		       "not aligned to hardware sector size %hu",
+		       dm_device_name(table->md), i,
+		       (unsigned long long) ti->begin,
+		       (unsigned long long) ti->len,
+		       table->limits.hardsect_size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int dm_table_add_target(struct dm_table *t, const char *type,
 			sector_t start, sector_t len, char *params)
 {
@@ -818,6 +882,10 @@ int dm_table_complete(struct dm_table *t
 
 	check_for_valid_limits(&t->limits);
 
+	r = validate_hardsect_alignment(t);
+	if (r)
+		return r;
+
 	/* how many indexes will the btree have ? */
 	leaf_nodes = dm_div_up(t->num_targets, KEYS_PER_NODE);
 	t->depth = 1 + int_log(leaf_nodes, CHILDREN_PER_NODE);

Re: [PATCH v2] dm table: verify a DM device is H/W sector aligned

[Mike Snitzer]

On Wed, Apr 22 2009 at  9:25am -0400,
Alasdair G Kergon <agk@redhat.com> wrote:

> I've tried to make this easier to understand.
> Please check I've not broken something in the process...

Looks good, I especially like what you did with the revised DM_WARN().
But there were some minor compiler warnings:

drivers/md/dm-table.c: In function ‘validate_hardsect_alignment’:
drivers/md/dm-table.c:763: warning: suggest parentheses around + or - in operand of &
drivers/md/dm-table.c: In function ‘dm_table_complete’:
drivers/md/dm-table.c:749: warning: ‘ti’ may be used uninitialized in this function

I fixed these warnings in the following revised patch.  I then fixed
this checkpatch warning (which resulted in somewhat uglier code):

WARNING: line over 80 characters
#74: FILE: drivers/md/dm-table.c:763:
+                   remaining & ((ti->limits.hardsect_size >> SECTOR_SHIFT) - 1))


diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 429b50b..73f0438 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -693,6 +693,71 @@ static void check_for_valid_limits(struct io_restrictions *rs)
 		rs->bounce_pfn = -1;
 }
 
+/*
+ * Impose necessary and sufficient conditions on a devices's table such
+ * that any incoming bio which respects its hardsect_size can be
+ * processed successfully.  If it falls across the boundary between
+ * two or more targets, the size of each piece it gets split into must
+ * be compatible with the hardsect_size of the target processing it.
+ */
+static int validate_hardsect_alignment(struct dm_table *table)
+{
+	/*
+	 * This function uses arithmetic modulo the hardsect_size
+	 * (in units of 512-byte sectors).
+	 */
+	unsigned short device_hardsect_size_sects =
+			    table->limits.hardsect_size >> SECTOR_SHIFT;
+
+	/*
+	 * Offset of the start of the next table entry, mod hardsect_size.
+	 */
+	unsigned short next_target_start = 0;
+
+	/*
+	 * Given an aligned bio that extends beyond the end of a
+	 * target, how many sectors must the next target handle?
+	 */
+	unsigned short remaining = 0;
+
+	struct dm_target *ti = NULL;
+	unsigned i = 0;
+
+	/*
+	 * Check each entry in the table in turn.
+	 */
+	while (i < dm_table_get_num_targets(table)) {
+		ti = dm_table_get_target(table, i++);
+
+		/*
+		 * If the remaining sectors fall entirely within this
+		 * table entry are they compatible with its hardsect_size?
+		 */
+		if (remaining < ti->len &&
+		    remaining & ((ti->limits.hardsect_size >>
+				  SECTOR_SHIFT) - 1))
+			break;	/* Error */
+
+		next_target_start =
+		    (unsigned short) ((next_target_start + ti->len) &
+				      (device_hardsect_size_sects - 1));
+		remaining = next_target_start ?
+		    device_hardsect_size_sects - next_target_start : 0;
+	}
+
+	if (remaining) {
+		DMWARN("%s: table line %u (start sect %llu len %llu) "
+		       "not aligned to hardware sector size %hu",
+		       dm_device_name(table->md), i,
+		       (unsigned long long) ti->begin,
+		       (unsigned long long) ti->len,
+		       table->limits.hardsect_size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int dm_table_add_target(struct dm_table *t, const char *type,
 			sector_t start, sector_t len, char *params)
 {
@@ -792,6 +857,10 @@ int dm_table_complete(struct dm_table *t)
 
 	check_for_valid_limits(&t->limits);
 
+	r = validate_hardsect_alignment(t);
+	if (r)
+		return r;
+
 	/* how many indexes will the btree have ? */
 	leaf_nodes = dm_div_up(t->num_targets, KEYS_PER_NODE);
 	t->depth = 1 + int_log(leaf_nodes, CHILDREN_PER_NODE);