Since we currently hold mmap_sem across both gup calls (and
nothing more), we can substitute it with two _fast()
alternatives and possibly avoid grabbing the lock.
This was found while adding mmap_sem wrappers, and was also
previously reported by Al: https://lkml.org/lkml/2017/11/17/777
Signed-off-by: Davidlohr Bueso <[email protected]>
---
arch/cris/arch-v32/drivers/cryptocop.c | 29 ++++++++++-------------------
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/arch/cris/arch-v32/drivers/cryptocop.c b/arch/cris/arch-v32/drivers/cryptocop.c
index d688fe117dca..76f8d3b1d39e 100644
--- a/arch/cris/arch-v32/drivers/cryptocop.c
+++ b/arch/cris/arch-v32/drivers/cryptocop.c
@@ -2717,37 +2717,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((unsigned long int)(oper.indata + prev_ix),
- noinpages,
- 0, /* read access only for in data */
- inpages,
- NULL);
+ err = get_user_pages_fast((unsigned long int)(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((unsigned long int)oper.cipher_outdata,
- nooutpages,
- FOLL_WRITE, /* write access for out data */
- outpages,
- NULL);
- up_read(¤t->mm->mmap_sem);
+ err = get_user_pages_fast((unsigned long int)oper.cipher_outdata,
+ nooutpages,
+ FOLL_WRITE, /* 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
--
2.13.6
On Sun, Jan 21, 2018 at 02:59:29PM -0800, Davidlohr Bueso wrote:
> Since we currently hold mmap_sem across both gup calls (and
> nothing more), we can substitute it with two _fast()
> alternatives and possibly avoid grabbing the lock.
>
> This was found while adding mmap_sem wrappers, and was also
> previously reported by Al: https://lkml.org/lkml/2017/11/17/777
See commit 9a949e8ff92246c0753b2805c2a001cb991fffe5
Author: Al Viro <[email protected]>
Date: Sat Nov 18 14:37:46 2017 -0500
cris: switch to get_user_pages_fast()
no point holding ->mmap_sem over both calls.
Signed-off-by: Al Viro <[email protected]>
in -next...
On Sun, 21 Jan 2018, Al Viro wrote:
>On Sun, Jan 21, 2018 at 02:59:29PM -0800, Davidlohr Bueso wrote:
>> Since we currently hold mmap_sem across both gup calls (and
>> nothing more), we can substitute it with two _fast()
>> alternatives and possibly avoid grabbing the lock.
>>
>> This was found while adding mmap_sem wrappers, and was also
>> previously reported by Al: https://lkml.org/lkml/2017/11/17/777
>
>See commit 9a949e8ff92246c0753b2805c2a001cb991fffe5
>Author: Al Viro <[email protected]>
>Date: Sat Nov 18 14:37:46 2017 -0500
>
> cris: switch to get_user_pages_fast()
>
> no point holding ->mmap_sem over both calls.
>
> Signed-off-by: Al Viro <[email protected]>
Serves me for developing on Linus' tree, cool.
Now, regarding the other two which are technically bugs (not
holding mmap_sem under gup), you mention I don't see those in
next, so I'm assuming you didn't get to them.
- mthca_map_user_db(): I came to the same conclusion and tempted
to send out the same idea of unlocking the mutex for gup_fast()
so we can take mmap_sem if needed.
- ia64's store_virtual_to_phys() is a sysfs attribute and
holds no (other) locks. How about we just convert it as well?
diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
index 85bba43e7d5d..658a8e06a69b 100644
--- a/arch/ia64/kernel/err_inject.c
+++ b/arch/ia64/kernel/err_inject.c
@@ -142,7 +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(virt_addr, 1, FOLL_WRITE, NULL, NULL);
+ ret = get_user_pages_fast(virt_addr, 1, FOLL_WRITE, NULL);
if (ret<=0) {
#ifdef ERR_INJ_DEBUG
printk("Virtual address %lx is not existing.\n",virt_addr);
Thanks,
Davidlohr