block, dm: use __blk_integrity_register to properly setup integrity profile

PATCH In review Mike Snitzer 3 participants 6 messages Apr 21, 2017 – Apr 23, 2017

Subsystems (2)

Block layer Device mapper

lore

Share & tools

lore

Mike Snitzer

author

2017-04-21 22:28 UTC #0 lore raw

PATCH 1/1 block, dm: use __blk_integrity_register to properly setup integrity profile

From: Mikulas Patocka <[email protected]>

When registering integrity profile on a DM device, the logical block size of the underlying device with integrity support must be used, not the logical block size of the stacked device we are creating.

Introduce __blk_integrity_register() to allow users more precise control over the logical block size used to register the integrity profile. blk_integrity_register() now calls __blk_integrity_register().

DM core's use of __blk_integrity_register() fixes a long-standing DM linear target bug where it cannot pass integrity data to the underlying device if its logical block size conflicts with the underlying device's logical block size.

Cc [email protected]

Signed-off-by Mikulas Patocka <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

---
 block/blk-integrity.c  | 14 +++++++++++---
 drivers/md/dm-table.c  |  6 ++++--
 include/linux/blkdev.h |  2 ++
 3 files changed, 17 insertions(+), 5 deletions(-)

NOTE: Jens, I'd like to get this staged in linux-dm.git for 4.12 because the dm-inegrity target depends on it, doing so would avoid some awkward coordination between block and DM trees. SO if you're OK with this patch, and with me sending it to Linus as part of the DM pull, please provide your Ack. Thanks!

block/blk-integrity.c +11 -3

index 9f0ff5b..d2a36e3 100644
@@ -396,9 +396,10 @@ static struct blk_integrity_profile nop_profile = {
 };
 
 /**
- * blk_integrity_register - Register a gendisk as being integrity-capable
+ * __blk_integrity_register - Register a gendisk as being integrity-capable
  * @disk:	struct gendisk pointer to make integrity-aware
  * @template:	block integrity profile to register
+ * @logical_block_size:	block size - integrity tag exists for each block
  *
  * Description: When a device needs to advertise itself as being able to
  * send/receive integrity metadata it must use this function to register

@@ -406,19 +407,26 @@ static struct blk_integrity_profile nop_profile = {
  * struct with values appropriate for the underlying hardware. See
  * Documentation/block/data-integrity.txt.
  */
-void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
+void __blk_integrity_register(struct gendisk *disk, struct blk_integrity *template,
+			      unsigned logical_block_size)
 {
 	struct blk_integrity *bi = &disk->queue->integrity;
 
 	bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
 		template->flags;
-	bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
+	bi->interval_exp = ilog2(logical_block_size);
 	bi->profile = template->profile ? template->profile : &nop_profile;
 	bi->tuple_size = template->tuple_size;
 	bi->tag_size = template->tag_size;
 
 	blk_integrity_revalidate(disk);
 }
+EXPORT_SYMBOL(__blk_integrity_register);
+
+void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template)
+{
+	__blk_integrity_register(disk, template, queue_logical_block_size(disk->queue));
+}
 EXPORT_SYMBOL(blk_integrity_register);
 
 /**

drivers/md/dm-table.c +4 -2

index b060084..b7a5843 100644
@@ -1181,13 +1181,15 @@ static int dm_table_register_integrity(struct dm_table *t)
 		return 0;
 
 	if (!integrity_profile_exists(dm_disk(md))) {
+		struct blk_integrity *integrity = blk_get_integrity(template_disk);
+
 		t->integrity_supported = true;
 		/*
 		 * Register integrity profile during table load; we can do
 		 * this because the final profile must match during resume.
 		 */
-		blk_integrity_register(dm_disk(md),
-				       blk_get_integrity(template_disk));
+		__blk_integrity_register(dm_disk(md), integrity,
+					 1U << integrity->interval_exp);
 		return 0;
 	}

include/linux/blkdev.h +2 -0

index 796016e..353400c 100644
@@ -1778,6 +1778,8 @@ struct blk_integrity_profile {
 	const char			*name;
 };
 
