On 7/24/2018 11:33 PM, Keith Busch wrote:
On Tue, Jul 24, 2018 at 04:33:41PM +0300, Max Gurtovoy wrote:+void t10_pi_prepare(struct request *rq, u8 protection_type) +{ + const int tuple_sz = rq->q->integrity.tuple_size; + u32 ref_tag = t10_pi_ref_tag(rq); + struct bio *bio; + + if (protection_type == T10_PI_TYPE3_PROTECTION) + return; + + __rq_for_each_bio(bio, rq) { + struct bio_integrity_payload *bip = bio_integrity(bio); + u32 virt = bip_get_seed(bip) & 0xffffffff; + struct bio_vec iv; + struct bvec_iter iter; + + /* Already remapped? */ + if (bip->bip_flags & BIP_MAPPED_INTEGRITY) + break; + + bip_for_each_vec(iv, bip, iter) { + struct t10_pi_tuple *pi = kmap_atomic(iv.bv_page) + + iv.bv_offset; + unsigned int j; + + for (j = 0; j < iv.bv_len; j += tuple_sz) { + if (be32_to_cpu(pi->ref_tag) == virt) + pi->ref_tag = cpu_to_be32(ref_tag); + virt++; + ref_tag++; + pi += tuple_sz; + } + + kunmap_atomic(pi); + }Since you're incrementing 'pi', you end up unmapping an address that you didn't map. It does appears harmless in current kunmap_atomic() implementation, though.
ohh, good catch. This code is taken from the scsi remap, but I guess it's time to fix some stuff while we doing this patchset.
I'll use another variable pi_map that will be unmapped. in the nvme code: pmap = kmap_atomic(bip->bip_vec->bv_page) + bip->bip_vec->bv_offset; ... but kunmap_atomic(pmap); should we unmap the same bv_page we mapped (without the offset) ?
You are also incrementing 'pi' by too many bytes since it is of type struct t10_pi_tuple. The nvme driver used void* to make the pointer arithmentic easier.
I'' use void*. Thanks
