get_user_pages() cleanup
Subsystems (7)
cover get_user_pages() cleanup
From: Jan Kara <[email protected]>
Hello,
Now when the usage of get_user_pages() in media drivers got cleaned up, here comes a series which removes knowledge about mmap_sem from a couple of other drivers. Patches are trivial and standalone but please check, they are only compile tested. If you are OK with them, either take them through your respective trees or ack them and I can take care of pushing them to Linus (probably through mm tree). Thanks.
After these patches there are some 12 call sites of get_user_pages() outside of core code (mostly infiniband and RDMA). So we are slowly getting to the goal of removing knowledge about page fault locking from drivers which will consequently allow us to change the locking rules with reasonable effort.
Honza
PATCH 1/7 cris: Convert cryptocop to use get_user_pages_fast()
From: Jan Kara <[email protected]>
--- arch/cris/arch-v32/drivers/cryptocop.c | 35 ++++++++++------------------------ 1 file changed, 10 insertions(+), 25 deletions(-)
arch/cris/arch-v32/drivers/cryptocop.c +10 -25
@@ -2716,43 +2716,28 @@ static int cryptocop_ioctl_process(struct inode *inode, struct file *filp, unsig } } -/* Acquire the mm page semaphore. */ -down_read (¤t->mm->mmap_sem); - - err =get_user_pages (current, - current->mm, - (unsigned long int )(oper.indata + prev_ix), - noinpages, -0 ,/* read access only for in data */ -0 ,/* no force */ - inpages, -NULL ); + err =get_user_pages_fast ((unsigned long )(oper.indata + prev_ix), + noinpages, +0 ,/* read access only for in data */ + inpages);if (err <0 ) { -up_read (¤t->mm->mmap_sem); nooutpages = noinpages =0 ; -DEBUG_API (printk ("cryptocop_ioctl_process: get_user_pages indata\n" )); +DEBUG_API (printk ("cryptocop_ioctl_process: get_user_pages_fast indata\n" )); goto error_cleanup; } noinpages = err;if (oper.do_cipher){ - err =get_user_pages (current, - current->mm, - (unsigned long int )oper.cipher_outdata, - nooutpages, -1 ,/* write access for out data */ -0 ,/* no force */ - outpages, -NULL ); -up_read (¤t->mm->mmap_sem); + err =get_user_pages_fast ((unsigned long )oper.cipher_outdata, + nooutpages, +1 ,/* write access for out data */ + outpages);if (err <0 ) { nooutpages =0 ; -DEBUG_API (printk ("cryptocop_ioctl_process: get_user_pages outdata\n" )); +DEBUG_API (printk ("cryptocop_ioctl_process: get_user_pages_fast outdata\n" )); goto error_cleanup; } nooutpages = err; - }else { -up_read (¤t->mm->mmap_sem); }/* Add 6 to nooutpages to make room for possibly inserted buffers for storing digest and
Signature · 7 lines
2.1.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 1/7] cris: Convert cryptocop to use get_user_pages_fast()
Thank you! I will do the same change in our out-of-tree modules! Jesper will do the ack.
Quoted 73 lines
6 okt 2015 kl. 11:43 skrev Jan Kara <[email protected]>:
From: Jan Kara <[email protected]>
CC Mikael Starvik <[email protected]>CC Jesper Nilsson <[email protected]>Signed-off-by Jan Kara <[email protected]>---
arch/cris/arch-v32/drivers/cryptocop.c| 35 ++++++++++------------------------1 file changed, 10 insertions(+), 25 deletions(-)arch/cris/arch-v32/drivers/cryptocop.c +10
-25@@ -2716,43 +2716,28 @@ static int cryptocop_ioctl_process(struct inode *inode, struct file *filp, unsig } } -/* Acquire the mm page semaphore. */ -down_read (¤t->mm->mmap_sem); - - err =get_user_pages (current, - current->mm, - (unsigned long int )(oper.indata + prev_ix), - noinpages, -0 ,/* read access only for in data */ -0 ,/* no force */ - inpages, -NULL ); + err =get_user_pages_fast ((unsigned long )(oper.indata + prev_ix), + noinpages, +0 ,/* read access only for in data */ + inpages);if (err <0 ) { -up_read (¤t->mm->mmap_sem); nooutpages = noinpages =0 ; -DEBUG_API (printk ("cryptocop_ioctl_process: get_user_pages indata\n" )); +DEBUG_API (printk ("cryptocop_ioctl_process: get_user_pages_fast indata\n" )); goto error_cleanup; } noinpages = err;if (oper.do_cipher){ - err =get_user_pages (current, - current->mm, - (unsigned long int )oper.cipher_outdata, - nooutpages, -1 ,/* write access for out data */ -0 ,/* no force */ - outpages, -NULL ); -up_read (¤t->mm->mmap_sem); + err =get_user_pages_fast ((unsigned long )oper.cipher_outdata, + nooutpages, +1 ,/* write access for out data */ + outpages);if (err <0 ) { nooutpages =0 ; -DEBUG_API (printk ("cryptocop_ioctl_process: get_user_pages outdata\n" )); +DEBUG_API (printk ("cryptocop_ioctl_process: get_user_pages_fast outdata\n" )); goto error_cleanup; } nooutpages = err; - }else { -up_read (¤t->mm->mmap_sem); }/* Add 6 to nooutpages to make room for possibly inserted buffers for storing digest and Signature · 1 line
2.1.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 1/7] cris: Convert cryptocop to use get_user_pages_fast()
In reply to Jan KaraQuoted 4 lines · Jan Kara
From: Jan Kara <[email protected]>
CC Mikael Starvik <[email protected]>
Quoted 1 line
Signed-off-by Jan Kara <[email protected]>
/^JN - Jesper Nilsson
Signature · 7 lines
Jesper Nilsson -- [email protected] -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 2/7 ia64: Use get_user_pages_fast() in err_inject.c
In reply to Jan KaraFrom: Jan Kara <[email protected]>
Convert get_user_pages() call to get_user_pages_fast(). This actually fixes an apparent bug where get_user_pages() has been called without mmap_sem for an arbitrary user-provided address.
--- arch/ia64/kernel/err_inject.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/ia64/kernel/err_inject.c +1 -2
@@ -142,8 +142,7 @@ store_virtual_to_phys(struct device *dev, struct device_attribute *attr,u64 virt_addr=simple_strtoull (buf,NULL ,16 );int ret; - ret =get_user_pages (current, current->mm, virt_addr, -1 ,VM_READ ,0 ,NULL ,NULL ); + ret =get_user_pages_fast (virt_addr,1 ,VM_READ ,NULL );if (ret<=0 ) {#ifdef ERR_INJ_DEBUG printk ("Virtual address %lx is not existing.\n" ,virt_addr);
Signature · 1 line
2.1.4
PATCH 3/7 drm: Convert via driver to use get_user_pages_fast()
In reply to Jan KaraFrom: Jan Kara <[email protected]>
--- drivers/gpu/drm/via/via_dmablit.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/gpu/drm/via/via_dmablit.c +4 -8
@@ -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;
Signature · 7 lines
2.1.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 4/7 fsl_hypervisor: Convert ioctl_memcpy() to use get_user_pages_fast()
In reply to Jan KaraFrom: Jan Kara <[email protected]>
--- drivers/virt/fsl_hypervisor.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/virt/fsl_hypervisor.c +2 -7
@@ -243,13 +243,8 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user *p) sg_list =PTR_ALIGN (sg_list_unaligned,sizeof (struct fh_sg_list ));/* Get the physical addresses of the source buffer */ -down_read (¤t->mm->mmap_sem); - num_pinned =get_user_pages (current, current->mm, - param.local_vaddr - lb_offset, num_pages, - (param.source ==-1 ) ?READ :WRITE , -0 , pages,NULL ); -up_read (¤t->mm->mmap_sem); - + num_pinned =get_user_pages_fast (param.local_vaddr - lb_offset, + num_pages, (param.source ==-1 ) ?READ :WRITE , pages);if (num_pinned != num_pages) {/* get_user_pages() failed */ pr_debug ("fsl-hv: could not lock source buffer\n" );
Signature · 7 lines
2.1.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 5/7 IB/mthca: Convert mthca_map_user_db() to use get_user_pages_fast()
In reply to Jan KaraFrom: Jan Kara <[email protected]>
Function mthca_map_user_db() appears to call get_user_pages() without holding mmap_sem. Fix the bug by using get_user_pages_fast() instead which also takes care of the locking.
--- drivers/infiniband/hw/mthca/mthca_memfree.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/infiniband/hw/mthca/mthca_memfree.c +1 -2
@@ -472,8 +472,7 @@ int mthca_map_user_db(struct mthca_dev *dev, struct mthca_uar *uar, goto out; } - ret =get_user_pages (current, current->mm, uaddr &PAGE_MASK ,1 ,1 ,0 , - pages,NULL ); + ret =get_user_pages_fast (uaddr &PAGE_MASK ,1 ,1 , pages);if (ret <0 ) goto out;
Signature · 6 lines
2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 6/7 drm/i915: Convert to use get_user_page_unlocked()
In reply to Jan KaraFrom: Jan Kara <[email protected]>
Convert __i915_gem_userptr_get_pages_worker() to use get_user_page_unlocked() so that we don't unnecessarily leak knowledge of mm locking into driver code.
--- drivers/gpu/drm/i915/i915_gem_userptr.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/gpu/drm/i915/i915_gem_userptr.c +6 -7
@@ -585,19 +585,18 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)if (pvec !=NULL ) {struct mm_struct *mm = obj->userptr.mm->mm; -down_read (&mm->mmap_sem);while (pinned < num_pages) { - ret =get_user_pages (work->task, mm, - obj->userptr.ptr + pinned *PAGE_SIZE , - num_pages - pinned, - !obj->userptr.read_only,0 , - pvec + pinned,NULL ); + ret =get_user_pages_unlocked ( + work->task, mm, + obj->userptr.ptr + pinned *PAGE_SIZE , + num_pages - pinned, + !obj->userptr.read_only,0 , + pvec + pinned);if (ret <0 )break ; pinned += ret; } -up_read (&mm->mmap_sem); }mutex_lock (&dev->struct_mutex);
Signature · 7 lines
2.1.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 7/7 ivtv: Convert to get_user_pages_unlocked()
In reply to Jan KaraFrom: Jan Kara <[email protected]>
Convert ivtv_yuv_prep_user_dma() to use get_user_pages_unlocked() so that we don't unnecessarily leak knowledge about mm locking into drivers code.
--- drivers/media/pci/ivtv/ivtv-yuv.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/media/pci/ivtv/ivtv-yuv.c +6 -6
@@ -75,15 +75,15 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct ivtv_user_dma *dma,ivtv_udma_get_page_info (&uv_dma, (unsigned long )args->uv_source,360 * uv_decode_height);/* Get user pages for DMA Xfer */ -down_read (¤t->mm->mmap_sem); - y_pages =get_user_pages (current, current->mm, y_dma.uaddr, y_dma.page_count,0 ,1 , &dma->map[0 ],NULL ); + y_pages =get_user_pages_unlocked (current, current->mm, + y_dma.uaddr, y_dma.page_count,0 ,1 , + &dma->map[0 ]); uv_pages =0 ;/* silence gcc. value is set and consumed only if: */ if (y_pages == y_dma.page_count) { - uv_pages =get_user_pages (current, current->mm, - uv_dma.uaddr, uv_dma.page_count,0 ,1 , - &dma->map[y_pages],NULL ); + uv_pages =get_user_pages_unlocked (current, current->mm, + uv_dma.uaddr, uv_dma.page_count,0 ,1 , + &dma->map[y_pages]); } -up_read (¤t->mm->mmap_sem);if (y_pages != y_dma.page_count || uv_pages != uv_dma.page_count) {int rc = -EFAULT ;
Signature · 1 line
2.1.4