Hello,
In my quest for changing locking around page faults to make things easier for filesystems I found out get_user_pages() users could use a cleanup. The knowledge about necessary locking for get_user_pages() is in tons of places in drivers and quite a few of them actually get it wrong (don't have mmap_sem when calling get_user_pages() or hold mmap_sem when calling copy_from_user() in the surrounding code). Rather often this actually doesn't seem necessary. This patch series converts lots of places to use either get_user_pages_fast() or a new simple wrapper get_user_pages_unlocked() to remove the knowledge of mmap_sem from the drivers. I'm still looking into converting a few remaining drivers (most notably v4l2) which are more complex.
As I already wrote, in some cases I actually think drivers were buggy (and I note that in corresponding changelogs). I would really like to ask respective maintainers to have a look at the patches in their area. Also any other comments are welcome. Thanks.
Honza
PS: Sorry for the huge recipient list but I don't really know how to trim it down...
CC: Alexander Viro viro@zeniv.linux.org.uk CC: Andreas Dilger andreas.dilger@intel.com CC: Andy Walls awalls@md.metrocast.net CC: Arnd Bergmann arnd@arndb.de CC: Benjamin LaHaise bcrl@kvack.org CC: ceph-devel@vger.kernel.org CC: Dan Williams dan.j.williams@intel.com CC: David Airlie airlied@linux.ie CC: dri-devel@lists.freedesktop.org CC: Gleb Natapov gleb@redhat.com CC: Greg Kroah-Hartman gregkh@linuxfoundation.org CC: hpdd-discuss@lists.01.org CC: Jarod Wilson jarod@wilsonet.com CC: Jayant Mangalampalli jayant.mangalampalli@intel.com CC: Jean-Christophe Plagniol-Villard plagnioj@jcrosoft.com CC: Jesper Nilsson jesper.nilsson@axis.com CC: Kai Makisara Kai.Makisara@kolumbus.fi CC: kvm@vger.kernel.org CC: Laurent Pinchart laurent.pinchart@ideasonboard.com CC: linux-aio@kvack.org CC: linux-cris-kernel@axis.com CC: linux-fbdev@vger.kernel.org CC: linux-fsdevel@vger.kernel.org CC: linux-ia64@vger.kernel.org CC: linux-media@vger.kernel.org CC: linux-nfs@vger.kernel.org CC: linux-rdma@vger.kernel.org CC: linux-scsi@vger.kernel.org CC: Manu Abraham abraham.manu@gmail.com CC: Mark Allyn mark.a.allyn@intel.com CC: Mikael Starvik starvik@axis.com CC: Mike Marciniszyn infinipath@intel.com CC: Naren Sankar nsankar@broadcom.com CC: Paolo Bonzini pbonzini@redhat.com CC: Peng Tao tao.peng@emc.com CC: Roland Dreier roland@kernel.org CC: Sage Weil sage@inktank.com CC: Scott Davilla davilla@4pi.com CC: Timur Tabi timur@freescale.com CC: Tomi Valkeinen tomi.valkeinen@ti.com CC: Tony Luck tony.luck@intel.com CC: Trond Myklebust Trond.Myklebust@netapp.com
CC: David Airlie airlied@linux.ie CC: dri-devel@lists.freedesktop.org Signed-off-by: Jan Kara jack@suse.cz --- drivers/gpu/drm/via/via_dmablit.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c index 8b0f25904e6d..7e3766667d78 100644 --- a/drivers/gpu/drm/via/via_dmablit.c +++ b/drivers/gpu/drm/via/via_dmablit.c @@ -238,14 +238,10 @@ via_lock_all_dma_pages(drm_via_sg_info_t *vsg, drm_via_dmablit_t *xfer) vsg->pages = vzalloc(sizeof(struct page *) * vsg->num_pages); if (NULL == vsg->pages) return -ENOMEM; - down_read(¤t->mm->mmap_sem); - ret = get_user_pages(current, current->mm, - (unsigned long)xfer->mem_addr, - vsg->num_pages, - (vsg->direction == DMA_FROM_DEVICE), - 0, vsg->pages, NULL); - - up_read(¤t->mm->mmap_sem); + ret = get_user_pages_fast((unsigned long)xfer->mem_addr, + vsg->num_pages, + (vsg->direction == DMA_FROM_DEVICE), + vsg->pages); if (ret != vsg->num_pages) { if (ret < 0) return ret;
On Wed, Oct 02, 2013 at 04:27:41PM +0200, Jan Kara wrote:
Even looking over the kerneldoc comment next to it I still fail to understand when you'd want to use get_user_pages_fast and when not.
This isn't meant as an argument against your series, but maybe a hint that we'd need further work in this direction.
On Wed 02-10-13 09:20:09, Christoph Hellwig wrote:
AFAIU get_user_pages_fast() should be used 1) if you don't need any special get_user_pages() arguments (like calling it for mm of a different process, forcing COW, or similar). 2) you don't expect pages to be unmapped (then get_user_pages_fast() is actually somewhat slower because it walks page tables twice).
Honza
dri-devel@lists.freedesktop.org
-
Christoph Hellwig -
Jan Kara