On Thu, Aug 22, 2019 at 11:41:03AM +0300, Yuval Shaia wrote:
> On Wed, Aug 21, 2019 at 04:37:37PM -0700, Ira Weiny wrote:
> > On Wed, Aug 21, 2019 at 05:21:01PM +0300, Yuval Shaia wrote:
> > > Following patch-set introduce the shared object feature.
> > >
> > > A shared object feature allows one process to create HW objects (currently
> > > PD and MR) so that a second process can import.
>
> Hi Ira,
>
> >
> > For something this fundamental I think the cover letter should be more
> > detailed than this. Questions I have without digging into the code:
> >
> > What is the use case?
>
> I have only one use case but i didn't added it to commit log just not to
> limit the usage of this feature but you are right, cover letter is great
> for such things, will add it for v2.
>
> Anyway, here is our use case: Consider a case of server with huge amount
> of memory and some hundreds or even thousands processes are using it to
> serves clients requests. In this case the HCA will have to manage hundreds
> or thousands MRs. A better design maybe would be that one process will
> create one (or several) MR(s) which will be shared with the other
> processes. This will reduce the number of address translation entries and
> cache miss dramatically.
I think Doug covered my concerns in this area.
>
> >
> > What is the "key" that allows a MR to be shared among 2 processes? Do you
> > introduce some PD identifier? And then some {PDID, lkey} tuple is used to ID
> > the MR?
> >
> > I assume you have to share the PD first and then any MR in the shared PD can be
> > shared? If so how does the MR get shared?
>
> Sorry, i'm not following.
> I think the term 'share' is somehow mistake, it is actually a process
> 'imports' objects into it's context. And yes, the workflow is first to
> import the PD and then import the MR.
Ok fair enough but the title of the thread is "Sharing PD and MR" so I used the
term Share. And I expect that any random process can't import objects to which
the owning process does not allow them to right?
I mean you can't just have any process grab a PD and MR and start using it. So
I assume there is some "sharing" by the originating process.
>
> >
> > Again I'm concerned with how this will interact with the RDMA and file system
> > interaction we have been trying to fix.
>
> I'm not aware of this file-system thing, can you point me to some
> discussion on that so i'll see how this patch-set affect it.
https://lkml.org/lkml/2019/6/6/1101
https://lkml.org/lkml/2019/8/9/1043
https://lwn.net/Articles/796000/
There are many more articles, patch sets, discussion threads... This work has
been going on much longer than I have been working on it.
>
> >
> > Why is SCM_RIGHTS on the rdma context FD not sufficient to share the entire
> > context, PD, and all MR's?
>
> Well, this SCM_RIGHTS is great, one can share the IB context with another.
> But it is not enough, because:
> - What API the second process can use to get his hands on one of the PDs or
> MRs from this context?
MRs can be passed by {PD,key} through any number of mechanisms. All you need
is an ID for them. Maybe this is clear in the code. If so sorry about that.
> - What mechanism takes care of the destruction of such objects (SCM_RIGHTS
> takes care for the ref counting of the context but i'm referring to the
> PDs and MRs objects)?
This is inherent in the lifetime of the uverbs file object to which cloned FDs
(one in each process) have a reference to.
Add to your list "how does destruction of a MR in 1 process get communicated to
the other?" Does the 2nd process just get failed WR's?
>
> The entire purpose of this patch set is to address these two questions.
Fair enough but the cover letter should spell out the above and how this series
fixes that problem.
I have some of the same concerns as Doug WRT memory sharing. FWIW I'm not sure
that what SCM_RIGHTS is doing is safe or correct.
For that work I'm really starting to think SCM_RIGHTS transfers should be
blocked. It just seems wrong that Process B gets references to Process A's
mm_struct and holds the memory Process A allocated. This seems wrong for any
type of memory, file system or not. That said I'm assuming that this is all
within a single user so admins can at least determine who is pinning down all
this memory.
Ira
