Re: [PATCH v4 7/9] IB/core: generic RDMA READ/WRITE API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 08, 2016 at 01:32:44PM -0800, Bart Van Assche wrote:
> The above pr_info() message does not provide enough context information to 
> allow a user to figure out why that message was reported. Please leave that 
> message out and make the callers of this function report MR allocation 
> failure wherever useful.

Ok, I'll remove it.

>> +		ret = ib_map_mr_sg(reg->mr, sg, nents, offset,
>> +				PAGE_SIZE);
> [ ... ]
>> +		sg = sg_next(sg);
>
> ib_map_mr_sg() processed 'ret' elements of scatterlist 'sg'. So why is 'sg' 
> only advanced by one element at the end of the loop?

I think the multiple MR case here is simply broken, as we never hit
for the iSER or NVMe over Fabrics I/O sizes.  I think the safest
is to simply not allow for it.  I'll update the patch to disable the
loop, and add a helper for the driver to query the max I/O size
supported by the device.

>> +		for (j = 0; j < nr_sge; j++, sg = sg_next(sg)) {
>> +			rdma_wr->wr.num_sge++;
>> +
>> +			sge->addr = ib_sg_dma_address(dev, sg) + offset;
>> +			sge->length = ib_sg_dma_len(dev, sg) - offset;
>> +			sge->lkey = qp->pd->local_dma_lkey;
>> +
>> +			total_len += sge->length;
>> +			sge++;
>> +			sge_left--;
>> +			offset = 0;
>> +		}
>
> Shouldn't there be a break statement in the above loop that stops this loop 
> if the end of the sg list has been reached? Less than nr_sge iterations 
> will be needed to reach the end of the sg list if the length of the sg list 
> is not a multiple of nr_sge.

nr_sge is calculated as:

	u32 nr_sge = min(sge_left, max_sge);

so it will never contain more iteration than we can possibly do.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux