lkml.org 
[lkml]   [2014]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] ib_umem_release should decrement mm->pinned_vm from ib_umem_get
On Thu, Aug 28, 2014 at 02:48:19PM +0300, Haggai Eran wrote:
> On 26/08/2014 00:07, Shawn Bohrer wrote:
> >>>> The following patch fixes the issue by storing the mm_struct of the
> >> >
> >> > You are doing more than just storing the mm_struct - you are taking
> >> > a reference to the process' mm. This can lead to a massive resource
> >> > leakage. The reason is bit complex: The destruction flow for IB
> >> > uverbs is based upon releasing the file handle for it. Once the file
> >> > handle is released, all MRs, QPs, CQs, PDs, etc. that the process
> >> > allocated are released. For the kernel to release the file handle,
> >> > the kernel reference count to it needs to reach zero. Most IB
> >> > implementations expose some hardware registers to the application by
> >> > allowing it to mmap the uverbs device file. This mmap takes a
> >> > reference to uverbs device file handle that the application opened.
> >> > This reference is dropped when the process mm is released during the
> >> > process destruction. Your code takes a reference to the mm that
> >> > will only be released when the parent MR/QP is released.
> >> >
> >> > Now, we have a deadlock - the mm is waiting for the MR to be
> >> > destroyed, the MR is waiting for the file handle to be destroyed,
> >> > and the file handle is waiting for the mm to be destroyed.
> >> >
> >> > The proper solution is to keep a reference to the task_pid (using
> >> > get_task_pid), and use this pid to get the task_struct and from it
> >> > the mm_struct during the destruction flow.
> >
> > I'll put together a patch using get_task_pid() and see if I can
> > test/reproduce the issue. This may take a couple of days since we
> > have to test this in production at the moment.
> >
>
> Hi,
>
> I just wanted to point out that while working on the on demand paging patches
> we also needed to keep a reference to the task pid (to make sure we always
> handle page faults on behalf of the correct mm struct). You can find the
> relevant code in the patch titled "IB/core: Add support for on demand paging
> regions" [1].
>
> Haggai
>
> [1] https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg20552.html

Haggai,

I looked over the on demand paging patch and I'm not sure if you are
suggesting that it already fixes my issue, or that I should use it as
a reference for my code. In any case I just sent a v2 of a patch that
appears to fix my issue.

--
Shawn


\
 
 \ /
  Last update: 2014-08-29 00:21    [W:0.042 / U:0.652 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site