From c803c9c6c9e591977635b7868e45fabb1e243f98 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 18 Nov 2017 14:17:46 -0500 Subject: [PATCH 1/5] fold __get_user_pages_unlocked() into its sole remaining caller Signed-off-by: Al Viro --- mm/gup.c | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/mm/gup.c b/mm/gup.c index dfcde13f289a..e7b9f5e97479 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -964,30 +964,6 @@ long get_user_pages_locked(unsigned long start, unsigned long nr_pages, } EXPORT_SYMBOL(get_user_pages_locked); -/* - * Same as get_user_pages_unlocked(...., FOLL_TOUCH) but it allows for - * tsk, mm to be specified. - * - * NOTE: here FOLL_TOUCH is not set implicitly and must be set by the - * caller if required (just like with __get_user_pages). "FOLL_GET" - * is set implicitly if "pages" is non-NULL. - */ -static __always_inline long __get_user_pages_unlocked(struct task_struct *tsk, - struct mm_struct *mm, unsigned long start, - unsigned long nr_pages, struct page **pages, - unsigned int gup_flags) -{ - long ret; - int locked = 1; - - down_read(&mm->mmap_sem); - ret = __get_user_pages_locked(tsk, mm, start, nr_pages, pages, NULL, - &locked, false, gup_flags); - if (locked) - up_read(&mm->mmap_sem); - return ret; -} - /* * get_user_pages_unlocked() is suitable to replace the form: * @@ -1006,8 +982,16 @@ static __always_inline long __get_user_pages_unlocked(struct task_struct *tsk, long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages, struct page **pages, unsigned int gup_flags) { - return __get_user_pages_unlocked(current, current->mm, start, nr_pages, - pages, gup_flags | FOLL_TOUCH); + struct mm_struct *mm = current->mm; + int locked = 1; + long ret; + + down_read(&mm->mmap_sem); + ret = __get_user_pages_locked(current, mm, start, nr_pages, pages, NULL, + &locked, false, gup_flags | FOLL_TOUCH); + if (locked) + up_read(&mm->mmap_sem); + return ret; } EXPORT_SYMBOL(get_user_pages_unlocked); From 9a949e8ff92246c0753b2805c2a001cb991fffe5 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 18 Nov 2017 14:37:46 -0500 Subject: [PATCH 2/5] cris: switch to get_user_pages_fast() no point holding ->mmap_sem over both calls. Signed-off-by: Al Viro --- arch/cris/arch-v32/drivers/cryptocop.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/arch/cris/arch-v32/drivers/cryptocop.c b/arch/cris/arch-v32/drivers/cryptocop.c index d688fe117dca..a3c353472a8c 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), + err = get_user_pages_fast((unsigned long)(oper.indata + prev_ix), noinpages, - 0, /* read access only for in data */ - inpages, - NULL); + false, /* 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")); goto error_cleanup; } noinpages = err; - if (oper.do_cipher){ - err = get_user_pages((unsigned long int)oper.cipher_outdata, + if (oper.do_cipher) { + err = get_user_pages_fast((unsigned long)oper.cipher_outdata, nooutpages, - FOLL_WRITE, /* write access for out data */ - outpages, - NULL); - up_read(¤t->mm->mmap_sem); + true, /* write access for out data */ + outpages); if (err < 0) { nooutpages = 0; DEBUG_API(printk("cryptocop_ioctl_process: get_user_pages 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 From 14cb138d7c1c749d81dc3e66cd70f7a884e1da56 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 19 Nov 2017 11:21:10 -0500 Subject: [PATCH 3/5] get_user_pages_unlocked(): pass true to __get_user_pages_locked() notify_drop Equivalent transformation - the only place in __get_user_pages_locked() where we look at notify_drop argument is if (notify_drop && lock_dropped && *locked) { up_read(&mm->mmap_sem); *locked = 0; } in the very end. Changing notify_drop from false to true won't change behaviour unless *locked is non-zero. The caller is ret = __get_user_pages_locked(current, mm, start, nr_pages, pages, NULL, &locked, false, gup_flags | FOLL_TOUCH); if (locked) up_read(&mm->mmap_sem); so in that case the original kernel would have done up_read() right after return from __get_user_pages_locked(), while the modified one would've done it right before the return. Signed-off-by: Al Viro --- mm/gup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/gup.c b/mm/gup.c index e7b9f5e97479..9418cbb3b1ad 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -988,7 +988,7 @@ long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages, down_read(&mm->mmap_sem); ret = __get_user_pages_locked(current, mm, start, nr_pages, pages, NULL, - &locked, false, gup_flags | FOLL_TOUCH); + &locked, true, gup_flags | FOLL_TOUCH); if (locked) up_read(&mm->mmap_sem); return ret; From e716712f83b635e62d5fb66c1375524ef2152cc0 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 19 Nov 2017 11:32:05 -0500 Subject: [PATCH 4/5] __get_user_pages_locked(): get rid of notify_drop argument The only caller that doesn't pass true in it is get_user_pages() and it passes NULL in locked. The only place where we check it is if (notify_locked && lock_dropped && *locked) and lock_dropped can become true only if we have locked != NULL. In other words, the second part of condition will be false when called by get_user_pages(). Just get rid of the argument and turn the condition into if (lock_dropped && *locked) Signed-off-by: Al Viro --- mm/gup.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mm/gup.c b/mm/gup.c index 9418cbb3b1ad..61015793f952 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -848,7 +848,7 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk, unsigned long nr_pages, struct page **pages, struct vm_area_struct **vmas, - int *locked, bool notify_drop, + int *locked, unsigned int flags) { long ret, pages_done; @@ -922,7 +922,7 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk, pages++; start += PAGE_SIZE; } - if (notify_drop && lock_dropped && *locked) { + if (lock_dropped && *locked) { /* * We must let the caller know we temporarily dropped the lock * and so the critical section protected by it was lost. @@ -959,7 +959,7 @@ long get_user_pages_locked(unsigned long start, unsigned long nr_pages, int *locked) { return __get_user_pages_locked(current, current->mm, start, nr_pages, - pages, NULL, locked, true, + pages, NULL, locked, gup_flags | FOLL_TOUCH); } EXPORT_SYMBOL(get_user_pages_locked); @@ -988,7 +988,7 @@ long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages, down_read(&mm->mmap_sem); ret = __get_user_pages_locked(current, mm, start, nr_pages, pages, NULL, - &locked, true, gup_flags | FOLL_TOUCH); + &locked, gup_flags | FOLL_TOUCH); if (locked) up_read(&mm->mmap_sem); return ret; @@ -1057,7 +1057,7 @@ long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm, struct vm_area_struct **vmas, int *locked) { return __get_user_pages_locked(tsk, mm, start, nr_pages, pages, vmas, - locked, true, + locked, gup_flags | FOLL_TOUCH | FOLL_REMOTE); } EXPORT_SYMBOL(get_user_pages_remote); @@ -1074,7 +1074,7 @@ long get_user_pages(unsigned long start, unsigned long nr_pages, struct vm_area_struct **vmas) { return __get_user_pages_locked(current, current->mm, start, nr_pages, - pages, vmas, NULL, false, + pages, vmas, NULL, gup_flags | FOLL_TOUCH); } EXPORT_SYMBOL(get_user_pages); From ce53053ce378c21e7ffc45241fd67d6ee79daa2b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 19 Nov 2017 17:47:33 -0500 Subject: [PATCH 5/5] kvm: switch get_user_page_nowait() to get_user_pages_unlocked() ... and fold into the sole caller, unifying async and non-async cases Signed-off-by: Al Viro --- virt/kvm/kvm_main.c | 43 ++++++++++++------------------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index f169ecc4f2e8..ae4985bc8a8a 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1314,17 +1314,6 @@ unsigned long kvm_vcpu_gfn_to_hva_prot(struct kvm_vcpu *vcpu, gfn_t gfn, bool *w return gfn_to_hva_memslot_prot(slot, gfn, writable); } -static int get_user_page_nowait(unsigned long start, int write, - struct page **page) -{ - int flags = FOLL_NOWAIT | FOLL_HWPOISON; - - if (write) - flags |= FOLL_WRITE; - - return get_user_pages(start, 1, flags, page, NULL); -} - static inline int check_user_page_hwpoison(unsigned long addr) { int rc, flags = FOLL_HWPOISON | FOLL_WRITE; @@ -1373,7 +1362,8 @@ static bool hva_to_pfn_fast(unsigned long addr, bool atomic, bool *async, static int hva_to_pfn_slow(unsigned long addr, bool *async, bool write_fault, bool *writable, kvm_pfn_t *pfn) { - struct page *page[1]; + unsigned int flags = FOLL_HWPOISON; + struct page *page; int npages = 0; might_sleep(); @@ -1381,35 +1371,26 @@ static int hva_to_pfn_slow(unsigned long addr, bool *async, bool write_fault, if (writable) *writable = write_fault; - if (async) { - down_read(¤t->mm->mmap_sem); - npages = get_user_page_nowait(addr, write_fault, page); - up_read(¤t->mm->mmap_sem); - } else { - unsigned int flags = FOLL_HWPOISON; + if (write_fault) + flags |= FOLL_WRITE; + if (async) + flags |= FOLL_NOWAIT; - if (write_fault) - flags |= FOLL_WRITE; - - npages = get_user_pages_unlocked(addr, 1, page, flags); - } + npages = get_user_pages_unlocked(addr, 1, &page, flags); if (npages != 1) return npages; /* map read fault as writable if possible */ if (unlikely(!write_fault) && writable) { - struct page *wpage[1]; + struct page *wpage; - npages = __get_user_pages_fast(addr, 1, 1, wpage); - if (npages == 1) { + if (__get_user_pages_fast(addr, 1, 1, &wpage) == 1) { *writable = true; - put_page(page[0]); - page[0] = wpage[0]; + put_page(page); + page = wpage; } - - npages = 1; } - *pfn = page_to_pfn(page[0]); + *pfn = page_to_pfn(page); return npages; }