Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

Chuck Lever
2015-07-09T13:52:59Z

On Jul 9, 2015, at 4:46 AM, Sagi Grimberg <sagig@dev.mellanox.co.il> wrote:

> On 7/8/2015 8:14 PM, Hefty, Sean wrote:
>>> I am still not clear if all of us agree that we need it.
>>> Sean and Steve had some disclaimers...
>>
>> A single entry point doesn't help a whole lot if the app must deal with different behavior based on how the API is used.
>
> It is true that different MRs will be used differently. However, not
> once we have found ourselves extending an API to add functionality. This
> means changing the API signature and changing all the call sites. Just
> recently we saw this in Steve's mr_roles and in Matan's timestamping
> support (changed ib_create_cq). When was the last time ib_create_qp API
> was modified?
>
>> We have a single entry point for post_send, for example, and that makes things worse.
>
> I don't necessarily agree. the fact that post_send have multiple entry
> points allows the consumer to concatenate a couple of those in a single
> post. That's beneficial to get maximum performance from your HW.
>
>> IMO, we need fewer registration *methods* not fewer calls.
>
> I do agree that we need fewer registration methods.

I also feel that would be a healthy direction.

> Let's review what we have today:
>
> - FRWR: which is *the* standard way to register memory. It's fast,
> non-blocking and has vast support.
>
> - FMR: which is only supported in some Mellanox devices if I'm not
> mistaken, it's fast, but has slow invalidation (unmap). It is also not
> supported over VF.
> * FMR_POOL API was designed to address the slow unmap but created a
> week invalidation semantics (unmap is done only when some number of
> remapping is met).
>
> - REG_PHYS_MR: which is supported by some devices. It actually
> combines both MR allocation and registration in a single call (it is
> the equivalent of user-space ibv_reg_mr)

There is also Memory Windows, but there may no longer be any
kernel consumers of that memory registration method.

> I don't consider the dma_mr a registration method. It provides physical
> memory access.
>
> As for REG_PHYS_MR, this is the only synchronous registration method in
> the kernel. It is a simple interface, which is currently used by xprtrdma when dma mr is not supported. We can consider removing it in
> the future if it turns out to be non useful.

Code review has shown the remaining ib_reg_phys_mr() call in xprtrdma
is never reached in the current code, and thus it will be removed
very soon.

There is one remaining kernel user of ib_reg_phys_mr() in 4.2: Lustre.

> As for FMR/FMR_POOL, I'd much rather to wait until it becomes
> deprecated on it's own rather than try and incorporate it in a
> modernized API.

> I think the stack can converge on FRWR as its primary registration
> method. Let's focus on making it better.

--
Chuck Lever