Re: [PATCH 13/15] nvmet: Add metadata/T10-PI support

[Max Gurtovoy <maxg@mellanox.com>]

On 1/9/2020 5:24 AM, Martin K. Petersen wrote:
> Max,


Martin,


>
>> -	/* no limit on data transfer sizes for now */
>> -	id->mdts = 0;
>> +	/* Limit MDTS for metadata capable controllers (assume mpsmin is 4k) */
>> +	id->mdts = ctrl->pi_support ? ilog2(NVMET_T10_PI_MAX_KB_SZ >> 2) : 0;
> [...]
>
>> +#define NVMET_T10_PI_MAX_KB_SZ 128
> Where does this limit come from?

This is coming from few reasons:

1. In the current driver implementation of the ib_core (RDMA) we are 
limited to 1MB IO for T10-DIF offload so we need to limit the mdts 
anyway. For that, I want to add a ctrl->ops->get_ctrl_mdts(ctrl).

2. There is some unclear (to me) behavior in the block layer regarding 
splitting integrity bios. We get guard error over the fabric transaction 
in case we need to split a bio (the error is at the target side). I'm 
debugging it and found potential bug that I fixed using:

@@ -378,6 +378,12 @@ void bio_integrity_advance(struct bio *bio, 
unsigned int bytes_done)

         bip->bip_iter.bi_sector += bytes_done >> 9;
         bvec_iter_advance(bip->bip_vec, &bip->bip_iter, bytes);
+
+       if (split && bip->bip_iter.bi_bvec_done) {
+               bip->bip_vec[bip->bip_iter.bi_idx].bv_len -= 
bip->bip_iter.bi_bvec_done;
+               bip->bip_vec[bip->bip_iter.bi_idx].bv_offset += 
bip->bip_iter.bi_bvec_done;
+               bip->bip_iter.bi_bvec_done = 0;
+       }
  }

but unfortunately it helped only if we split the bio once. In case we 
split it more than once, we get to guard check error again.

maybe you have an idea how to overcome this ?


3. The maximum_integrity_segments = 1 for NVMe devices, so we can use 
4096/8=512 integrity sectors before reaching the limit of 
max_integrity_segments. This leads to mdts=6 (256KB) if the bs=512 . I'm 
afraid that we might have issues with the fact that the target prot_sgl 
 > 1 and the NVMe driver set maximum_integrity_segments = 1.


Therefore, I suggest to limit the mdts per transport (and internally 
check if this ctrl is pi_enabled).


All the above is happening when using submit_bio --> ... --> bio_split 
function that call bio_advance as well (that is called from completion 
context as well). This path is not used in local NVMe IO.

In the local NVMe IO (e.g using FIO application), there is no call to 
bio_split/bio_advance in the submission flow, and there is only one call 
to bio_advance in the completion flow.


-Max.


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme