On Wed, Jun 27, 2018 at 10:44:27AM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>
> DMA mapping is time consuming operation and doesn't need to be
> performed with mmap_sem semaphore is held on. Limit the scope of
> that semaphore for accounting part only.
>
> Signed-off-by: Huy Nguyen <huyn@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> drivers/infiniband/core/umem.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 498f59bb4989..626f3f3b8f44 100644
> +++ b/drivers/infiniband/core/umem.c
> @@ -84,7 +84,6 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> struct ib_umem *umem;
> struct page **page_list;
> struct vm_area_struct **vma_list;
> - unsigned long locked;
> unsigned long lock_limit;
> unsigned long cur_base;
> unsigned long npages;
> @@ -149,15 +148,16 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
>
> npages = ib_umem_num_pages(umem);
>
> - down_write(¤t->mm->mmap_sem);
> -
> - locked = npages + current->mm->pinned_vm;
> lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>
> - if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) {
> + down_write(¤t->mm->mmap_sem);
> + current->mm->pinned_vm += npages;
> + if ((current->mm->pinned_vm > lock_limit) && !capable(CAP_IPC_LOCK)) {
> + up_write(¤t->mm->mmap_sem);
> ret = -ENOMEM;
> goto out;
> }
> + up_write(¤t->mm->mmap_sem);
This change removes the lock around get_user_pages_longterm(), and we
have this calltree:
get_user_pages_longterm()
get_user_pages()
__get_user_pages_locked()
__get_user_pages()
And the comment for __get_user_pages() says:
* Must be called with mmap_sem held. It may be released. See below.
[..]
* A caller using such a combination of @nonblocking and @gup_flags
* must therefore hold the mmap_sem for reading only, and recognize
* when it's been released. Otherwise, it must be held for either
* reading or writing and will not be released.
Though, confusingly, I checked a random selection of get_user_pages()
callers and couldn't obviously find a current->mm->mmap_sem in a few
cases. I think those are bugs. Will try to find out..
If this is safe without a lock for some tricky reason then please
respin with a comment explaining why
Otherwise respin to hold the lock across get_user_pages_longterm, as
it was before..
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
