[PATCH] dm: Add support for data integrity to DM

[PATCH] dm: Add support for data integrity to DM

[Martin K. Petersen]

If all subdevices support the same protection format the DM device is
flagged as capable.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/md/dm-table.c         |   35 ++++++++++++++++++++++++++++++++++-
 drivers/md/dm.c               |   26 ++++++++++++++++++++++++--
 drivers/md/dm.h               |    2 +-
 include/linux/device-mapper.h |    1 +
 4 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 61f4414..006a10e 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -841,8 +841,12 @@ struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector)
 	return &t->targets[(KEYS_PER_NODE * n) + k];
 }
 
-void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
+void dm_table_set_restrictions(struct dm_table *t, struct mapped_device *md)
 {
+	struct request_queue *q = dm_queue(md);
+	struct list_head *devices = dm_table_get_devices(t);
+	struct dm_dev *prev, *cur;
+
 	/*
 	 * Make sure we obey the optimistic sub devices
 	 * restrictions.
@@ -861,6 +865,35 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
 	else
 		queue_flag_set_unlocked(QUEUE_FLAG_CLUSTER, q);
 
+	/*
+	 * Run through all devices to ensure they have matching
+	 * integrity profile
+	 */
+	cur = prev = NULL;
+
+	list_for_each_entry(cur, devices, list) {
+
+		if (prev && blk_integrity_compare(prev->bdev, cur->bdev) < 0) {
+			printk(KERN_ERR "%s: %s %s Integrity mismatch!\n",
+			       __func__, prev->bdev->bd_disk->disk_name,
+			       cur->bdev->bd_disk->disk_name);
+			return;
+		}
+		prev = cur;
+	}
+
+	/* Register dm device as being integrity capable */
+	if (prev && bdev_get_integrity(prev->bdev)) {
+		struct gendisk *disk = dm_disk(md);
+
+		if (blk_integrity_register(dm_disk(md), 
+					   bdev_get_integrity(prev->bdev)))
+			printk(KERN_ERR "%s: %s Could not register integrity!\n",
+			       __func__, disk->disk_name);
+		else
+			printk(KERN_INFO "Enabling data integrity on %s\n",
+			       disk->disk_name);
+	}
 }
 
 unsigned int dm_table_get_num_targets(struct dm_table *t)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8d40369..e3d14b0 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -669,6 +669,12 @@ static struct bio *split_bvec(struct bio *bio, sector_t sector,
 	clone->bi_size = to_bytes(len);
 	clone->bi_io_vec->bv_offset = offset;
 	clone->bi_io_vec->bv_len = clone->bi_size;
+	clone->bi_flags |= 1 << BIO_CLONED;
+
+	if (bio_integrity(bio)) {
+		bio_integrity_clone(clone, bio, bs);
+		bio_integrity_trim(clone, bio_sector_offset(bio, idx, offset), len);
+	}
 
 	return clone;
 }
@@ -691,6 +697,13 @@ static struct bio *clone_bio(struct bio *bio, sector_t sector,
 	clone->bi_size = to_bytes(len);
 	clone->bi_flags &= ~(1 << BIO_SEG_VALID);
 
+	if (bio_integrity(bio)) {
+		bio_integrity_clone(clone, bio, bs);
+
+		if (idx != bio->bi_idx || clone->bi_size < bio->bi_size)
+			bio_integrity_trim(clone, bio_sector_offset(bio, idx, 0), len);
+	}
+
 	return clone;
 }
 
@@ -1108,6 +1121,7 @@ static struct mapped_device *alloc_dev(int minor)
 	md->disk->queue = md->queue;
 	md->disk->private_data = md;
 	sprintf(md->disk->disk_name, "dm-%d", minor);
+	printk(KERN_ERR "DM: Created %s\n", md->disk->disk_name);
 	add_disk(md->disk);
 	format_dev_t(md->name, MKDEV(_major, minor));
 
@@ -1157,6 +1171,7 @@ static void free_dev(struct mapped_device *md)
 	mempool_destroy(md->tio_pool);
 	mempool_destroy(md->io_pool);
 	bioset_free(md->bs);
