Re: [PATCH v2 2/5] IB/core, RDMA RW API: Do not exceed QP SGE send limit

[Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>]

On 13/07/16 13:18, Bart Van Assche wrote:
> On 07/13/2016 04:32 PM, Sagi Grimberg wrote:
>>
>>> -static inline u32 rdma_rw_max_sge(struct ib_device *dev,
>>> -        enum dma_data_direction dir)
>>> -{
>>> -    return dir == DMA_TO_DEVICE ?
>>> -        dev->attrs.max_sge : dev->attrs.max_sge_rd;
>>> -}
>>> -
>>
>> Wait, this looks wrong...
>>
>> iWARP devices have max_sge_rd = 1, Steve, did you get to test this?
>>
>>>   static inline u32 rdma_rw_fr_page_list_len(struct ib_device *dev)
>>>   {
>>>       /* arbitrary limit to avoid allocating gigantic resources */
>>> @@ -186,7 +179,7 @@ static int rdma_rw_init_map_wrs(struct
>>> rdma_rw_ctx *ctx, struct ib_qp *qp,
>>>           u64 remote_addr, u32 rkey, enum dma_data_direction dir)
>>>   {
>>>       struct ib_device *dev = qp->pd->device;
>>> -    u32 max_sge = rdma_rw_max_sge(dev, dir);
>>> +    u32 max_sge = qp->max_send_sge;
>>
>> Here, rdma_read will use qp.max_send_sge = max_sge which is set by the
>> ULP.
>>
>> If the ULP used max(max_sge, max_sge_rd) which is the right thing, then
>> we get a bug, if the ULP set min(max_sge, max_sge_rd) then it has
>> effectively a single sge even for writes (which is not good).
>>
>> The QP user needs to set the maximum sge allowed for reads or writes,
>> but for reads use max_sge_rd and for writes use max_sge. Otherwise this
>> will break.
>
> Hello Sagi,
>
> How about keeping rdma_rw_max_sge() and using the minimum of
> qp->max_send_sge and dev->attrs.max_sge_rd for RDMA READs?

That would work.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html