Re: [PATCH 02/10] mm: remove write/force parameters from __get_user_pages_unlocked()

[Jan Kara]

On Thu 13-10-16 01:20:12, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from
> __get_user_pages_unlocked() to make the use of FOLL_FORCE explicit in callers 
> as
> use of this flag can result in surprising behaviour (and hence bugs) within 
> the
> mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes 

The patch looks good. You can add:

Reviewed-by: Jan Kara 

Honza
-- 
Jan Kara 
SUSE Labs, CR

[PATCH 02/10] mm: remove write/force parameters from __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch removes the write and force parameters from
__get_user_pages_unlocked() to make the use of FOLL_FORCE explicit in callers as
use of this flag can result in surprising behaviour (and hence bugs) within the
mm subsystem.

Signed-off-by: Lorenzo Stoakes 
---
 include/linux/mm.h |  3 +--
 mm/gup.c   | 17 +
 mm/nommu.c | 12 +---
 mm/process_vm_access.c |  7 +--
 virt/kvm/async_pf.c|  3 ++-
 virt/kvm/kvm_main.c| 11 ---
 6 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index e9caec6..2db98b6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1285,8 +1285,7 @@ long get_user_pages_locked(unsigned long start, unsigned 
long nr_pages,
int write, int force, struct page **pages, int *locked);
 long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
   unsigned long start, unsigned long nr_pages,
-  int write, int force, struct page **pages,
-  unsigned int gup_flags);
+  struct page **pages, unsigned int gup_flags);
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
int write, int force, struct page **pages);
 int get_user_pages_fast(unsigned long start, int nr_pages, int write,
diff --git a/mm/gup.c b/mm/gup.c
index ba83942..3d620dd 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -865,17 +865,11 @@ EXPORT_SYMBOL(get_user_pages_locked);
  */
 __always_inline long __get_user_pages_unlocked(struct task_struct *tsk, struct 
mm_struct *mm,
   unsigned long start, unsigned 
long nr_pages,
-  int write, int force, struct 
page **pages,
-  unsigned int gup_flags)
+  struct page **pages, unsigned 
int gup_flags)
 {
long ret;
int locked = 1;
 
-   if (write)
-   gup_flags |= FOLL_WRITE;
-   if (force)
-   gup_flags |= FOLL_FORCE;
-
down_read(>mmap_sem);
ret = __get_user_pages_locked(tsk, mm, start, nr_pages, pages, NULL,
  , false, gup_flags);
@@ -905,8 +899,15 @@ EXPORT_SYMBOL(__get_user_pages_unlocked);
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 int write, int force, struct page **pages)
 {
+   unsigned int flags = FOLL_TOUCH;
+
+   if (write)
+   flags |= FOLL_WRITE;
+   if (force)
+   flags |= FOLL_FORCE;
+
return __get_user_pages_unlocked(current, current->mm, start, nr_pages,
-write, force, pages, FOLL_TOUCH);
+pages, flags);
 }
 EXPORT_SYMBOL(get_user_pages_unlocked);
 
diff --git a/mm/nommu.c b/mm/nommu.c
index 95daf81..925dcc1 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -185,8 +185,7 @@ EXPORT_SYMBOL(get_user_pages_locked);
 
 long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
   unsigned long start, unsigned long nr_pages,
-  int write, int force, struct page **pages,
-  unsigned int gup_flags)
+  struct page **pages, unsigned int gup_flags)
 {
long ret;
down_read(>mmap_sem);
@@ -200,8 +199,15 @@ EXPORT_SYMBOL(__get_user_pages_unlocked);
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 int write, int force, struct page **pages)
 {
+   unsigned int flags = 0;
+
+   if (write)
+   flags |= FOLL_WRITE;
+   if (force)
+   flags |= FOLL_FORCE;
+
return __get_user_pages_unlocked(current, current->mm, start, nr_pages,
-write, force, pages, 0);
+pages, flags);
 }
 EXPORT_SYMBOL(get_user_pages_unlocked);
 
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index 07514d4..be8dc8d 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -88,12 +88,16 @@ static int process_vm_rw_single_vec(unsigned long addr,
ssize_t rc = 0;
unsigned long max_pages_per_loop = PVM_MAX_KMALLOC_PAGES
/ sizeof(struct pages *);
+   unsigned int flags = FOLL_REMOTE;
 
/* Work out address and page range required */
if (len == 0)
return 0;
nr_pages = (addr + len - 1) / PAGE_SIZE - addr / PAGE_SIZE + 1;
 
+   if (vm_write)
+   flags |= FOLL_WRITE;
+
while (!rc && nr_pages && iov_iter_count(iter)) {
int pages = min(nr_pages, max_pages_per_loop);
size_t bytes;
@@ -104,8