+	blk_integrity_unregister(md->disk);
 	del_gendisk(md->disk);
 	free_minor(minor);
 
@@ -1200,7 +1215,6 @@ static void __set_size(struct mapped_device *md, sector_t size)
 
 static int __bind(struct mapped_device *md, struct dm_table *t)
 {
-	struct request_queue *q = md->queue;
 	sector_t size;
 
 	size = dm_table_get_size(t);
@@ -1221,7 +1235,7 @@ static int __bind(struct mapped_device *md, struct dm_table *t)
 
 	write_lock(&md->map_lock);
 	md->map = t;
-	dm_table_set_restrictions(t, q);
+	dm_table_set_restrictions(t, md);
 	write_unlock(&md->map_lock);
 
 	return 0;
@@ -1674,9 +1688,17 @@ void dm_uevent_add(struct mapped_device *md, struct list_head *elist)
  */
 struct gendisk *dm_disk(struct mapped_device *md)
 {
+	BUG_ON(md == NULL);
+	BUG_ON(md->disk == NULL);
+
 	return md->disk;
 }
 
+struct request_queue *dm_queue(struct mapped_device *md)
+{
+	return md->queue;
+}
+
 int dm_suspended(struct mapped_device *md)
 {
 	return test_bit(DMF_SUSPENDED, &md->flags);
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 1e59a0b..907795f 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -43,7 +43,7 @@ void dm_table_event_callback(struct dm_table *t,
 			     void (*fn)(void *), void *context);
 struct dm_target *dm_table_get_target(struct dm_table *t, unsigned int index);
 struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector);
-void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q);
+void dm_table_set_restrictions(struct dm_table *t, struct mapped_device *md);
 struct list_head *dm_table_get_devices(struct dm_table *t);
 void dm_table_presuspend_targets(struct dm_table *t);
 void dm_table_postsuspend_targets(struct dm_table *t);
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index a90222e..5cf3a89 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -200,6 +200,7 @@ void dm_uevent_add(struct mapped_device *md, struct list_head *elist);
 const char *dm_device_name(struct mapped_device *md);
 int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid);
 struct gendisk *dm_disk(struct mapped_device *md);
+struct request_queue *dm_queue(struct mapped_device *md);
 int dm_suspended(struct mapped_device *md);
 int dm_noflush_suspending(struct dm_target *ti);
 
-- 
1.5.5.1

Re: [PATCH] dm: Add support for data integrity to DM

[Andrew Morton]

On Sat, 20 Sep 2008 20:16:21 -0400 "Martin K. Petersen" <martin.petersen@oracle.com> wrote:

