Re: [PATCH V2 3/5] RDMA/core: transport-independent access flags

Steve Wise
2015-07-02T13:17:20Z

On 7/2/2015 1:22 AM, Sagi Grimberg wrote:
> On 6/30/2015 8:10 PM, Hefty, Sean wrote:
>>> I suggest to start consolidating to ib_create_mr() that receives an
>>> extensible ib_mr_init_attr and additional attributes can be mr_roles
>>> and mr_attrs.
>>
>> I think this makes sense, but does it really help?
>> If the end result is that the app and providers basically
>> end up switching on mr_attr::type, we end up reducing the
>> number of APIs, but the code complexity remains the same.
>>
>
> I think it will reduce code complexity. First, the callers
> will have a single API to allocate a memory key. I'm not sure why you
> say that app needs to switch on mr::type, it knows which type it wants.
>

I dont see how doing this is less complex:

attr = FASTREG
mr = create_mr(attr)

vs:

mr = lloc_fast_reg_mr()

> As for drivers, From what I've seen, usually there is a single memory
> key allocation interface to the HW and for each API and drivers call it
> with a slightly different attributes and maybe do some extra driver
> specific logic.
>
> Yes, probably drivers will need to switch on mr_attr::type, but maybe
> just just to determine the correct HW interface and hopefully not to
> a completely different logic. I think that drivers can benefit in
> reduced code duplication in total.
>

There isn't much code duplication now because as you mentioned above,
most drivers have common services that get called from the various entry
points: alloc_fast_reg_mr(), get_dma_mr(), reg_user_mr(), etc...

> As a first step, we can do a naive switch on mr_attr::type in the
> drivers but I'd expect driver developers to change this logic in their
> driver.
>
> Thoughts?

I don't see any reduction in complexity of the application nor
driver/core code. Perhaps a reduction in API complexity by having a
single entry point...but that entry point would be the place I would fan
out and call the driver functions vs changing the core<->driver interface...

My 2 centimes...