+extern void __blk_integrity_register(struct gendisk *, struct blk_integrity *,
+				     unsigned);
 extern void blk_integrity_register(struct gendisk *, struct blk_integrity *);
 extern void blk_integrity_unregister(struct gendisk *);
 extern int blk_integrity_compare(struct gendisk *, struct gendisk *);

Signature · 1 line

2.10.1

Martin K. Petersen

2017-04-21 22:43 UTC #1 lore raw

Mike/Mikulas,

Quoted 3 lines

Introduce __blk_integrity_register() to allow users more precise control over the logical block size used to register the integrity profile. blk_integrity_register() now calls __blk_integrity_register().

This seems a bit kludgy. It would be cleaner having the integrity interval be explicitly specified in the integrity template for all callers. Maybe with a fallback to qlbs if it's set to 0.

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Mike Snitzer

author

2017-04-22 15:10 UTC #2 lore raw

Quoted 10 lines · Martin K. Petersen

Mike/Mikulas,
Quoted 3 lines
Introduce __blk_integrity_register() to allow users more precise control over the logical block size used to register the integrity profile. blk_integrity_register() now calls __blk_integrity_register().
This seems a bit kludgy. It would be cleaner having the integrity interval be explicitly specified in the integrity template for all callers. Maybe with a fallback to qlbs if it's set to 0.

Fair enough, I'll revise and send v2 shortly. Thanks.

Mike Snitzer

author

2017-04-22 21:22 UTC #3 lore raw

PATCH v2 1/1 block: fix blk_integrity_register to use template's interval_exp if not 0

When registering an integrity profile: if the template's interval_exp is not 0 use it, otherwise use the ilog2() of logical block size of the provided gendisk.

This fixes a long-standing DM linear target bug where it cannot pass integrity data to the underlying device if its logical block size conflicts with the underlying device's logical block size.

Cc [email protected]

Reported-by Mikulas Patocka <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

---
 block/blk-integrity.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Jens, please feel free to pick this up for 4.12 and I'll just make the dm-integrity commit Depends-on it once you've staged it.

block/blk-integrity.c +2 -1

index 9f0ff5b..d6e1b9a 100644
@@ -412,7 +412,8 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template
 
 	bi->flags = BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE |
 		template->flags;
-	bi->interval_exp = ilog2(queue_logical_block_size(disk->queue));
+	bi->interval_exp = template->interval_exp ? :
+		ilog2(queue_logical_block_size(disk->queue));
 	bi->profile = template->profile ? template->profile : &nop_profile;
 	bi->tuple_size = template->tuple_size;
 	bi->tag_size = template->tag_size;

Signature · 1 line

2.10.1

Martin K. Petersen

Acked-by on v2 1/1

2017-04-23 18:42 UTC #4 lore raw

Re: [PATCH v2 for-4.12] block: fix blk_integrity_register to use template's interval_exp if not 0

Mike,

Quoted 7 lines

When registering an integrity profile: if the template's interval_exp is not 0 use it, otherwise use the ilog2() of logical block size of the provided gendisk.
This fixes a long-standing DM linear target bug where it cannot pass integrity data to the underlying device if its logical block size conflicts with the underlying device's logical block size.

Looks good!

Acked-by Martin K. Petersen <[email protected]>

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Jens Axboe

maintainer

2017-04-23 19:00 UTC #5 lore raw

Re: [PATCH v2 for-4.12] block: fix blk_integrity_register to use template's interval_exp if not 0

In reply to Mike Snitzer

Quoted 11 lines · Mike Snitzer

When registering an integrity profile: if the template's interval_exp is not 0 use it, otherwise use the ilog2() of logical block size of the provided gendisk.
This fixes a long-standing DM linear target bug where it cannot pass integrity data to the underlying device if its logical block size conflicts with the underlying device's logical block size.
Cc [email protected]
Reported-by Mikulas Patocka <[email protected]>
Signed-off-by Mike Snitzer <[email protected]>

Quoted 6 lines

---
 block/blk-integrity.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Jens, please feel free to pick this up for 4.12 and I'll just make the dm-integrity commit Depends-on it once you've staged it.

OK, I'll commit it. I'll send out my pull request as soon as the merge window opens, so you should not have any delays.

Signature · 1 line

Jens Axboe