> 
> If all subdevices support the same protection format the DM device is
> flagged as capable.
> 
> ..
..
> +	/* Register dm device as being integrity capable */
> +	if (prev && bdev_get_integrity(prev->bdev)) {
> +		struct gendisk *disk = dm_disk(md);
> +
> +		if (blk_integrity_register(dm_disk(md), 

Please use checkpatch.  Always.  There's just no reason not to.  Sure,
you can make a decision to ignore some of its reports, but at least
this avoids the accidental introduction of layout problems.

> +					   bdev_get_integrity(prev->bdev)))
> +			printk(KERN_ERR "%s: %s Could not register integrity!\n",
> +			       __func__, disk->disk_name);
> +		else
> +			printk(KERN_INFO "Enabling data integrity on %s\n",
> +			       disk->disk_name);
> +	}
>  }
>
> ...
>
> +	if (bio_integrity(bio)) {
> +		bio_integrity_clone(clone, bio, bs);
> +
> +		if (idx != bio->bi_idx || clone->bi_size < bio->bi_size)
> +			bio_integrity_trim(clone, bio_sector_offset(bio, idx, 0), len);

For better or for worse, the developers of dm.c have chosen to keep the
whole file presentable in an 80-col display.  This patch breaks that,
and they might not like this.

> +	}
> +
>  	return clone;
>  }
>  
> @@ -1108,6 +1121,7 @@ static struct mapped_device *alloc_dev(int minor)
>  	md->disk->queue = md->queue;
>  	md->disk->private_data = md;
>  	sprintf(md->disk->disk_name, "dm-%d", minor);
> +	printk(KERN_ERR "DM: Created %s\n", md->disk->disk_name);
>  	add_disk(md->disk);
>  	format_dev_t(md->name, MKDEV(_major, minor));
>  
> @@ -1157,6 +1171,7 @@ static void free_dev(struct mapped_device *md)
>  	mempool_destroy(md->tio_pool);
>  	mempool_destroy(md->io_pool);
>  	bioset_free(md->bs);
> +	blk_integrity_unregister(md->disk);
>  	del_gendisk(md->disk);
>  	free_minor(minor);
>  
> @@ -1200,7 +1215,6 @@ static void __set_size(struct mapped_device *md, sector_t size)
>  
>  static int __bind(struct mapped_device *md, struct dm_table *t)
>  {
> -	struct request_queue *q = md->queue;
>  	sector_t size;
>  
>  	size = dm_table_get_size(t);
> @@ -1221,7 +1235,7 @@ static int __bind(struct mapped_device *md, struct dm_table *t)
>  
>  	write_lock(&md->map_lock);
>  	md->map = t;
> -	dm_table_set_restrictions(t, q);
> +	dm_table_set_restrictions(t, md);
>  	write_unlock(&md->map_lock);
>  
>  	return 0;
> @@ -1674,9 +1688,17 @@ void dm_uevent_add(struct mapped_device *md, struct list_head *elist)
>   */
>  struct gendisk *dm_disk(struct mapped_device *md)
>  {
> +	BUG_ON(md == NULL);

This will provide no more information than the oops which will happen
two lines later.

> +	BUG_ON(md->disk == NULL);

well, that will provide a little bit of information I guess.

>  	return md->disk;
>  }
>  
> +struct request_queue *dm_queue(struct mapped_device *md)
> +{
> +	return md->queue;
> +}

This (unused, undocumented) function should be inlined.

Re: [PATCH] dm: Add support for data integrity to DM

[Martin K. Petersen]

>>>>> "Andrew" == Andrew Morton <akpm@linux-foundation.org> writes:

Sorry about the delayed response.  I'm still on the road after LPC.

Andrew> For better or for worse, the developers of dm.c have chosen to
Andrew> keep the whole file presentable in an 80-col display.  This
Andrew> patch breaks that, and they might not like this.

I've broken the lines.


[Excessive BUG_ONs]

Andrew> This will provide no more information than the oops which will
Andrew> happen two lines later.

Debugging debris.  Gone.


>> +struct request_queue *dm_queue(struct mapped_device *md)
>> +{
>> +    return md->queue;
>> +}

Andrew> This (unused, undocumented) function should be inlined.

 - It's not unused.

 - return md->queue is more descriptive than anything I could possibly
   put in a comment.

 - None of the other accessor functions in DM are inlined.  I'll let
   Alasdair decide.

Updated patches follow.

-- 
Martin K. Petersen      Oracle Linux Engineering

[PATCH 1/2] block: Find bio sector offset given idx and offset

[Martin K. Petersen]

Helper function to find the sector offset in a bio given bvec index
and page offset.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 fs/bio.c            |   34 ++++++++++++++++++++++++++++++++++
 include/linux/bio.h |    1 +
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/fs/bio.c b/fs/bio.c
index e56e768..55ee2fd 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -1300,6 +1300,40 @@ struct bio_pair *bio_split(struct bio *bi, mempool_t *pool, int first_sectors)
 	return bp;
 }
 
