[PATCH] block: pass correct seed to integrity metadata generation function

[PATCH] block: pass correct seed to integrity metadata generation function

[Sam Bradshaw]

The seed value passed to the blk integrity metadata generation function 
was wrong depending on the device block size (interval) and how many 
blocks comprised the bvec.  This patch converts the seed to 'interval' 
units and increments it correctly for each iteration.  Tested against a
4k+8 (w/ type1 PI) sector size.

Signed-off-by: Sam Bradshaw <sbradshaw@micron.com>
Signed-off-by: Selvan Mani <smani@micron.com>
---
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 5cbd5d9..7114040 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -219,20 +219,22 @@ static int bio_integrity_process(struct bio *bio,
 	struct bvec_iter bviter;
 	struct bio_vec bv;
 	struct bio_integrity_payload *bip = bio_integrity(bio);
-	unsigned int ret = 0;
+	unsigned int len = 0, ret = 0;
 	void *prot_buf = page_address(bip->bip_vec->bv_page) +
 		bip->bip_vec->bv_offset;
+	sector_t seed, start = bio_integrity_intervals(bi, bip_get_seed(bip));
 
 	iter.disk_name = bio->bi_bdev->bd_disk->disk_name;
 	iter.interval = bi->interval;
-	iter.seed = bip_get_seed(bip);
-	iter.prot_buf = prot_buf;
+	seed = start;
 
 	bio_for_each_segment(bv, bio, bviter) {
 		void *kaddr = kmap_atomic(bv.bv_page);
 
 		iter.data_buf = kaddr + bv.bv_offset;
 		iter.data_size = bv.bv_len;
+		iter.seed = seed;
+		iter.prot_buf = prot_buf;
 
 		ret = proc_fn(&iter);
 		if (ret) {
@@ -241,6 +243,9 @@ static int bio_integrity_process(struct bio *bio,
 		}
 
 		kunmap_atomic(kaddr);
+		len += bv.bv_len;
+		seed = start + (len / bi->interval);
+		prot_buf += bi->tuple_size;
 	}
 	return ret;
 }

Re: [PATCH] block: pass correct seed to integrity metadata generation function

[Sam Bradshaw]

On 12/18/2014 04:11 PM, Sam Bradshaw wrote:
> The seed value passed to the blk integrity metadata generation function
> was wrong depending on the device block size (interval) and how many
> blocks comprised the bvec.  This patch converts the seed to 'interval'
> units and increments it correctly for each iteration.  Tested against a
> 4k+8 (w/ type1 PI) sector size.

Any particular objection to this patch?  It would be nice if the kernel
supported PI on block sizes other than just 512 bytes.

Re: [PATCH] block: pass correct seed to integrity metadata generation function

[Martin K. Petersen]

>>>>> "Sam" == Sam Bradshaw <sbradshaw@micron.com> writes:

Sam> Any particular objection to this patch?  It would be nice if the
Sam> kernel supported PI on block sizes other than just 512 bytes.

I never saw the patch. Did you CC: me on it?

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH] block: pass correct seed to integrity metadata generation function

[Sam Bradshaw (sbradshaw)]

> Sam> Any particular objection to this patch?  It would be nice if the
> Sam> kernel supported PI on block sizes other than just 512 bytes.
> 
> I never saw the patch. Did you CC: me on it?

Yes, you were CC: on the original.  Could you take a peek at the patch 
and give me feedback?
https://lkml.org/lkml/2014/12/18/521

Re: [PATCH] block: pass correct seed to integrity metadata generation function

[Martin K. Petersen]

>>>>> "Sam" == Sam Bradshaw (sbradshaw) <sbradshaw@micron.com> writes:

Sam> Yes, you were CC: on the original.  Could you take a peek at the
Sam> patch and give me feedback?  https://lkml.org/lkml/2014/12/18/521

Is the target an NVMe device?

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH] block: pass correct seed to integrity metadata generation function

[Sam Bradshaw (sbradshaw)]

> -----Original Message-----
> From: Martin K. Petersen [mailto:martin.petersen@oracle.com]
> Sent: Wednesday, January 07, 2015 3:43 PM
> To: Sam Bradshaw (sbradshaw)
> Cc: Martin K. Petersen; axboe@kernel.dk; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] block: pass correct seed to integrity metadata
> generation function
> 
> >>>>> "Sam" == Sam Bradshaw (sbradshaw) <sbradshaw@micron.com> writes:
> 
> Sam> Yes, you were CC: on the original.  Could you take a peek at the
> Sam> patch and give me feedback?  https://lkml.org/lkml/2014/12/18/521
> 
> Is the target an NVMe device?

Yes.

Re: [PATCH] block: pass correct seed to integrity metadata generation function

[Martin K. Petersen]

>>>>> "Sam" == Sam Bradshaw (sbradshaw) <sbradshaw@micron.com> writes:

Sam> Yes.

The seed is just a seed. We happen to set it to the (block layer) sector
number if nothing else is provided but it's essentially just an
incrementing counter starting at an arbitrary value chosen by the
caller.

Since an I/O may get remapped many times as block devices are stacked
(DM, MD, stripe splits, partition offset shifts, etc.) the seed is not
expected to match the LBA on the storage device. That is almost never
the case.

In SCSI we do a remapping pass before submitting a WRITE or upon
completion of a READ. You will have to do the same for NVMe.

It was done this way to avoid remapping the ref tag several times. We
adjust the seed as the I/O gets sliced and diced and only map the PI
pages once to do a single traversal at the bottom of the stack.

For next gen devices we simply pass the seed to the hardware and let it
handle the remapping. This saves us having to map the PI pages and pull
them through the cache.

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH] block: pass correct seed to integrity metadata generation function

[Sam Bradshaw (sbradshaw)]

> -----Original Message-----
> From: Martin K. Petersen [mailto:martin.petersen@oracle.com]
> Sent: Wednesday, January 07, 2015 4:19 PM
> To: Sam Bradshaw (sbradshaw)
> Cc: Martin K. Petersen; axboe@kernel.dk; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] block: pass correct seed to integrity metadata
> generation function
> 
> >>>>> "Sam" == Sam Bradshaw (sbradshaw) <sbradshaw@micron.com> writes:
> 
> Sam> Yes.
> 
> The seed is just a seed. We happen to set it to the (block layer)
> sector number if nothing else is provided but it's essentially just an
> incrementing counter starting at an arbitrary value chosen by the
> caller.
> 
> Since an I/O may get remapped many times as block devices are stacked
> (DM, MD, stripe splits, partition offset shifts, etc.) the seed is not
> expected to match the LBA on the storage device. That is almost never
> the case.
> 
> In SCSI we do a remapping pass before submitting a WRITE or upon
> completion of a READ. You will have to do the same for NVMe.
> 
> It was done this way to avoid remapping the ref tag several times. We
> adjust the seed as the I/O gets sliced and diced and only map the PI
> pages once to do a single traversal at the bottom of the stack.
> 
> For next gen devices we simply pass the seed to the hardware and let it
> handle the remapping. This saves us having to map the PI pages and pull
> them through the cache.

Got it, thanks!