[ofa-general] Re: [PATCH RFC] sharing userspace IB objects

[Michael S. Tsirkin]

> Quoting Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCH RFC] sharing userspace IB objects
> 
> Some initial reaction, in no particular order:
> 
>  - Having to allocate everything in memory that the library mmap()s
>    adds a lot of yucky stuff -- basically we need to implement our own
>    allocator for the shared memory offets.

Right.

>    I guess we could wrap this
>    in libibverbs and only implement it once but still we're basically
>    reimplementing malloc().

Right.

>    Is there really a strong use case for making every type of object
>    shareable?  Can we handle the SRC stuff without going to this
>    extreme of complexity?

This is not directly related to SRC: this is an effort
to make it possible to share QPs, CQ etc across processes
in the same way as they can be currently shared across threads.
So assuming that we want multiple processes to post to
the same QP, how can we support this?

>  - Given that everything shared is in shared memory,

I think we should try and keep shared memory usage to minimum.
For example, in mthca mr object just needs a key: we could
keep it in non-shared memory, just pass the key around
and save on sahred memory usage.

>    it seems we could
>    avoid all the marshalling/unmarshalling stuff, and just have the
>    shared objects have an ID along with an API to look up objects by
>    API.  That way we could let applications use more than just unix
>    sockets -- eg pipe() + fork() would work too.

We need to share file descriptors too. Is there a way to pass these
around besides unix domain sockets?

>  > +struct ibv_context *ibv_open_shared_device(struct ibv_device *device,
>  > +					   int fd, off_t offset);
> 
>  - This seems like too low-level an interface; I don't think there's
>    any way to enforce the fact that fd came from shm_open(), and I
>    don't see the use of offset at all.

Hmm, I accept offset is not too important.  About fd coming from shm_open - we
don't care, if the user wants to use a storage-backed file for this, let him.
And if you consider that case, maybe people want to use e.g.  mkstemp to open
these.  Even for shm_open, if you want a unique name, you'll have to implement
something complicated on top of shm_open.

So maybe add just fd to ibv_open_device, and value -1 would mean non-shared?
OK?

>    I think it would be more
>    sensible to extend the normal ibv_open_device() with a pathname,
>    and maybe a flag about whether to create or map an existing shared
>    context, and do all the shm stuff internally.  Then if someone
>    passes a NULL pathname, the context isn't shareable.

But are you sure we want to break API for all users just to add
a new capability for a minority that wants shared memory support?

-- 
MST