+/**
+ *	bio_sector_offset - Find hardware sector offset in bio
+ *	@bio:		bio to inspect
+ *	@index:		bio_vec index
+ *	@offset:	offset in bv_page
+ *
+ *	Return the number of hardware sectors between beginning of bio
+ *	and an end point indicated by a bio_vec index and an offset
+ *	within that vector's page.
+ */
+sector_t bio_sector_offset(struct bio *bio, unsigned short index,
+			   unsigned int offset)
+{
+	struct bio_vec *bv;
+	unsigned int sector_sz = bio->bi_bdev->bd_disk->queue->hardsect_size;
+	sector_t sectors;
+	int i;
+
+	sectors = 0;
+
+	BUG_ON(index >= bio->bi_vcnt);
+
+	bio_for_each_segment(bv, bio, i) {
+		if (i == index) {
+			if (offset > bv->bv_offset)
+				sectors += (offset - bv->bv_offset) / sector_sz;
+			return sectors;
+		}
+
+		sectors += bv->bv_len / sector_sz;
+	}
+
+	BUG();
+}
 
 /*
  * create memory pools for biovec's in a bio_set.
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 6aba97d..386a1df 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -327,6 +327,7 @@ extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int);
 extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *,
 			   unsigned int, unsigned int);
 extern int bio_get_nr_vecs(struct block_device *);
+extern sector_t bio_sector_offset(struct bio *, unsigned short, unsigned int);
 extern struct bio *bio_map_user(struct request_queue *, struct block_device *,
 				unsigned long, unsigned int, int, gfp_t);
 struct sg_iovec;
-- 
1.5.5.1

[PATCH 2/2] dm: Add support for data integrity to DM

[Martin K. Petersen]

If all subdevices support the same protection format the DM device is
flagged as capable.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/md/dm-table.c         |   35 ++++++++++++++++++++++++++++++++++-
 drivers/md/dm.c               |   24 ++++++++++++++++++++++--
 drivers/md/dm.h               |    2 +-
 include/linux/device-mapper.h |    1 +
 4 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 61f4414..7290a90 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -841,8 +841,12 @@ struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector)
 	return &t->targets[(KEYS_PER_NODE * n) + k];
 }
 
-void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
+void dm_table_set_restrictions(struct dm_table *t, struct mapped_device *md)
 {
+	struct request_queue *q = dm_queue(md);
+	struct list_head *devices = dm_table_get_devices(t);
+	struct dm_dev *prev, *cur;
+
 	/*
 	 * Make sure we obey the optimistic sub devices
 	 * restrictions.
@@ -861,6 +865,35 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
 	else
 		queue_flag_set_unlocked(QUEUE_FLAG_CLUSTER, q);
 
+	/*
+	 * Run through all devices to ensure they have matching
+	 * integrity profile
+	 */
+	cur = prev = NULL;
+
+	list_for_each_entry(cur, devices, list) {
+
+		if (prev && blk_integrity_compare(prev->bdev, cur->bdev) < 0) {
+			printk(KERN_ERR "%s: %s %s Integrity mismatch!\n",
+			       __func__, prev->bdev->bd_disk->disk_name,
+			       cur->bdev->bd_disk->disk_name);
+			return;
+		}
+		prev = cur;
+	}
+
+	/* Register dm device as being integrity capable */
+	if (prev && bdev_get_integrity(prev->bdev)) {
+		struct gendisk *disk = dm_disk(md);
+
+		if (blk_integrity_register(dm_disk(md),
+					   bdev_get_integrity(prev->bdev)))
+			printk(KERN_ERR "%s: %s Could not register integrity!\n",
+			       __func__, disk->disk_name);
+		else
+			printk(KERN_INFO "Enabling data integrity on %s\n",
+			       disk->disk_name);
+	}
 }
 
 unsigned int dm_table_get_num_targets(struct dm_table *t)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8d40369..2e27c6c 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -669,6 +669,13 @@ static struct bio *split_bvec(struct bio *bio, sector_t sector,
 	clone->bi_size = to_bytes(len);
 	clone->bi_io_vec->bv_offset = offset;
 	clone->bi_io_vec->bv_len = clone->bi_size;
+	clone->bi_flags |= 1 << BIO_CLONED;
+
+	if (bio_integrity(bio)) {
+		bio_integrity_clone(clone, bio, bs);
+		bio_integrity_trim(clone,
+				   bio_sector_offset(bio, idx, offset), len);
+	}
 
 	return clone;
 }
