Re: [PATCH v3 05/15] xprtrdma: Remove last ib_reg_phys_mr() call site

[Tom Talpey <tom@talpey.com>]

On 7/20/2015 3:21 PM, Chuck Lever wrote:
>
> On Jul 20, 2015, at 5:55 PM, Tom Talpey <tom@talpey.com> wrote:
>
>> On 7/20/2015 1:55 PM, Chuck Lever wrote:
>>> On Jul 20, 2015, at 4:34 PM, Tom Talpey <tom@talpey.com> wrote:
>>>
>>>> On 7/20/2015 12:03 PM, Chuck Lever wrote:
>>>>> All HCA providers have an ib_get_dma_mr() verb. Thus
>>>>> rpcrdma_ia_open() will either grab the device's local_dma_key if one
>>>>> is available, or it will call ib_get_dma_mr() which is a 100%
>>>>> guaranteed fallback.
>>>>
>>>> I recall that in the past, some providers did not support mapping
>>>> all of the machine's potential physical memory with a single dma_mr.
>>>> If an rnic did/does not support 44-ish bits of length per region,
>>>> for example.
>>>
>>> The buffers affected by this change are small, so I’m confident that
>>> restriction would not be a problem here.
>>
>> It's not about the buffer size, it's about the region. Because the
>> get_dma_mr does not specify a base address and length, the rnic must
>> basically attempt to map a base of zero and a length of the largest
>> physical offset.
>
> Understood now, but:
>
>
>> This is not the case with the previous phys_reg_mr, which specified
>> the exact phys page range.
>
> rpcrdma_ia_open() fails immediately if IB_DEVICE_LOCAL_DMA_LKEY
> is not asserted _and_ ib_get_dma_mr() fails. We never get to the
> logic in rpcrdma_regbuf_alloc() in that case, so ib_reg_phys_mr()
> would still never be invoked. That really is dead code.
>
> If you prefer, I can adjust the patch description to remove the
> “100% guaranteed fallback” line.


Ok, good and yes 100% sounds like a risky statement.