| Message ID | 1567956405-5585-2-git-send-email-maxg@mellanox.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/3] block: centralize PI remapping logic to the block layer | expand |
Max,
> Only type 1 and type 2 have a reference tag by definition.
DIX Type 0 needs remapping so this assertion is not correct.
On Sun, Sep 08, 2019 at 10:22:50PM -0400, Martin K. Petersen wrote: > > Max, > > > Only type 1 and type 2 have a reference tag by definition. > > DIX Type 0 needs remapping so this assertion is not correct. At least for nvme, type 0 means you have meta data but not for protection information, so remapping the place the where reference tag exists for other PI types corrupts the metadata.
Keith, > At least for nvme, type 0 means you have meta data but not for > protection information, Yeah, NVMe does not support DIX Type 0. > so remapping the place the where reference tag exists for other PI > types corrupts the metadata. But the device shouldn't have an integrity profile in that case (see previous mail about why keying off of the protection_type is a problem).
On 9/9/2019 5:49 AM, Martin K. Petersen wrote: > Keith, > >> At least for nvme, type 0 means you have meta data but not for >> protection information, > Yeah, NVMe does not support DIX Type 0. > >> so remapping the place the where reference tag exists for other PI >> types corrupts the metadata. > But the device shouldn't have an integrity profile in that case (see > previous mail about why keying off of the protection_type is a problem). I see. Ok I'll not handle NVMe type 0 in this patchset. There is no kernel implementation for that so I don't see a good reason doing so after Martin's proposal.
diff --git a/block/t10-pi.c b/block/t10-pi.c index a33eac4..753b5a8 100644 --- a/block/t10-pi.c +++ b/block/t10-pi.c @@ -168,6 +168,12 @@ static blk_status_t t10_pi_type3_verify_ip(struct blk_integrity_iter *iter) }; EXPORT_SYMBOL(t10_pi_type3_ip); +static inline bool blk_integrity_need_remap(struct gendisk *disk) +{ + return disk->protection_type == T10_PI_TYPE1_PROTECTION || + disk->protection_type == T10_PI_TYPE2_PROTECTION; +} + /** * t10_pi_prepare - prepare PI prior submitting request to device * @rq: request with PI that should be prepared @@ -178,7 +184,7 @@ static blk_status_t t10_pi_type3_verify_ip(struct blk_integrity_iter *iter) * likely to be different. Remap protection information to match the * physical LBA. * - * Type 3 does not have a reference tag so no remapping is required. + * Types 0 and 3 don't have a reference tag so no remapping is required. */ void t10_pi_prepare(struct request *rq) { @@ -186,7 +192,7 @@ void t10_pi_prepare(struct request *rq) u32 ref_tag = t10_pi_ref_tag(rq); struct bio *bio; - if (rq->rq_disk->protection_type == T10_PI_TYPE3_PROTECTION) + if (!blk_integrity_need_remap(rq->rq_disk)) return; __rq_for_each_bio(bio, rq) { @@ -234,7 +240,7 @@ void t10_pi_prepare(struct request *rq) * to the device, we should remap it back to virtual values expected by the * block layer. * - * Type 3 does not have a reference tag so no remapping is required. + * Types 0 and 3 don't have a reference tag so no remapping is required. */ void t10_pi_complete(struct request *rq, unsigned int nr_bytes) { @@ -243,7 +249,7 @@ void t10_pi_complete(struct request *rq, unsigned int nr_bytes) u32 ref_tag = t10_pi_ref_tag(rq); struct bio *bio; - if (rq->rq_disk->protection_type == T10_PI_TYPE3_PROTECTION) + if (!blk_integrity_need_remap(rq->rq_disk)) return; __rq_for_each_bio(bio, rq) {
Only type 1 and type 2 have a reference tag by definition. Suggested-by: Keith Busch <kbusch@kernel.org> Signed-off-by: Max Gurtovoy <maxg@mellanox.com> --- changes from v3: - added blk_integrity_need_remap helper --- block/t10-pi.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)