From: Jan Kara <jack@suse.cz>
To: "Marciniszyn, Mike" <mike.marciniszyn@intel.com>
Cc: Jan Kara <jack@suse.cz>, LKML <linux-kernel@vger.kernel.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
infinipath <infinipath@intel.com>,
Roland Dreier <roland@kernel.org>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: Re: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()
Date: Mon, 7 Oct 2013 19:26:04 +0200 [thread overview]
Message-ID: <20131007172604.GD30441@quack.suse.cz> (raw)
In-Reply-To: <32E1700B9017364D9B60AED9960492BC211B07B7@FMSMSX107.amr.corp.intel.com>
On Mon 07-10-13 15:38:24, Marciniszyn, Mike wrote:
> > > This patch and the sibling ipath patch will nominally take the mmap_sem
> > > twice where the old routine only took it once. This is a performance
> > > issue.
> > It will take mmap_sem only once during normal operation. Only if
> > get_user_pages_unlocked() fail, we have to take mmap_sem again to undo
> > the change of mm->pinned_vm.
> >
> > > Is the intent here to deprecate get_user_pages()?
>
> The old code looked like:
> __qib_get_user_pages()
> (broken) ulimit test
> for (...)
> get_user_pages()
>
> qib_get_user_pages()
> mmap_sem lock
> __qib_get_user_pages()
> mmap_sem() unlock
>
> The new code is:
>
> get_user_pages_unlocked()
> mmap_sem lock
> get_user_pages()
> mmap_sem unlock
>
> qib_get_user_pages()
> mmap_sem lock
> ulimit test and locked pages maintenance
> mmap_sem unlock
> for (...)
> get_user_pages_unlocked()
>
> I count an additional pair of mmap_sem transactions in the normal case.
Ah, sorry, you are right.
> > > Could the lock limit test be pushed into another version of the
> > > wrapper so that there is only one set of mmap_sem transactions?
> > I'm sorry, I don't understand what you mean here...
> >
>
> This is what I had in mind:
>
> get_user_pages_ulimit_unlocked()
> mmap_sem lock
> ulimit test and locked pages maintenance (from qib/ipath)
> for (...)
> get_user_pages_unlocked()
> mmap_sem unlock
>
> qib_get_user_pages()
> get_user_pages_ulimit_unlocked()
>
> This really pushes the code into a new wrapper common to ipath/qib and
> any others that might want to combine locking with ulimit enforcement.
We could do that but frankly, I'd rather change ulimit enforcement to not
require mmap_sem and use atomic counter instead. I'll see what I can do.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: "Marciniszyn, Mike" <mike.marciniszyn@intel.com>
Cc: Jan Kara <jack@suse.cz>, LKML <linux-kernel@vger.kernel.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
infinipath <infinipath@intel.com>,
Roland Dreier <roland@kernel.org>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: Re: [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked()
Date: Mon, 7 Oct 2013 19:26:04 +0200 [thread overview]
Message-ID: <20131007172604.GD30441@quack.suse.cz> (raw)
In-Reply-To: <32E1700B9017364D9B60AED9960492BC211B07B7@FMSMSX107.amr.corp.intel.com>
On Mon 07-10-13 15:38:24, Marciniszyn, Mike wrote:
> > > This patch and the sibling ipath patch will nominally take the mmap_sem
> > > twice where the old routine only took it once. This is a performance
> > > issue.
> > It will take mmap_sem only once during normal operation. Only if
> > get_user_pages_unlocked() fail, we have to take mmap_sem again to undo
> > the change of mm->pinned_vm.
> >
> > > Is the intent here to deprecate get_user_pages()?
>
> The old code looked like:
> __qib_get_user_pages()
> (broken) ulimit test
> for (...)
> get_user_pages()
>
> qib_get_user_pages()
> mmap_sem lock
> __qib_get_user_pages()
> mmap_sem() unlock
>
> The new code is:
>
> get_user_pages_unlocked()
> mmap_sem lock
> get_user_pages()
> mmap_sem unlock
>
> qib_get_user_pages()
> mmap_sem lock
> ulimit test and locked pages maintenance
> mmap_sem unlock
> for (...)
> get_user_pages_unlocked()
>
> I count an additional pair of mmap_sem transactions in the normal case.
Ah, sorry, you are right.
> > > Could the lock limit test be pushed into another version of the
> > > wrapper so that there is only one set of mmap_sem transactions?
> > I'm sorry, I don't understand what you mean here...
> >
>
> This is what I had in mind:
>
> get_user_pages_ulimit_unlocked()
> mmap_sem lock
> ulimit test and locked pages maintenance (from qib/ipath)
> for (...)
> get_user_pages_unlocked()
> mmap_sem unlock
>
> qib_get_user_pages()
> get_user_pages_ulimit_unlocked()
>
> This really pushes the code into a new wrapper common to ipath/qib and
> any others that might want to combine locking with ulimit enforcement.
We could do that but frankly, I'd rather change ulimit enforcement to not
require mmap_sem and use atomic counter instead. I'll see what I can do.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
next prev parent reply other threads:[~2013-10-07 17:26 UTC|newest]
Thread overview: 142+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-02 14:27 [PATCH 0/26] get_user_pages() cleanup Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 01/26] cris: Convert cryptocop to use get_user_pages_fast() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 02/26] ia64: Use get_user_pages_fast() in err_inject.c Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 03/26] dma: Use get_user_pages_fast() in dma_pin_iovec_pages() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 04/26] drm: Convert via driver to use get_user_pages_fast() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 05/26] omap3isp: Make isp_video_buffer_prepare_user() " Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 19:41 ` Laurent Pinchart
2013-10-02 19:41 ` Laurent Pinchart
2013-10-02 20:18 ` Jan Kara
2013-10-02 20:18 ` Jan Kara
2013-10-02 14:27 ` [PATCH 06/26] vmw_vmci: Convert driver to " Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 07/26] st: Convert sgl_map_user_pages() " Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 08/26] ced1401: Convert driver " Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 09/26] crystalhd: Convert crystalhd_map_dio() " Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 10/26] lustre: Convert ll_get_user_pages() " Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-05 6:27 ` Dilger, Andreas
2013-10-05 6:27 ` Dilger, Andreas
2013-10-02 14:27 ` [PATCH 11/26] sep: Convert sep_lock_user_pages() to get_user_pages_fast() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 12/26] pvr2fb: Convert pvr2fb_write() to use get_user_pages_fast() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 13/26] fsl_hypervisor: Convert ioctl_memcpy() " Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-04 2:38 ` Timur Tabi
2013-10-04 2:38 ` Timur Tabi
2013-10-02 14:27 ` [PATCH 14/26] nfs: Convert direct IO " Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 15/26] ceph: Convert ceph_get_direct_page_vector() to get_user_pages_fast() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:27 ` [PATCH 16/26] mm: Provide get_user_pages_unlocked() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 16:25 ` Christoph Hellwig
2013-10-02 16:25 ` Christoph Hellwig
2013-10-02 16:28 ` KOSAKI Motohiro
2013-10-02 16:28 ` KOSAKI Motohiro
2013-10-02 19:39 ` Jan Kara
2013-10-02 19:39 ` Jan Kara
2013-10-02 14:27 ` [PATCH 17/26] kvm: Use get_user_pages_unlocked() in async_pf_execute() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 14:59 ` Gleb Natapov
2013-10-02 14:59 ` Gleb Natapov
2013-10-02 14:27 ` [PATCH 18/26] mm: Convert process_vm_rw_pages() to use get_user_pages_unlocked() Jan Kara
2013-10-02 14:27 ` Jan Kara
2013-10-02 16:32 ` KOSAKI Motohiro
2013-10-02 16:32 ` KOSAKI Motohiro
2013-10-02 19:36 ` Jan Kara
2013-10-02 19:36 ` Jan Kara
2013-10-03 22:40 ` KOSAKI Motohiro
2013-10-03 22:40 ` KOSAKI Motohiro
2013-10-07 20:55 ` Jan Kara
2013-10-07 20:55 ` Jan Kara
2013-10-08 0:10 ` KOSAKI Motohiro
2013-10-08 0:10 ` KOSAKI Motohiro
2013-10-02 14:28 ` [PATCH 19/26] ivtv: Convert driver " Jan Kara
2013-10-02 14:28 ` Jan Kara
2013-10-05 12:02 ` Andy Walls
2013-10-05 12:02 ` Andy Walls
2013-10-07 17:22 ` Jan Kara
2013-10-07 17:22 ` Jan Kara
2013-10-02 14:28 ` [PATCH 20/26] ib: Convert ib_umem_get() to get_user_pages_unlocked() Jan Kara
2013-10-02 14:28 ` Jan Kara
2013-10-02 14:28 ` [PATCH 21/26] ib: Convert ipath_get_user_pages() " Jan Kara
2013-10-02 14:28 ` Jan Kara
2013-10-02 14:28 ` [PATCH 22/26] ib: Convert ipath_user_sdma_pin_pages() to use get_user_pages_unlocked() Jan Kara
2013-10-02 14:28 ` Jan Kara
2013-10-02 14:28 ` [PATCH 23/26] ib: Convert qib_get_user_pages() to get_user_pages_unlocked() Jan Kara
2013-10-02 14:28 ` Jan Kara
[not found] ` <1380724087-13927-24-git-send-email-jack-AlSwsSmVLrQ@public.gmane.org>
2013-10-02 14:54 ` Marciniszyn, Mike
2013-10-02 14:54 ` Marciniszyn, Mike
2013-10-02 14:54 ` Marciniszyn, Mike
[not found] ` <32E1700B9017364D9B60AED9960492BC211AEF75-AtyAts71sc9zLByeVOV5+bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2013-10-02 15:28 ` Jan Kara
2013-10-02 15:28 ` Jan Kara
2013-10-02 15:28 ` Jan Kara
[not found] ` <20131002152811.GC32181-+0h/O2h83AeN3ZZ/Hiejyg@public.gmane.org>
2013-10-02 15:32 ` Marciniszyn, Mike
2013-10-02 15:32 ` Marciniszyn, Mike
2013-10-02 15:32 ` Marciniszyn, Mike
[not found] ` <32E1700B9017364D9B60AED9960492BC211AF005-AtyAts71sc9zLByeVOV5+bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2013-10-02 15:38 ` Jan Kara
2013-10-02 15:38 ` Jan Kara
2013-10-02 15:38 ` Jan Kara
[not found] ` <20131002153842.GD32181-+0h/O2h83AeN3ZZ/Hiejyg@public.gmane.org>
2013-10-04 13:39 ` Marciniszyn, Mike
2013-10-04 13:39 ` Marciniszyn, Mike
2013-10-04 13:39 ` Marciniszyn, Mike
[not found] ` <32E1700B9017364D9B60AED9960492BC211B0123-AtyAts71sc9zLByeVOV5+bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2013-10-04 13:46 ` Marciniszyn, Mike
2013-10-04 13:46 ` Marciniszyn, Mike
2013-10-04 13:46 ` Marciniszyn, Mike
2013-10-04 13:44 ` Marciniszyn, Mike
2013-10-04 13:44 ` Marciniszyn, Mike
2013-10-04 13:52 ` Marciniszyn, Mike
2013-10-04 13:52 ` Marciniszyn, Mike
2013-10-04 13:52 ` Marciniszyn, Mike
[not found] ` <32E1700B9017364D9B60AED9960492BC211B0176-AtyAts71sc9zLByeVOV5+bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2013-10-04 18:33 ` Jan Kara
2013-10-04 18:33 ` Jan Kara
2013-10-04 18:33 ` Jan Kara
[not found] ` <20131004183315.GA19557-+0h/O2h83AeN3ZZ/Hiejyg@public.gmane.org>
2013-10-07 15:20 ` Marciniszyn, Mike
2013-10-07 15:20 ` Marciniszyn, Mike
2013-10-07 15:20 ` Marciniszyn, Mike
2013-10-07 15:38 ` Marciniszyn, Mike
2013-10-07 15:38 ` Marciniszyn, Mike
2013-10-07 17:26 ` Jan Kara [this message]
2013-10-07 17:26 ` Jan Kara
2013-10-08 19:06 ` Jan Kara
2013-10-08 19:06 ` Jan Kara
[not found] ` <20131008190604.GB14223-+0h/O2h83AeN3ZZ/Hiejyg@public.gmane.org>
2013-10-16 21:39 ` Jan Kara
2013-10-16 21:39 ` Jan Kara
2013-10-16 21:39 ` Jan Kara
2013-10-02 14:28 ` [PATCH 24/26] ib: Convert qib_user_sdma_pin_pages() to use get_user_pages_unlocked() Jan Kara
2013-10-02 14:28 ` Jan Kara
2013-10-02 14:28 ` [PATCH 25/26] ib: Convert mthca_map_user_db() to use get_user_pages_fast() Jan Kara
2013-10-02 14:28 ` Jan Kara
2013-10-02 14:28 ` [PATCH 26/26] aio: Remove useless get_user_pages() call Jan Kara
2013-10-02 14:28 ` Jan Kara
2013-10-02 14:28 ` Jan Kara
2013-10-02 16:20 ` [PATCH 0/26] get_user_pages() cleanup Christoph Hellwig
2013-10-02 16:20 ` Christoph Hellwig
2013-10-02 20:29 ` Jan Kara
2013-10-02 20:29 ` Jan Kara
2013-10-04 20:31 ` KOSAKI Motohiro
2013-10-04 20:31 ` KOSAKI Motohiro
2013-10-04 20:42 ` KOSAKI Motohiro
2013-10-04 20:42 ` KOSAKI Motohiro
2013-10-07 21:18 ` Jan Kara
2013-10-07 21:18 ` Jan Kara
2013-10-07 21:18 ` Jan Kara
2013-10-08 0:27 ` KOSAKI Motohiro
2013-10-08 0:27 ` KOSAKI Motohiro
2013-10-08 0:27 ` KOSAKI Motohiro
2013-10-08 6:06 ` Jan Kara
2013-10-08 6:06 ` Jan Kara
2013-10-08 6:06 ` Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131007172604.GD30441@quack.suse.cz \
--to=jack@suse.cz \
--cc=infinipath@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mike.marciniszyn@intel.com \
--cc=roland@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.