@@ -691,6 +698,14 @@ static struct bio *clone_bio(struct bio *bio, sector_t sector,
 	clone->bi_size = to_bytes(len);
 	clone->bi_flags &= ~(1 << BIO_SEG_VALID);
 
+	if (bio_integrity(bio)) {
+		bio_integrity_clone(clone, bio, bs);
+
+		if (idx != bio->bi_idx || clone->bi_size < bio->bi_size)
+			bio_integrity_trim(clone,
+					   bio_sector_offset(bio, idx, 0), len);
+	}
+
 	return clone;
 }
 
@@ -1157,6 +1172,7 @@ static void free_dev(struct mapped_device *md)
 	mempool_destroy(md->tio_pool);
 	mempool_destroy(md->io_pool);
 	bioset_free(md->bs);
+	blk_integrity_unregister(md->disk);
 	del_gendisk(md->disk);
 	free_minor(minor);
 
@@ -1200,7 +1216,6 @@ static void __set_size(struct mapped_device *md, sector_t size)
 
 static int __bind(struct mapped_device *md, struct dm_table *t)
 {
-	struct request_queue *q = md->queue;
 	sector_t size;
 
 	size = dm_table_get_size(t);
@@ -1221,7 +1236,7 @@ static int __bind(struct mapped_device *md, struct dm_table *t)
 
 	write_lock(&md->map_lock);
 	md->map = t;
-	dm_table_set_restrictions(t, q);
+	dm_table_set_restrictions(t, md);
 	write_unlock(&md->map_lock);
 
 	return 0;
@@ -1677,6 +1692,11 @@ struct gendisk *dm_disk(struct mapped_device *md)
 	return md->disk;
 }
 
+struct request_queue *dm_queue(struct mapped_device *md)
+{
+	return md->queue;
+}
+
 int dm_suspended(struct mapped_device *md)
 {
 	return test_bit(DMF_SUSPENDED, &md->flags);
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 1e59a0b..907795f 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -43,7 +43,7 @@ void dm_table_event_callback(struct dm_table *t,
 			     void (*fn)(void *), void *context);
 struct dm_target *dm_table_get_target(struct dm_table *t, unsigned int index);
 struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector);
-void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q);
+void dm_table_set_restrictions(struct dm_table *t, struct mapped_device *md);
 struct list_head *dm_table_get_devices(struct dm_table *t);
 void dm_table_presuspend_targets(struct dm_table *t);
 void dm_table_postsuspend_targets(struct dm_table *t);
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index a90222e..5cf3a89 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -200,6 +200,7 @@ void dm_uevent_add(struct mapped_device *md, struct list_head *elist);
 const char *dm_device_name(struct mapped_device *md);
 int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid);
 struct gendisk *dm_disk(struct mapped_device *md);
+struct request_queue *dm_queue(struct mapped_device *md);
 int dm_suspended(struct mapped_device *md);
 int dm_noflush_suspending(struct dm_target *ti);
 
-- 
1.5.5.1

Re: [dm-devel] [PATCH 2/2] dm: Add support for data integrity to DM

[Kiyoshi Ueda]

Hi Martin,

