block: pass correct seed to integrity metadata generation function
PATCH 1/1 block: pass correct seed to integrity metadata generation function
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.
---
block/bio-integrity.c +8 -3
@@ -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; }
Quoted 5 lines · 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.
Any particular objection to this patch? It would be nice if the kernel supported PI on block sizes other than just 512 bytes.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sam" == Sam Bradshaw <[email protected]> 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?
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Quoted 6 lines
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
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sam" == Sam Bradshaw (sbradshaw) <[email protected]> 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?
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Quoted 10 lines
-----Original Message-----
From: Martin K. Petersen [mailto:[email protected]]
Sent: Wednesday, January 07, 2015 3:43 PM
To: Sam Bradshaw (sbradshaw)
Cc: Martin K. Petersen; [email protected]; [email protected]
Subject: Re: [PATCH] block: pass correct seed to integrity metadata generation function
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sam" == Sam Bradshaw (sbradshaw) <[email protected]> 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.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sam" == Sam Bradshaw (sbradshaw) <[email protected]> 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.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Quoted 27 lines
-----Original Message-----
From: Martin K. Petersen [mailto:[email protected]]
Sent: Wednesday, January 07, 2015 4:19 PM
To: Sam Bradshaw (sbradshaw)
Cc: Martin K. Petersen; [email protected]; [email protected]
Subject: Re: [PATCH] block: pass correct seed to integrity metadata generation functionQuoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sam" == Sam Bradshaw (sbradshaw) <[email protected]> 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.
Quoted 7 lines
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!