On Tue, 23 Sep 2008 14:41:12 -0400, "Martin K. Petersen" wrote:
> If all subdevices support the same protection format the DM device is
> flagged as capable.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>  drivers/md/dm-table.c         |   35 ++++++++++++++++++++++++++++++++++-
>  drivers/md/dm.c               |   24 ++++++++++++++++++++++--
>  drivers/md/dm.h               |    2 +-
>  include/linux/device-mapper.h |    1 +
>  4 files changed, 58 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 61f4414..7290a90 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -841,8 +841,12 @@ struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector)
>  	return &t->targets[(KEYS_PER_NODE * n) + k];
>  }
>  
> -void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
> +void dm_table_set_restrictions(struct dm_table *t, struct mapped_device *md)
>  {
> +	struct request_queue *q = dm_queue(md);
> +	struct list_head *devices = dm_table_get_devices(t);
> +	struct dm_dev *prev, *cur;
> +
>  	/*
>  	 * Make sure we obey the optimistic sub devices
>  	 * restrictions.
> @@ -861,6 +865,35 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
>  	else
>  		queue_flag_set_unlocked(QUEUE_FLAG_CLUSTER, q);
>  
> +	/*
> +	 * Run through all devices to ensure they have matching
> +	 * integrity profile
> +	 */
> +	cur = prev = NULL;
> +
> +	list_for_each_entry(cur, devices, list) {
> +
> +		if (prev && blk_integrity_compare(prev->bdev, cur->bdev) < 0) {
> +			printk(KERN_ERR "%s: %s %s Integrity mismatch!\n",
> +			       __func__, prev->bdev->bd_disk->disk_name,
> +			       cur->bdev->bd_disk->disk_name);
> +			return;
> +		}
> +		prev = cur;
> +	}
> +
> +	/* Register dm device as being integrity capable */
> +	if (prev && bdev_get_integrity(prev->bdev)) {
> +		struct gendisk *disk = dm_disk(md);
> +
> +		if (blk_integrity_register(dm_disk(md),
> +					   bdev_get_integrity(prev->bdev)))
> +			printk(KERN_ERR "%s: %s Could not register integrity!\n",
> +			       __func__, disk->disk_name);
> +		else
> +			printk(KERN_INFO "Enabling data integrity on %s\n",
> +			       disk->disk_name);
> +	}
>  }
<snip>
> @@ -1200,7 +1216,6 @@ static void __set_size(struct mapped_device *md, sector_t size)
>  
>  static int __bind(struct mapped_device *md, struct dm_table *t)
>  {
> -	struct request_queue *q = md->queue;
>  	sector_t size;
>  
>  	size = dm_table_get_size(t);
> @@ -1221,7 +1236,7 @@ static int __bind(struct mapped_device *md, struct dm_table *t)
>  
>  	write_lock(&md->map_lock);
>  	md->map = t;
> -	dm_table_set_restrictions(t, q);
> +	dm_table_set_restrictions(t, md);
>  	write_unlock(&md->map_lock);
>  
>  	return 0;

Creating another function (e.g. dm_table_set_integrity(t, md)) may
be better, since then you wouldn't need dm_queue() nor need to change
the function interface of dm_table_set_restrictions() I think.


> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index a90222e..5cf3a89 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -200,6 +200,7 @@ void dm_uevent_add(struct mapped_device *md, struct list_head *elist);
>  const char *dm_device_name(struct mapped_device *md);
>  int dm_copy_name_and_uuid(struct mapped_device *md, char *name, char *uuid);
>  struct gendisk *dm_disk(struct mapped_device *md);
> +struct request_queue *dm_queue(struct mapped_device *md);
>  int dm_suspended(struct mapped_device *md);
>  int dm_noflush_suspending(struct dm_target *ti);

If you need dm_queue() anyway, this part should be in drivers/md/dm.h
because other kernel modules like dm target drivers (e.g. dm-mirror)
don't use it.

Thanks,
Kiyoshi Ueda

Re: [dm-devel] [PATCH 2/2] dm: Add support for data integrity to DM

[Kiyoshi Ueda]

Hi Martin,

On Tue, 23 Sep 2008 15:29:42 -0400 (EDT), Kiyoshi Ueda wrote:
> On Tue, 23 Sep 2008 14:41:12 -0400, "Martin K. Petersen" wrote:
> > If all subdevices support the same protection format the DM device is
> > flagged as capable.
> > 
> > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> > ---
> >  drivers/md/dm-table.c         |   35 ++++++++++++++++++++++++++++++++++-
> >  drivers/md/dm.c               |   24 ++++++++++++++++++++++--
> >  drivers/md/dm.h               |    2 +-
> >  include/linux/device-mapper.h |    1 +
> >  4 files changed, 58 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> > index 61f4414..7290a90 100644
> > --- a/drivers/md/dm-table.c
> > +++ b/drivers/md/dm-table.c
> > @@ -841,8 +841,12 @@ struct dm_target *dm_table_find_target(struct dm_table *t, sector_t sector)
> >  	return &t->targets[(KEYS_PER_NODE * n) + k];
> >  }
> >  
> > -void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
> > +void dm_table_set_restrictions(struct dm_table *t, struct mapped_device *md)
> >  {
> > +	struct request_queue *q = dm_queue(md);
> > +	struct list_head *devices = dm_table_get_devices(t);
> > +	struct dm_dev *prev, *cur;
> > +
> >  	/*
> >  	 * Make sure we obey the optimistic sub devices
> >  	 * restrictions.
> > @@ -861,6 +865,35 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q)
> >  	else
> >  		queue_flag_set_unlocked(QUEUE_FLAG_CLUSTER, q);
> >  
> > +	/*
> > +	 * Run through all devices to ensure they have matching
> > +	 * integrity profile
> > +	 */
> > +	cur = prev = NULL;
> > +
> > +	list_for_each_entry(cur, devices, list) {
> > +
> > +		if (prev && blk_integrity_compare(prev->bdev, cur->bdev) < 0) {
> > +			printk(KERN_ERR "%s: %s %s Integrity mismatch!\n",
> > +			       __func__, prev->bdev->bd_disk->disk_name,
> > +			       cur->bdev->bd_disk->disk_name);
> > +			return;
> > +		}
> > +		prev = cur;
> > +	}
> > +
> > +	/* Register dm device as being integrity capable */
> > +	if (prev && bdev_get_integrity(prev->bdev)) {
> > +		struct gendisk *disk = dm_disk(md);
> > +
> > +		if (blk_integrity_register(dm_disk(md),
> > +					   bdev_get_integrity(prev->bdev)))
> > +			printk(KERN_ERR "%s: %s Could not register integrity!\n",
> > +			       __func__, disk->disk_name);
> > +		else
> > +			printk(KERN_INFO "Enabling data integrity on %s\n",
> > +			       disk->disk_name);
> > +	}
> >  }
> <snip>
> > @@ -1200,7 +1216,6 @@ static void __set_size(struct mapped_device *md, sector_t size)
> >  
> >  static int __bind(struct mapped_device *md, struct dm_table *t)
> >  {
> > -	struct request_queue *q = md->queue;
> >  	sector_t size;
> >  
> >  	size = dm_table_get_size(t);
> > @@ -1221,7 +1236,7 @@ static int __bind(struct mapped_device *md, struct dm_table *t)
> >  
> >  	write_lock(&md->map_lock);
> >  	md->map = t;
> > -	dm_table_set_restrictions(t, q);
> > +	dm_table_set_restrictions(t, md);
> >  	write_unlock(&md->map_lock);
> >  
> >  	return 0;
> 
> Creating another function (e.g. dm_table_set_integrity(t, md)) may
> be better, since then you wouldn't need dm_queue() nor need to change
> the function interface of dm_table_set_restrictions() I think.

Or you can also get md from t->md or q->queuedata, if you don't mind
to pass q as an argument of the function which sets integrity feature.
(I thought you mind that, since the integrity feature is set to
 md (md->disk), not q.)

Thanks,
Kiyoshi Ueda

Re: [dm-devel] [PATCH 2/2] dm: Add support for data integrity to DM

[Martin K. Petersen]

>>>>> "Kiyoshi" == Kiyoshi Ueda <k-ueda@ct.jp.nec.com> writes:

Kiyoshi> Creating another function (e.g. dm_table_set_integrity(t,
Kiyoshi> md)) may be better, since then you wouldn't need dm_queue()
Kiyoshi> nor need to change the function interface of
Kiyoshi> dm_table_set_restrictions() I think.

You are right.  Originally I had more reasons for having that calling
interface change but that's no longer the case.  I'll fan the
integrity bits out to a separate function.

-- 
Martin K. Petersen	Oracle Linux Engineering