[tip: x86/mm] x86/mm: Increase pgt_buf size for 5-level page tables

[tip-bot2 for Lorenzo Stoakes]

The following commit has been merged into the x86/mm branch of tip:

Commit-ID: 167dcfc08b0b1f964ea95d410aa496fd78adf475
Gitweb:
https://git.kernel.org/tip/167dcfc08b0b1f964ea95d410aa496fd78adf475
Author:Lorenzo Stoakes 
AuthorDate:Tue, 15 Dec 2020 20:56:41 
Committer: Borislav Petkov 
CommitterDate: Mon, 04 Jan 2021 18:07:50 +01:00

x86/mm: Increase pgt_buf size for 5-level page tables

pgt_buf is used to allocate page tables on initial direct page mapping
which bootstraps the kernel into being able to allocate these before the
direct mapping makes further pages available.

INIT_PGD_PAGE_COUNT is set to 6 pages (doubled for KASLR) - 3 (PUD, PMD,
PTE) for the 1 MiB ISA mapping and 3 more for the first direct mapping
assignment in each case providing 2 MiB of address space.

This has not been updated for 5-level page tables which has an
additional P4D page table level above PUD.

In most instances, this will not have a material impact as the first
4 page levels allocated for the ISA mapping will provide sufficient
address space to encompass all further address mappings.

If the first direct mapping is within 512 GiB of the ISA mapping, only
a PMD and PTE needs to be added in the instance the kernel is using 4
KiB page tables (e.g. CONFIG_DEBUG_PAGEALLOC is enabled) and only a PMD
if the kernel can use 2 MiB pages (the first allocation is limited to
PMD_SIZE so a GiB page cannot be used there).

However, if the machine has more than 512 GiB of RAM and the kernel is
allocating 4 KiB page size, 3 further page tables are required.

If the machine has more than 256 TiB of RAM at 4 KiB or 2 MiB page size,
further 3 or 4 page tables are required respectively.

Update INIT_PGD_PAGE_COUNT to reflect this.

 [ bp: Sanitize text into passive voice without ambiguous personal pronouns. ]

Signed-off-by: Lorenzo Stoakes 
Signed-off-by: Borislav Petkov 
Acked-by: Kirill A. Shutemov 
Acked-by: Dave Hansen 
Link: https://lkml.kernel.org/r/20201215205641.34096-1-lstoa...@gmail.com
---
 arch/x86/mm/init.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index e26f5c5..dd694fb 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -157,16 +157,25 @@ __ref void *alloc_low_pages(unsigned int num)
 }
 
 /*
- * By default need 3 4k for initial PMD_SIZE,  3 4k for 0-ISA_END_ADDRESS.
- * With KASLR memory randomization, depending on the machine e820 memory
- * and the PUD alignment. We may need twice more pages when KASLR memory
+ * By default need to be able to allocate page tables below PGD firstly for
+ * the 0-ISA_END_ADDRESS range and secondly for the initial PMD_SIZE mapping.
+ * With KASLR memory randomization, depending on the machine e820 memory and 
the
+ * PUD alignment, twice that many pages may be needed when KASLR memory
  * randomization is enabled.
  */
+
+#ifndef CONFIG_X86_5LEVEL
+#define INIT_PGD_PAGE_TABLES3
+#else
+#define INIT_PGD_PAGE_TABLES4
+#endif
+
 #ifndef CONFIG_RANDOMIZE_MEMORY
-#define INIT_PGD_PAGE_COUNT  6
+#define INIT_PGD_PAGE_COUNT  (2 * INIT_PGD_PAGE_TABLES)
 #else
-#define INIT_PGD_PAGE_COUNT  12
+#define INIT_PGD_PAGE_COUNT  (4 * INIT_PGD_PAGE_TABLES)
 #endif
+
 #define INIT_PGT_BUF_SIZE  (INIT_PGD_PAGE_COUNT * PAGE_SIZE)
 RESERVE_BRK(early_pgt_alloc, INIT_PGT_BUF_SIZE);
 void  __init early_alloc_pgt_buf(void)

Re: [PATCH] x86/mm: increase pgt_buf size for 5-level page tables

[Lorenzo Stoakes]

Happy NY all, thanks for the review! I haven't contributed an
x86-specific mm patch before so am not sure of the process - usually I
submit patches to the mm mailing list and Andrew gathers them up into
his tree, is there anything else I need to do with this?

Thanks, Lorenzo

[PATCH] x86/mm: increase pgt_buf size for 5-level page tables

[Lorenzo Stoakes]

pgt_buf is used to allocate page tables on initial direct page mapping
bootstrapping us into being able to allocate these before the direct
mapping makes further pages available.

INIT_PGD_PAGE_COUNT is set to 6 pages (doubled for KASLR) - 3 (PUD, PMD,
PTE) for the 1 MiB ISA mapping and 3 more for the first direct mapping
assignment in each case providing 2 MiB of address space.

This has not been updated for 5-level page tables which additionally has a
P4D page table level above PUD.

In most instances this will not have a material impact as the first 4 page
levels allocated for the ISA mapping will provide sufficient address space
to encompass all further address mappings. If the first direct mapping is
within 512 GiB of the ISA mapping we need only add a PMD and PTE in the
instance where we are using 4 KiB page tables (e.g. CONFIG_DEBUG_PAGEALLOC
is enabled) and only a PMD if we can use 2 MiB pages (the first allocation
is limited to PMD_SIZE so we can't use a GiB page there).

However if we have more than 512 GiB of RAM and are allocating 4 KiB page
size we require 3 further page tables and if we have more than 256 TiB of
RAM at 4 KiB or 2 MiB page size we require a further 3 or 4 page tables
respectively.

This patch updates INIT_PGD_PAGE_COUNT to reflect this.

Signed-off-by: Lorenzo Stoakes 
---
 arch/x86/mm/init.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index e26f5c5c6565..0ee7dc9a5a65 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -157,16 +157,25 @@ __ref void *alloc_low_pages(unsigned int num)
 }
 
 /*
- * By default need 3 4k for initial PMD_SIZE,  3 4k for 0-ISA_END_ADDRESS.
- * With KASLR memory randomization, depending on the machine e820 memory
- * and the PUD alignment. We may need twice more pages when KASLR memory
+ * By default we need to be able to allocate page tables below PGD firstly for
+ * the 0-ISA_END_ADDRESS range and secondly for the initial PMD_SIZE mapping.
+ * With KASLR memory randomization, depending on the machine e820 memory and 
the
+ * PUD alignment, we may need twice that many pages when KASLR memory
  * randomization is enabled.
  */
+
+#ifndef CONFIG_X86_5LEVEL
+#define INIT_PGD_PAGE_TABLES3
+#else
+#define INIT_PGD_PAGE_TABLES4
+#endif
+
 #ifndef CONFIG_RANDOMIZE_MEMORY
-#define INIT_PGD_PAGE_COUNT  6
+#define INIT_PGD_PAGE_COUNT  (2 * INIT_PGD_PAGE_TABLES)
 #else
-#define INIT_PGD_PAGE_COUNT  12
+#define INIT_PGD_PAGE_COUNT  (4 * INIT_PGD_PAGE_TABLES)
 #endif
+
 #define INIT_PGT_BUF_SIZE  (INIT_PGD_PAGE_COUNT * PAGE_SIZE)
 RESERVE_BRK(early_pgt_alloc, INIT_PGT_BUF_SIZE);
 void  __init early_alloc_pgt_buf(void)
-- 
2.29.2

Re: linux-next boot error: kernel BUG at include/linux/page-flags.h:LINE!

[Lorenzo Stoakes]

On Thu, 3 Dec 2020 at 12:24, Mike Rapoport  wrote:
> Yeah, the change to initialization of "unavailable" memory missed pfn 0 :(
> This should fix it:

Tried locally and it fixes the issue for me :)

[PATCH] mm: page_alloc: refactor setup_per_zone_lowmem_reserve()

[Lorenzo Stoakes]

setup_per_zone_lowmem_reserve() iterates through each zone setting
zone->lowmem_reserve[j] = 0 (where j is the zone's index) then
iterates backwards through all proceeding zones, setting
lower_zone->lowmem_reserve[j] = sum(managed pages of higher zones) /
lowmem_reserve_ratio[idx] for each (where idx is the lower zone's
index).

If the lower zone has no managed pages or its ratio is 0 then all of its
lowmem_reserve[] entries are effectively zeroed.

As these arrays are only assigned here and all lowmem_reserve[] entries
for index < this zone's index are implicitly assumed to be 0 (as these
are specifically output in show_free_areas() and zoneinfo_show_print()
for example) there is no need to additionally zero index == this zone's
index too. This patch avoids zeroing unnecessarily.

Rather than iterating through zones and setting lowmem_reserve[j] for
each lower zone this patch reverse the process and populates each zone's
lowmem_reserve[] values in ascending order.

This clarifies what is going on especially in the case of zero managed
pages or ratio which is now explicitly shown to clear these values.

Signed-off-by: Lorenzo Stoakes 
---
 mm/page_alloc.c | 35 ++-
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f91df593bf71..39f92de1228f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7862,31 +7862,24 @@ static void calculate_totalreserve_pages(void)
 static void setup_per_zone_lowmem_reserve(void)
 {
struct pglist_data *pgdat;
-   enum zone_type j, idx;
+   enum zone_type i, j;
 
for_each_online_pgdat(pgdat) {
-   for (j = 0; j < MAX_NR_ZONES; j++) {
-   struct zone *zone = pgdat->node_zones + j;
-   unsigned long managed_pages = zone_managed_pages(zone);
-
-   zone->lowmem_reserve[j] = 0;
-
-   idx = j;
-   while (idx) {
-   struct zone *lower_zone;
-
-   idx--;
-   lower_zone = pgdat->node_zones + idx;
-
-   if (!sysctl_lowmem_reserve_ratio[idx] ||
-   !zone_managed_pages(lower_zone)) {
-   lower_zone->lowmem_reserve[j] = 0;
-   continue;
+   for (i = 0; i < MAX_NR_ZONES - 1; i++) {
+   struct zone *zone = >node_zones[i];
+   int ratio = sysctl_lowmem_reserve_ratio[i];
+   bool clear = !ratio || !zone_managed_pages(zone);
+   unsigned long managed_pages = 0;
+
+   for (j = i + 1; j < MAX_NR_ZONES; j++) {
+   if (clear) {
+   zone->lowmem_reserve[j] = 0;
} else {
-   lower_zone->lowmem_reserve[j] =
-   managed_pages / 
sysctl_lowmem_reserve_ratio[idx];
+   struct zone *upper_zone = 
>node_zones[j];
+
+   managed_pages += 
zone_managed_pages(upper_zone);
+   zone->lowmem_reserve[j] = managed_pages 
/ ratio;
}
-   managed_pages += zone_managed_pages(lower_zone);
}
}
}
-- 
2.29.2

Re: linux-next boot error: WARNING in prepare_kswapd_sleep

[Lorenzo Stoakes]

On Wed, 25 Nov 2020 at 06:25, Alex Shi  wrote:
> Acked.

Thanks. I submitted as an actual patch, refactored it slightly to
avoid duplication of page_memcg().

> and further more, could you like try another patch?

I tried that patch against the syzkaller failure case and it worked fine!

Cheers, Lorenzo

[PATCH] mm/memcg: warn on missing memcg on mem_cgroup_page_lruvec()

[Lorenzo Stoakes]

Move memcg check to mem_cgroup_page_lruvec() as there are callers which
may invoke this with !memcg in mem_cgroup_lruvec(), whereas they should
not in mem_cgroup_page_lruvec().

We expect that we have always charged a page to the memcg before
mem_cgroup_page_lruvec() is invoked, so add a warning to assert that this
is the case.

Signed-off-by: Lorenzo Stoakes 
Reported-by: syzbot+ce635500093181f39...@syzkaller.appspotmail.com
---
 include/linux/memcontrol.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 87ed56dc75f9..3e6a1df3bdb9 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -618,7 +618,6 @@ static inline struct lruvec *mem_cgroup_lruvec(struct 
mem_cgroup *memcg,
goto out;
}
 
-   VM_WARN_ON_ONCE(!memcg);
if (!memcg)
memcg = root_mem_cgroup;
 
@@ -645,7 +644,10 @@ static inline struct lruvec *mem_cgroup_lruvec(struct 
mem_cgroup *memcg,
 static inline struct lruvec *mem_cgroup_page_lruvec(struct page *page,
struct pglist_data *pgdat)
 {
-   return mem_cgroup_lruvec(page_memcg(page), pgdat);
+   struct mem_cgroup *memcg = page_memcg(page);
+
+   VM_WARN_ON_ONCE_PAGE(!memcg, page);
+   return mem_cgroup_lruvec(memcg, pgdat);
 }
 
 static inline bool lruvec_holds_page_lru_lock(struct page *page,
-- 
2.29.2

Re: linux-next boot error: WARNING in prepare_kswapd_sleep

[Lorenzo Stoakes]

On Tue, 24 Nov 2020 at 07:54, syzbot
 wrote:
> syzbot found the following issue on:
>
> HEAD commit:d9137320 Add linux-next specific files for 20201124

This appears to be a product of 4b2904f3 ("mm/memcg: add missed
warning in mem_cgroup_lruvec") adding a VM_WARN_ON_ONCE() to
mem_cgroup_lruvec, which when invoked from a function other than
mem_cgroup_page_lruvec() can in fact be called with the condition
false.
If we move the check back into mem_cgroup_page_lruvec() it resolves
the issue. I enclose a simple version of this below, happy to submit
as a proper patch if this is the right approach:


diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 87ed56dc75f9..27cc40a490b2 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -618,7 +618,6 @@ static inline struct lruvec
*mem_cgroup_lruvec(struct mem_cgroup *memcg,
goto out;
}

-   VM_WARN_ON_ONCE(!memcg);
if (!memcg)
memcg = root_mem_cgroup;

@@ -645,6 +644,7 @@ static inline struct lruvec
*mem_cgroup_lruvec(struct mem_cgroup *memcg,
 static inline struct lruvec *mem_cgroup_page_lruvec(struct page *page,
struct pglist_data *pgdat)
 {
+   VM_WARN_ON_ONCE_PAGE(!page_memcg(page), page);
return mem_cgroup_lruvec(page_memcg(page), pgdat);
 }

Re: linux-next boot error: BUG: unable to handle kernel NULL pointer dereference in mempool_init_node

[Lorenzo Stoakes]

On Wed, 11 Nov 2020 at 17:44, Andrey Konovalov  wrote:
> I'll try to reproduce this and figure out the issue. Thanks for letting us 
> know!

I hope you don't mind me diving in here, I was taking a look just now
and managed to reproduce this locally - I bisected the issue to
105397399 ("kasan: simplify kasan_poison_kfree").

If I stick a simple check in as below it fixes the issue, so I'm
guessing something is violating the assumptions in 105397399?


diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 7a94cebc0324..16163159a017 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -387,6 +387,11 @@ void __kasan_slab_free_mempool(void *ptr, unsigned long ip)
struct page *page;

page = virt_to_head_page(ptr);
+
+   if (!PageSlab(page)) {
+   return;
+   }
+
kasan_slab_free(page->slab_cache, ptr, ip, false);
 }


--
Lorenzo Stoakes
https://ljs.io

Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

On 28 February 2017 at 19:35, Al Viro <v...@zeniv.linux.org.uk> wrote:
> On Tue, Feb 28, 2017 at 10:01:10AM +0100, Daniel Vetter wrote:
>
>> > +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
>> > +   vsg->num_pages, vsg->pages,
>> > +   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
>
> Umm...  Why not
> ret = get_user_pages_fast((unsigned long)xfer->mem_addr,
> vsg->num_pages,
> vsg->direction == DMA_FROM_DEVICE,
> vsg->pages);
>
> IOW, do you really need a warranty that ->mmap_sem will be grabbed and
> released?

Daniel will be better placed to answer in this specific case, but more
generally is there any reason why we can't just use
get_user_pages_fast() in all such cases? These patches were simply a
mechanical/cautious replacement for code that is more or less exactly
equivalent but if this would make sense perhaps it'd be worth using
gup_fast() where possible?

-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

On 28 February 2017 at 19:35, Al Viro  wrote:
> On Tue, Feb 28, 2017 at 10:01:10AM +0100, Daniel Vetter wrote:
>
>> > +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
>> > +   vsg->num_pages, vsg->pages,
>> > +   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
>
> Umm...  Why not
> ret = get_user_pages_fast((unsigned long)xfer->mem_addr,
> vsg->num_pages,
> vsg->direction == DMA_FROM_DEVICE,
> vsg->pages);
>
> IOW, do you really need a warranty that ->mmap_sem will be grabbed and
> released?

Daniel will be better placed to answer in this specific case, but more
generally is there any reason why we can't just use
get_user_pages_fast() in all such cases? These patches were simply a
mechanical/cautious replacement for code that is more or less exactly
equivalent but if this would make sense perhaps it'd be worth using
gup_fast() where possible?

-- 
Lorenzo Stoakes
https://ljs.io

[PATCH RESEND] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 drivers/gpu/drm/via/via_dmablit.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dmablit.c 
b/drivers/gpu/drm/via/via_dmablit.c
index 1a3ad769f8c8..98aae9809249 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
-   ret = get_user_pages((unsigned long)xfer->mem_addr,
-vsg->num_pages,
-(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 
0,
-vsg->pages, NULL);
-
-   up_read(>mm->mmap_sem);
+   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
+   vsg->num_pages, vsg->pages,
+   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
if (ret != vsg->num_pages) {
if (ret < 0)
return ret;
-- 
2.11.1

[PATCH RESEND] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes 
---
 drivers/gpu/drm/via/via_dmablit.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dmablit.c 
b/drivers/gpu/drm/via/via_dmablit.c
index 1a3ad769f8c8..98aae9809249 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
-   ret = get_user_pages((unsigned long)xfer->mem_addr,
-vsg->num_pages,
-(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 
0,
-vsg->pages, NULL);
-
-   up_read(>mm->mmap_sem);
+   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
+   vsg->num_pages, vsg->pages,
+   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
if (ret != vsg->num_pages) {
if (ret < 0)
return ret;
-- 
2.11.1

Re: [PATCH] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

On 6 January 2017 at 07:09, Lorenzo Stoakes <lstoa...@gmail.com> wrote:
>
> Adding Andrew, as this may be another less active corner of the corner, 
> thanks!

Hi all,

Thought I'd also give this one a gentle nudge now the merge window has
re-opened, Andrew - are you ok to pick this up? I've checked the patch
and it still applies, for convenience the raw patch is available at
https://marc.info/?l=linux-mm=147802942832515=raw - let me know if
there's anything else I can do on this or if you'd prefer a re-send.

Best, Lorenzo

> On 3 January 2017 at 20:23, Lorenzo Stoakes <lstoa...@gmail.com> wrote:
>> Hi All,
>>
>> Just a gentle ping on this one :)
>>
>> Cheers, Lorenzo
>>
>> On 1 November 2016 at 19:43, Lorenzo Stoakes <lstoa...@gmail.com> wrote:
>>> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the 
>>> code
>>> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>>>
>>> Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
>>> ---
>>>  drivers/gpu/drm/via/via_dmablit.c | 10 +++---
>>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/via/via_dmablit.c 
>>> b/drivers/gpu/drm/via/via_dmablit.c
>>> index 1a3ad76..98aae98 100644
>>> --- a/drivers/gpu/drm/via/via_dmablit.c
>>> +++ b/drivers/gpu/drm/via/via_dmablit.c
>>> @@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
>>> -   ret = get_user_pages((unsigned long)xfer->mem_addr,
>>> -vsg->num_pages,
>>> -(vsg->direction == DMA_FROM_DEVICE) ? 
>>> FOLL_WRITE : 0,
>>> -vsg->pages, NULL);
>>> -
>>> -   up_read(>mm->mmap_sem);
>>> +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
>>> +   vsg->num_pages, vsg->pages,
>>> +   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 
>>> 0);
>>> if (ret != vsg->num_pages) {
>>> if (ret < 0)
>>> return ret;
>>> --
>>> 2.10.2
>>>

-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

On 6 January 2017 at 07:09, Lorenzo Stoakes  wrote:
>
> Adding Andrew, as this may be another less active corner of the corner, 
> thanks!

Hi all,

Thought I'd also give this one a gentle nudge now the merge window has
re-opened, Andrew - are you ok to pick this up? I've checked the patch
and it still applies, for convenience the raw patch is available at
https://marc.info/?l=linux-mm=147802942832515=raw - let me know if
there's anything else I can do on this or if you'd prefer a re-send.

Best, Lorenzo

> On 3 January 2017 at 20:23, Lorenzo Stoakes  wrote:
>> Hi All,
>>
>> Just a gentle ping on this one :)
>>
>> Cheers, Lorenzo
>>
>> On 1 November 2016 at 19:43, Lorenzo Stoakes  wrote:
>>> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the 
>>> code
>>> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>>>
>>> Signed-off-by: Lorenzo Stoakes 
>>> ---
>>>  drivers/gpu/drm/via/via_dmablit.c | 10 +++---
>>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/via/via_dmablit.c 
>>> b/drivers/gpu/drm/via/via_dmablit.c
>>> index 1a3ad76..98aae98 100644
>>> --- a/drivers/gpu/drm/via/via_dmablit.c
>>> +++ b/drivers/gpu/drm/via/via_dmablit.c
>>> @@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
>>> -   ret = get_user_pages((unsigned long)xfer->mem_addr,
>>> -vsg->num_pages,
>>> -(vsg->direction == DMA_FROM_DEVICE) ? 
>>> FOLL_WRITE : 0,
>>> -vsg->pages, NULL);
>>> -
>>> -   up_read(>mm->mmap_sem);
>>> +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
>>> +   vsg->num_pages, vsg->pages,
>>> +   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 
>>> 0);
>>> if (ret != vsg->num_pages) {
>>> if (ret < 0)
>>> return ret;
>>> --
>>> 2.10.2
>>>

-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH] drivers/virt: use get_user_pages_unlocked()

[Lorenzo Stoakes]

On 3 January 2017 at 21:36, Michal Hocko <mho...@kernel.org> wrote:
> On Tue 03-01-17 21:14:20, Lorenzo Stoakes wrote:
>> Just a gentle ping on this :) I think this might be a slightly
>> abandoned corner of the kernel so not sure who else to ping to get
>> this moving.
>
> Maybe Andrew can pick it up?
> http://lkml.kernel.org/r/20161101194332.23961-1-lstoa...@gmail.com
>

Hi all, since the merge window has opened thought I'd give another
gentle nudge on this - Andrew, are you ok to pick this up? For
convenience the raw patch is at
https://marc.info/?l=linux-mm=147802941732512=raw I've checked and
it still applies. Let me know if you want me to simply resend this or
if there is anything else I can do to nudge this along!

Thanks, Lorenzo

>> On 1 November 2016 at 19:43, Lorenzo Stoakes <lstoa...@gmail.com> wrote:
>> > Moving from get_user_pages() to get_user_pages_unlocked() simplifies the 
>> > code
>> > and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>> >
>> > Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
>> > ---
>> >  drivers/virt/fsl_hypervisor.c | 7 ++-
>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
>> > index 150ce2a..d3eca87 100644
>> > --- a/drivers/virt/fsl_hypervisor.c
>> > +++ b/drivers/virt/fsl_hypervisor.c
>> > @@ -243,11 +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(>mm->mmap_sem);
>> > -   num_pinned = get_user_pages(param.local_vaddr - lb_offset,
>> > -   num_pages, (param.source == -1) ? 0 : FOLL_WRITE,
>> > -   pages, NULL);
>> > -   up_read(>mm->mmap_sem);
>> > +   num_pinned = get_user_pages_unlocked(param.local_vaddr - lb_offset,
>> > +       num_pages, pages, (param.source == -1) ? 0 : FOLL_WRITE);
>> >
>> > if (num_pinned != num_pages) {
>> > /* get_user_pages() failed */
>> > --
>> > 2.10.2
>> >
>>
>>
>>
>> --
>> Lorenzo Stoakes
>> https://ljs.io
>
> --
> Michal Hocko
> SUSE Labs



-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH] drivers/virt: use get_user_pages_unlocked()

[Lorenzo Stoakes]

On 3 January 2017 at 21:36, Michal Hocko  wrote:
> On Tue 03-01-17 21:14:20, Lorenzo Stoakes wrote:
>> Just a gentle ping on this :) I think this might be a slightly
>> abandoned corner of the kernel so not sure who else to ping to get
>> this moving.
>
> Maybe Andrew can pick it up?
> http://lkml.kernel.org/r/20161101194332.23961-1-lstoa...@gmail.com
>

Hi all, since the merge window has opened thought I'd give another
gentle nudge on this - Andrew, are you ok to pick this up? For
convenience the raw patch is at
https://marc.info/?l=linux-mm=147802941732512=raw I've checked and
it still applies. Let me know if you want me to simply resend this or
if there is anything else I can do to nudge this along!

Thanks, Lorenzo

>> On 1 November 2016 at 19:43, Lorenzo Stoakes  wrote:
>> > Moving from get_user_pages() to get_user_pages_unlocked() simplifies the 
>> > code
>> > and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>> >
>> > Signed-off-by: Lorenzo Stoakes 
>> > ---
>> >  drivers/virt/fsl_hypervisor.c | 7 ++-
>> >  1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
>> > index 150ce2a..d3eca87 100644
>> > --- a/drivers/virt/fsl_hypervisor.c
>> > +++ b/drivers/virt/fsl_hypervisor.c
>> > @@ -243,11 +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(>mm->mmap_sem);
>> > -   num_pinned = get_user_pages(param.local_vaddr - lb_offset,
>> > -   num_pages, (param.source == -1) ? 0 : FOLL_WRITE,
>> > -   pages, NULL);
>> > -   up_read(>mm->mmap_sem);
>> > +   num_pinned = get_user_pages_unlocked(param.local_vaddr - lb_offset,
>> > +   num_pages, pages, (param.source == -1) ? 0 : FOLL_WRITE);
>> >
>> > if (num_pinned != num_pages) {
>> > /* get_user_pages() failed */
>> > --
>> > 2.10.2
>> >
>>
>>
>>
>> --
>> Lorenzo Stoakes
>> https://ljs.io
>
> --
> Michal Hocko
> SUSE Labs



-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

On 3 January 2017 at 20:23, Lorenzo Stoakes <lstoa...@gmail.com> wrote:
> Hi All,
>
> Just a gentle ping on this one :)
>
> Cheers, Lorenzo
>
> On 1 November 2016 at 19:43, Lorenzo Stoakes <lstoa...@gmail.com> wrote:
>> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
>> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>>
>> Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
>> ---
>>  drivers/gpu/drm/via/via_dmablit.c | 10 +++---
>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/via/via_dmablit.c 
>> b/drivers/gpu/drm/via/via_dmablit.c
>> index 1a3ad76..98aae98 100644
>> --- a/drivers/gpu/drm/via/via_dmablit.c
>> +++ b/drivers/gpu/drm/via/via_dmablit.c
>> @@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
>> -   ret = get_user_pages((unsigned long)xfer->mem_addr,
>> -vsg->num_pages,
>> -(vsg->direction == DMA_FROM_DEVICE) ? 
>> FOLL_WRITE : 0,
>> -vsg->pages, NULL);
>> -
>> -   up_read(>mm->mmap_sem);
>> +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
>> +   vsg->num_pages, vsg->pages,
>> +   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 
>> 0);
>> if (ret != vsg->num_pages) {
>> if (ret < 0)
>> return ret;
>> --
>> 2.10.2
>>

Adding Andrew, as this may be another less active corner of the corner, thanks!

-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

On 3 January 2017 at 20:23, Lorenzo Stoakes  wrote:
> Hi All,
>
> Just a gentle ping on this one :)
>
> Cheers, Lorenzo
>
> On 1 November 2016 at 19:43, Lorenzo Stoakes  wrote:
>> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
>> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>>
>> Signed-off-by: Lorenzo Stoakes 
>> ---
>>  drivers/gpu/drm/via/via_dmablit.c | 10 +++---
>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/via/via_dmablit.c 
>> b/drivers/gpu/drm/via/via_dmablit.c
>> index 1a3ad76..98aae98 100644
>> --- a/drivers/gpu/drm/via/via_dmablit.c
>> +++ b/drivers/gpu/drm/via/via_dmablit.c
>> @@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
>> -   ret = get_user_pages((unsigned long)xfer->mem_addr,
>> -vsg->num_pages,
>> -(vsg->direction == DMA_FROM_DEVICE) ? 
>> FOLL_WRITE : 0,
>> -vsg->pages, NULL);
>> -
>> -   up_read(>mm->mmap_sem);
>> +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
>> +   vsg->num_pages, vsg->pages,
>> +   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 
>> 0);
>> if (ret != vsg->num_pages) {
>> if (ret < 0)
>> return ret;
>> --
>> 2.10.2
>>

Adding Andrew, as this may be another less active corner of the corner, thanks!

-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH] drivers/virt: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Just a gentle ping on this :) I think this might be a slightly
abandoned corner of the kernel so not sure who else to ping to get
this moving.

On 1 November 2016 at 19:43, Lorenzo Stoakes <lstoa...@gmail.com> wrote:
> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>
> Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
> ---
>  drivers/virt/fsl_hypervisor.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
> index 150ce2a..d3eca87 100644
> --- a/drivers/virt/fsl_hypervisor.c
> +++ b/drivers/virt/fsl_hypervisor.c
> @@ -243,11 +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(>mm->mmap_sem);
> -   num_pinned = get_user_pages(param.local_vaddr - lb_offset,
> -   num_pages, (param.source == -1) ? 0 : FOLL_WRITE,
> -   pages, NULL);
> -   up_read(>mm->mmap_sem);
> +   num_pinned = get_user_pages_unlocked(param.local_vaddr - lb_offset,
> +   num_pages, pages, (param.source == -1) ? 0 : FOLL_WRITE);
>
> if (num_pinned != num_pages) {
> /* get_user_pages() failed */
> --
> 2.10.2
>



-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH] drivers/virt: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Just a gentle ping on this :) I think this might be a slightly
abandoned corner of the kernel so not sure who else to ping to get
this moving.

On 1 November 2016 at 19:43, Lorenzo Stoakes  wrote:
> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>
> Signed-off-by: Lorenzo Stoakes 
> ---
>  drivers/virt/fsl_hypervisor.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
> index 150ce2a..d3eca87 100644
> --- a/drivers/virt/fsl_hypervisor.c
> +++ b/drivers/virt/fsl_hypervisor.c
> @@ -243,11 +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(>mm->mmap_sem);
> -   num_pinned = get_user_pages(param.local_vaddr - lb_offset,
> -   num_pages, (param.source == -1) ? 0 : FOLL_WRITE,
> -   pages, NULL);
> -   up_read(>mm->mmap_sem);
> +   num_pinned = get_user_pages_unlocked(param.local_vaddr - lb_offset,
> +   num_pages, pages, (param.source == -1) ? 0 : FOLL_WRITE);
>
> if (num_pinned != num_pages) {
> /* get_user_pages() failed */
> --
> 2.10.2
>



-- 
Lorenzo Stoakes
https://ljs.io

[PATCH RESEND] rapidio: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 drivers/rapidio/devices/rio_mport_cdev.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/rapidio/devices/rio_mport_cdev.c 
b/drivers/rapidio/devices/rio_mport_cdev.c
index 9013a585507e..50b617af81bd 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -889,17 +889,16 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
goto err_req;
}

-   down_read(>mm->mmap_sem);
-   pinned = get_user_pages(
+   pinned = get_user_pages_unlocked(
(unsigned long)xfer->loc_addr & PAGE_MASK,
nr_pages,
-   dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
-   page_list, NULL);
-   up_read(>mm->mmap_sem);
+   page_list,
+   dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0);

if (pinned != nr_pages) {
if (pinned < 0) {
-   rmcd_error("get_user_pages err=%ld", pinned);
+   rmcd_error("get_user_pages_unlocked err=%ld",
+  pinned);
nr_pages = 0;
} else
rmcd_error("pinned %ld out of %ld pages",
--
2.11.0

[PATCH RESEND] rapidio: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes 
---
 drivers/rapidio/devices/rio_mport_cdev.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/rapidio/devices/rio_mport_cdev.c 
b/drivers/rapidio/devices/rio_mport_cdev.c
index 9013a585507e..50b617af81bd 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -889,17 +889,16 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
goto err_req;
}

-   down_read(>mm->mmap_sem);
-   pinned = get_user_pages(
+   pinned = get_user_pages_unlocked(
(unsigned long)xfer->loc_addr & PAGE_MASK,
nr_pages,
-   dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
-   page_list, NULL);
-   up_read(>mm->mmap_sem);
+   page_list,
+   dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0);

if (pinned != nr_pages) {
if (pinned < 0) {
-   rmcd_error("get_user_pages err=%ld", pinned);
+   rmcd_error("get_user_pages_unlocked err=%ld",
+  pinned);
nr_pages = 0;
} else
rmcd_error("pinned %ld out of %ld pages",
--
2.11.0

Re: [PATCH] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Hi All,

Just a gentle ping on this one :)

Cheers, Lorenzo

On 1 November 2016 at 19:43, Lorenzo Stoakes <lstoa...@gmail.com> wrote:
> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>
> Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
> ---
>  drivers/gpu/drm/via/via_dmablit.c | 10 +++---
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/via/via_dmablit.c 
> b/drivers/gpu/drm/via/via_dmablit.c
> index 1a3ad76..98aae98 100644
> --- a/drivers/gpu/drm/via/via_dmablit.c
> +++ b/drivers/gpu/drm/via/via_dmablit.c
> @@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
> -   ret = get_user_pages((unsigned long)xfer->mem_addr,
> -vsg->num_pages,
> -(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE 
> : 0,
> -vsg->pages, NULL);
> -
> -   up_read(>mm->mmap_sem);
> +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
> +   vsg->num_pages, vsg->pages,
> +   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
> if (ret != vsg->num_pages) {
> if (ret < 0)
> return ret;
> --
> 2.10.2
>



-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Hi All,

Just a gentle ping on this one :)

Cheers, Lorenzo

On 1 November 2016 at 19:43, Lorenzo Stoakes  wrote:
> Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
> and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.
>
> Signed-off-by: Lorenzo Stoakes 
> ---
>  drivers/gpu/drm/via/via_dmablit.c | 10 +++---
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/via/via_dmablit.c 
> b/drivers/gpu/drm/via/via_dmablit.c
> index 1a3ad76..98aae98 100644
> --- a/drivers/gpu/drm/via/via_dmablit.c
> +++ b/drivers/gpu/drm/via/via_dmablit.c
> @@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
> -   ret = get_user_pages((unsigned long)xfer->mem_addr,
> -vsg->num_pages,
> -(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE 
> : 0,
> -vsg->pages, NULL);
> -
> -   up_read(>mm->mmap_sem);
> +   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
> +   vsg->num_pages, vsg->pages,
> +   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
>     if (ret != vsg->num_pages) {
> if (ret < 0)
> return ret;
> --
> 2.10.2
>



-- 
Lorenzo Stoakes
https://ljs.io

Re: [PATCH 1/2] mm: add locked parameter to get_user_pages()

[Lorenzo Stoakes]

On Mon, Nov 07, 2016 at 11:49:18AM +0100, Jesper Nilsson wrote:
> For the cris-part:
> Acked-by: Jesper Nilsson 

Thanks very much for that, however just to avoid any confusion, I realised this
series was not not the right way forward after discussion with Paolo and rather
it makes more sense to keep the API as it is and to update callers where it
makes sense to.

Re: [PATCH 1/2] mm: add locked parameter to get_user_pages()

[Lorenzo Stoakes]

On Mon, Nov 07, 2016 at 11:49:18AM +0100, Jesper Nilsson wrote:
> For the cris-part:
> Acked-by: Jesper Nilsson 

Thanks very much for that, however just to avoid any confusion, I realised this
series was not not the right way forward after discussion with Paolo and rather
it makes more sense to keep the API as it is and to update callers where it
makes sense to.

[PATCH] rapidio: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 drivers/rapidio/devices/rio_mport_cdev.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/rapidio/devices/rio_mport_cdev.c 
b/drivers/rapidio/devices/rio_mport_cdev.c
index 9013a58..5fdd081 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -889,13 +889,11 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
goto err_req;
}
 
-   down_read(>mm->mmap_sem);
-   pinned = get_user_pages(
+   pinned = get_user_pages_unlocked(
(unsigned long)xfer->loc_addr & PAGE_MASK,
nr_pages,
-   dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
-   page_list, NULL);
-   up_read(>mm->mmap_sem);
+   page_list,
+   dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0);
 
if (pinned != nr_pages) {
if (pinned < 0) {
-- 
2.10.2

[PATCH] rapidio: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes 
---
 drivers/rapidio/devices/rio_mport_cdev.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/rapidio/devices/rio_mport_cdev.c 
b/drivers/rapidio/devices/rio_mport_cdev.c
index 9013a58..5fdd081 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -889,13 +889,11 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
goto err_req;
}
 
-   down_read(>mm->mmap_sem);
-   pinned = get_user_pages(
+   pinned = get_user_pages_unlocked(
(unsigned long)xfer->loc_addr & PAGE_MASK,
nr_pages,
-   dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
-   page_list, NULL);
-   up_read(>mm->mmap_sem);
+   page_list,
+   dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0);
 
if (pinned != nr_pages) {
if (pinned < 0) {
-- 
2.10.2

[PATCH] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 drivers/gpu/drm/via/via_dmablit.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dmablit.c 
b/drivers/gpu/drm/via/via_dmablit.c
index 1a3ad76..98aae98 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
-   ret = get_user_pages((unsigned long)xfer->mem_addr,
-vsg->num_pages,
-(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 
0,
-vsg->pages, NULL);
-
-   up_read(>mm->mmap_sem);
+   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
+   vsg->num_pages, vsg->pages,
+   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
if (ret != vsg->num_pages) {
if (ret < 0)
return ret;
-- 
2.10.2

[PATCH] drivers/virt: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 drivers/virt/fsl_hypervisor.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
index 150ce2a..d3eca87 100644
--- a/drivers/virt/fsl_hypervisor.c
+++ b/drivers/virt/fsl_hypervisor.c
@@ -243,11 +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(>mm->mmap_sem);
-   num_pinned = get_user_pages(param.local_vaddr - lb_offset,
-   num_pages, (param.source == -1) ? 0 : FOLL_WRITE,
-   pages, NULL);
-   up_read(>mm->mmap_sem);
+   num_pinned = get_user_pages_unlocked(param.local_vaddr - lb_offset,
+   num_pages, pages, (param.source == -1) ? 0 : FOLL_WRITE);
 
if (num_pinned != num_pages) {
/* get_user_pages() failed */
-- 
2.10.2

[PATCH] platform: goldfish: pipe: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 drivers/platform/goldfish/goldfish_pipe.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c 
b/drivers/platform/goldfish/goldfish_pipe.c
index 1aba2c7..2b21033 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -308,10 +308,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, 
char __user *buffer,
 * returns a small amount, then there's no need to pin that
 * much memory to the process.
 */
-   down_read(>mm->mmap_sem);
-   ret = get_user_pages(address, 1, is_write ? 0 : FOLL_WRITE,
-   , NULL);
-   up_read(>mm->mmap_sem);
+   ret = get_user_pages_unlocked(address, 1, ,
+   is_write ? 0 : FOLL_WRITE);
if (ret < 0)
break;
 
-- 
2.10.2

[PATCH] drm/via: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes 
---
 drivers/gpu/drm/via/via_dmablit.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/via/via_dmablit.c 
b/drivers/gpu/drm/via/via_dmablit.c
index 1a3ad76..98aae98 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -238,13 +238,9 @@ 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(>mm->mmap_sem);
-   ret = get_user_pages((unsigned long)xfer->mem_addr,
-vsg->num_pages,
-(vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 
0,
-vsg->pages, NULL);
-
-   up_read(>mm->mmap_sem);
+   ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr,
+   vsg->num_pages, vsg->pages,
+   (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0);
if (ret != vsg->num_pages) {
if (ret < 0)
return ret;
-- 
2.10.2

[PATCH] drivers/virt: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes 
---
 drivers/virt/fsl_hypervisor.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
index 150ce2a..d3eca87 100644
--- a/drivers/virt/fsl_hypervisor.c
+++ b/drivers/virt/fsl_hypervisor.c
@@ -243,11 +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(>mm->mmap_sem);
-   num_pinned = get_user_pages(param.local_vaddr - lb_offset,
-   num_pages, (param.source == -1) ? 0 : FOLL_WRITE,
-   pages, NULL);
-   up_read(>mm->mmap_sem);
+   num_pinned = get_user_pages_unlocked(param.local_vaddr - lb_offset,
+   num_pages, pages, (param.source == -1) ? 0 : FOLL_WRITE);
 
if (num_pinned != num_pages) {
/* get_user_pages() failed */
-- 
2.10.2

[PATCH] platform: goldfish: pipe: use get_user_pages_unlocked()

[Lorenzo Stoakes]

Moving from get_user_pages() to get_user_pages_unlocked() simplifies the code
and takes advantage of VM_FAULT_RETRY functionality when faulting in pages.

Signed-off-by: Lorenzo Stoakes 
---
 drivers/platform/goldfish/goldfish_pipe.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c 
b/drivers/platform/goldfish/goldfish_pipe.c
index 1aba2c7..2b21033 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -308,10 +308,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, 
char __user *buffer,
 * returns a small amount, then there's no need to pin that
 * much memory to the process.
 */
-   down_read(>mm->mmap_sem);
-   ret = get_user_pages(address, 1, is_write ? 0 : FOLL_WRITE,
-   , NULL);
-   up_read(>mm->mmap_sem);
+   ret = get_user_pages_unlocked(address, 1, ,
+   is_write ? 0 : FOLL_WRITE);
if (ret < 0)
break;
 
-- 
2.10.2

Re: [PATCH 2/2] mm: remove get_user_pages_locked()

[Lorenzo Stoakes]

On Mon, Oct 31, 2016 at 06:55:33PM +0100, Paolo Bonzini wrote:
> > 2. There is currently only one caller of get_user_pages_locked() in
> >mm/frame_vector.c which seems to suggest this function isn't widely
> >used/known.
>
> Or not widely necessary. :)

Well, quite :)
>
> > 3. This change results in all slow-path get_user_pages*() functions having 
> > the
> >ability to use VM_FAULT_RETRY logic rather than 'defaulting' to using
> >get_user_pages() that doesn't let you do this even if you wanted to.
>
> This is only true if someone does the work though.  From a quick look
> at your series, the following files can be trivially changed to use
> get_user_pages_unlocked:
>
> - drivers/gpu/drm/via/via_dmablit.c
> - drivers/platform/goldfish/goldfish_pipe.c
> - drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> - drivers/rapidio/devices/rio_mport_cdev.c
> - drivers/virt/fsl_hypervisor.c
>

Ah indeed, I rather glossed through the callers and noticed that at least some
held locks long enough or were called with a lock held sufficient that I wasn't
sure it could be released.

> Also, videobuf_dma_init_user could be changed to use retry by adding a
> *locked argument to videobuf_dma_init_user_locked, prototype patch
> after my signature.
>

Very nice!

> Everything else is probably best kept using get_user_pages.
>
> > 4. It's somewhat confusing/redundant having both get_user_pages_locked() and
> >get_user_pages() functions which both require mmap_sem to be held (i.e. 
> > both
> >are 'locked' versions.)
> >
> >> If all callers were changed, then sure removing the _locked suffix would
> >> be a good idea.
> >
> > It seems many callers cannot release mmap_sem so VM_FAULT_RETRY logic 
> > couldn't
> > happen anyway in this cases and in these cases we couldn't change the 
> > caller.
>
> But then get_user_pages_locked remains a less-common case, so perhaps
> it's a good thing to give it a longer name!

My (somewhat minor) concern was that there would be confusion due to the
existence of the triumvirate of g_u_p()/g_u_p_unlocked()/g_u_p_locked(), however
the comments do a decent enough job of explaining the situation.

>
> > Overall, an alternative here might be to remove get_user_pages() and update
> > get_user_pages_locked() to add a 'vmas' parameter, however doing this 
> > renders
> > get_user_pages_unlocked() asymmetric as it would lack an vmas parameter 
> > (adding
> > one there would make no sense as VM_FAULT_RETRY logic invalidates VMAs) 
> > though
> > perhaps not such a big issue as it makes sense as to why this is the case.
>
> Adding the 'vmas' parameter to get_user_pages_locked would make little
> sense.  Since VM_FAULT_RETRY invalidates it and g_u_p_locked can and
> does retry, it would generally not be useful.

I meant only in the case where we'd remove get_user_pages() and instead be left
with get_user_pages_[un]locked() only, meaning we'd have to add some means of
retrieving vmas if locked was set NULL, of course in cases where locked is not
NULL it makes no sense to add it.

>
> So I think we have the right API now:
>
> - do not have lock?  Use get_user_pages_unlocked, get retry for free,
> no need to handle  mmap_sem and the locked argument; cannot get back vmas.
>
> - have and cannot drop lock?  User get_user_pages, no need to pass NULL
> for the locked argument; can get back vmas.
>
> - have but can drop lock (rare case)?  Use get_user_pages_locked,
> cannot get back vams.

Yeah I think this is sane as it is actually, this patch set was a lot less
convincing of a cleanup than prior ones and overall it seems we are better off
with the existing API.

I wonder whether a better patch series to come out of this would be to find
cases where the lock could be dropped (i.e. the ones you mention above) and to
switch to using get_user_pages_[un]locked() where it makes sense to.

I am happy to look into these cases (though of course I must leave your
suggested patch here to you :)

Re: [PATCH 2/2] mm: remove get_user_pages_locked()

[Lorenzo Stoakes]

On Mon, Oct 31, 2016 at 06:55:33PM +0100, Paolo Bonzini wrote:
> > 2. There is currently only one caller of get_user_pages_locked() in
> >mm/frame_vector.c which seems to suggest this function isn't widely
> >used/known.
>
> Or not widely necessary. :)

Well, quite :)
>
> > 3. This change results in all slow-path get_user_pages*() functions having 
> > the
> >ability to use VM_FAULT_RETRY logic rather than 'defaulting' to using
> >get_user_pages() that doesn't let you do this even if you wanted to.
>
> This is only true if someone does the work though.  From a quick look
> at your series, the following files can be trivially changed to use
> get_user_pages_unlocked:
>
> - drivers/gpu/drm/via/via_dmablit.c
> - drivers/platform/goldfish/goldfish_pipe.c
> - drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> - drivers/rapidio/devices/rio_mport_cdev.c
> - drivers/virt/fsl_hypervisor.c
>

Ah indeed, I rather glossed through the callers and noticed that at least some
held locks long enough or were called with a lock held sufficient that I wasn't
sure it could be released.

> Also, videobuf_dma_init_user could be changed to use retry by adding a
> *locked argument to videobuf_dma_init_user_locked, prototype patch
> after my signature.
>

Very nice!

> Everything else is probably best kept using get_user_pages.
>
> > 4. It's somewhat confusing/redundant having both get_user_pages_locked() and
> >get_user_pages() functions which both require mmap_sem to be held (i.e. 
> > both
> >are 'locked' versions.)
> >
> >> If all callers were changed, then sure removing the _locked suffix would
> >> be a good idea.
> >
> > It seems many callers cannot release mmap_sem so VM_FAULT_RETRY logic 
> > couldn't
> > happen anyway in this cases and in these cases we couldn't change the 
> > caller.
>
> But then get_user_pages_locked remains a less-common case, so perhaps
> it's a good thing to give it a longer name!

My (somewhat minor) concern was that there would be confusion due to the
existence of the triumvirate of g_u_p()/g_u_p_unlocked()/g_u_p_locked(), however
the comments do a decent enough job of explaining the situation.

>
> > Overall, an alternative here might be to remove get_user_pages() and update
> > get_user_pages_locked() to add a 'vmas' parameter, however doing this 
> > renders
> > get_user_pages_unlocked() asymmetric as it would lack an vmas parameter 
> > (adding
> > one there would make no sense as VM_FAULT_RETRY logic invalidates VMAs) 
> > though
> > perhaps not such a big issue as it makes sense as to why this is the case.
>
> Adding the 'vmas' parameter to get_user_pages_locked would make little
> sense.  Since VM_FAULT_RETRY invalidates it and g_u_p_locked can and
> does retry, it would generally not be useful.

I meant only in the case where we'd remove get_user_pages() and instead be left
with get_user_pages_[un]locked() only, meaning we'd have to add some means of
retrieving vmas if locked was set NULL, of course in cases where locked is not
NULL it makes no sense to add it.

>
> So I think we have the right API now:
>
> - do not have lock?  Use get_user_pages_unlocked, get retry for free,
> no need to handle  mmap_sem and the locked argument; cannot get back vmas.
>
> - have and cannot drop lock?  User get_user_pages, no need to pass NULL
> for the locked argument; can get back vmas.
>
> - have but can drop lock (rare case)?  Use get_user_pages_locked,
> cannot get back vams.

Yeah I think this is sane as it is actually, this patch set was a lot less
convincing of a cleanup than prior ones and overall it seems we are better off
with the existing API.

I wonder whether a better patch series to come out of this would be to find
cases where the lock could be dropped (i.e. the ones you mention above) and to
switch to using get_user_pages_[un]locked() where it makes sense to.

I am happy to look into these cases (though of course I must leave your
suggested patch here to you :)

Re: [PATCH 2/2] mm: remove get_user_pages_locked()

[Lorenzo Stoakes]

On Mon, Oct 31, 2016 at 12:45:36PM +0100, Paolo Bonzini wrote:
>
>
> On 31/10/2016 11:02, Lorenzo Stoakes wrote:
> > - *
> > - * get_user_pages should be phased out in favor of
> > - * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
> > - * should use get_user_pages because it cannot pass
> > - * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
>
> This comment should be preserved in some way.  In addition, removing

Could you clarify what you think should be retained?

The comment seems to me to be largely rendered redundant by this change -
get_user_pages() now offers identical behaviour, and of course the latter part
of the comment ('because it cannot pass FAULT_FLAG_ALLOW_RETRY') is rendered
incorrect by this change.

It could be replaced with a recommendation to make use of VM_FAULT_RETRY logic
if possible. Note that the line above retains the reference to
'get_user_pages_fast()' - 'See also get_user_pages_fast, for performance
critical applications.'

One important thing to note here is this is a comment on get_user_pages_remote()
for which it was already incorrect syntactically (I'm guessing once
get_user_pages_remote() was added it 'stole' get_user_pages()'s comment :)

> get_user_pages_locked() makes it harder (compared to a simple "git grep
> -w") to identify callers that lack allow-retry functionality).  So I'm
> not sure about the benefits of these patches.
>

That's a fair point, and certainly this cleanup series is less obviously helpful
than previous ones.

However, there are a few points in its favour:

1. get_user_pages_remote() was the mirror of get_user_pages() prior to adding an
   int *locked parameter to the former (necessary to allow for the unexport of
   __get_user_pages_unlocked()), differing only in task/mm being specified and
   FOLL_REMOTE being set. This patch series keeps these functions 'synchronised'
   in this respect.

2. There is currently only one caller of get_user_pages_locked() in
   mm/frame_vector.c which seems to suggest this function isn't widely
   used/known.

3. This change results in all slow-path get_user_pages*() functions having the
   ability to use VM_FAULT_RETRY logic rather than 'defaulting' to using
   get_user_pages() that doesn't let you do this even if you wanted to.

4. It's somewhat confusing/redundant having both get_user_pages_locked() and
   get_user_pages() functions which both require mmap_sem to be held (i.e. both
   are 'locked' versions.)

> If all callers were changed, then sure removing the _locked suffix would
> be a good idea.

It seems many callers cannot release mmap_sem so VM_FAULT_RETRY logic couldn't
happen anyway in this cases and in these cases we couldn't change the caller.


Overall, an alternative here might be to remove get_user_pages() and update
get_user_pages_locked() to add a 'vmas' parameter, however doing this renders
get_user_pages_unlocked() asymmetric as it would lack an vmas parameter (adding
one there would make no sense as VM_FAULT_RETRY logic invalidates VMAs) though
perhaps not such a big issue as it makes sense as to why this is the case.

get_user_pages_unlocked() definitely seems to be a useful helper and therefore
makes sense to retain.

Of course another alternative is to leave things be :)

Re: [PATCH 2/2] mm: remove get_user_pages_locked()

[Lorenzo Stoakes]

On Mon, Oct 31, 2016 at 12:45:36PM +0100, Paolo Bonzini wrote:
>
>
> On 31/10/2016 11:02, Lorenzo Stoakes wrote:
> > - *
> > - * get_user_pages should be phased out in favor of
> > - * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
> > - * should use get_user_pages because it cannot pass
> > - * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
>
> This comment should be preserved in some way.  In addition, removing

Could you clarify what you think should be retained?

The comment seems to me to be largely rendered redundant by this change -
get_user_pages() now offers identical behaviour, and of course the latter part
of the comment ('because it cannot pass FAULT_FLAG_ALLOW_RETRY') is rendered
incorrect by this change.

It could be replaced with a recommendation to make use of VM_FAULT_RETRY logic
if possible. Note that the line above retains the reference to
'get_user_pages_fast()' - 'See also get_user_pages_fast, for performance
critical applications.'

One important thing to note here is this is a comment on get_user_pages_remote()
for which it was already incorrect syntactically (I'm guessing once
get_user_pages_remote() was added it 'stole' get_user_pages()'s comment :)

> get_user_pages_locked() makes it harder (compared to a simple "git grep
> -w") to identify callers that lack allow-retry functionality).  So I'm
> not sure about the benefits of these patches.
>

That's a fair point, and certainly this cleanup series is less obviously helpful
than previous ones.

However, there are a few points in its favour:

1. get_user_pages_remote() was the mirror of get_user_pages() prior to adding an
   int *locked parameter to the former (necessary to allow for the unexport of
   __get_user_pages_unlocked()), differing only in task/mm being specified and
   FOLL_REMOTE being set. This patch series keeps these functions 'synchronised'
   in this respect.

2. There is currently only one caller of get_user_pages_locked() in
   mm/frame_vector.c which seems to suggest this function isn't widely
   used/known.

3. This change results in all slow-path get_user_pages*() functions having the
   ability to use VM_FAULT_RETRY logic rather than 'defaulting' to using
   get_user_pages() that doesn't let you do this even if you wanted to.

4. It's somewhat confusing/redundant having both get_user_pages_locked() and
   get_user_pages() functions which both require mmap_sem to be held (i.e. both
   are 'locked' versions.)

> If all callers were changed, then sure removing the _locked suffix would
> be a good idea.

It seems many callers cannot release mmap_sem so VM_FAULT_RETRY logic couldn't
happen anyway in this cases and in these cases we couldn't change the caller.


Overall, an alternative here might be to remove get_user_pages() and update
get_user_pages_locked() to add a 'vmas' parameter, however doing this renders
get_user_pages_unlocked() asymmetric as it would lack an vmas parameter (adding
one there would make no sense as VM_FAULT_RETRY logic invalidates VMAs) though
perhaps not such a big issue as it makes sense as to why this is the case.

get_user_pages_unlocked() definitely seems to be a useful helper and therefore
makes sense to retain.

Of course another alternative is to leave things be :)

[PATCH 1/2] mm: add locked parameter to get_user_pages()

[Lorenzo Stoakes]

This patch adds an int *locked parameter to get_user_pages() to allow
VM_FAULT_RETRY faulting behaviour similar to get_user_pages_[un]locked().

It additionally clears the way for get_user_pages_locked() to be removed as its
sole remaining useful characteristic was to allow for VM_FAULT_RETRY behaviour
when faulting in pages.

It should not introduce any functional changes, however it does allow for
subsequent changes to get_user_pages() callers to take advantage of
VM_FAULT_RETRY.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 arch/cris/arch-v32/drivers/cryptocop.c| 2 ++
 arch/ia64/kernel/err_inject.c | 2 +-
 arch/x86/mm/mpx.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c   | 2 +-
 drivers/gpu/drm/via/via_dmablit.c | 2 +-
 drivers/infiniband/core/umem.c| 2 +-
 drivers/infiniband/hw/mthca/mthca_memfree.c   | 3 ++-
 drivers/infiniband/hw/qib/qib_user_pages.c| 2 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c  | 2 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c | 2 +-
 drivers/misc/mic/scif/scif_rma.c  | 1 +
 drivers/misc/sgi-gru/grufault.c   | 3 ++-
 drivers/platform/goldfish/goldfish_pipe.c | 2 +-
 drivers/rapidio/devices/rio_mport_cdev.c  | 2 +-
 .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c| 3 ++-
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
 drivers/virt/fsl_hypervisor.c | 2 +-
 include/linux/mm.h| 2 +-
 mm/gup.c  | 8 
 mm/ksm.c  | 3 ++-
 mm/mempolicy.c| 2 +-
 mm/nommu.c| 4 ++--
 virt/kvm/kvm_main.c   | 4 ++--
 24 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/arch/cris/arch-v32/drivers/cryptocop.c 
b/arch/cris/arch-v32/drivers/cryptocop.c
index 0068fd4..7444b45 100644
--- a/arch/cris/arch-v32/drivers/cryptocop.c
+++ b/arch/cris/arch-v32/drivers/cryptocop.c
@@ -2723,6 +2723,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
 noinpages,
 0,  /* read access only for in data */
 inpages,
+NULL,
 NULL);
 
if (err < 0) {
@@ -2737,6 +2738,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
 nooutpages,
 FOLL_WRITE, /* write access for out data */
 outpages,
+NULL,
 NULL);
up_read(>mm->mmap_sem);
if (err < 0) {
diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
index 5ed0ea9..99f3fa2 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(virt_addr, 1, FOLL_WRITE, NULL, NULL, NULL);
if (ret<=0) {
 #ifdef ERR_INJ_DEBUG
printk("Virtual address %lx is not existing.\n",virt_addr);
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index e4f8009..b74a7b2 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -546,7 +546,7 @@ static int mpx_resolve_fault(long __user *addr, int write)
int nr_pages = 1;
 
gup_ret = get_user_pages((unsigned long)addr, nr_pages,
-   write ? FOLL_WRITE : 0, NULL, NULL);
+   write ? FOLL_WRITE : 0, NULL, NULL, NULL);
/*
 * get_user_pages() returns number of pages gotten.
 * 0 means we failed to fault in and get anything,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index dcaf691..3d9a195 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -584,7 +584,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct 
page **pages)
list_add(, >guptasks);
spin_unlock(>guptasklock);
 
-

[PATCH 1/2] mm: add locked parameter to get_user_pages()

[Lorenzo Stoakes]

This patch adds an int *locked parameter to get_user_pages() to allow
VM_FAULT_RETRY faulting behaviour similar to get_user_pages_[un]locked().

It additionally clears the way for get_user_pages_locked() to be removed as its
sole remaining useful characteristic was to allow for VM_FAULT_RETRY behaviour
when faulting in pages.

It should not introduce any functional changes, however it does allow for
subsequent changes to get_user_pages() callers to take advantage of
VM_FAULT_RETRY.

Signed-off-by: Lorenzo Stoakes 
---
 arch/cris/arch-v32/drivers/cryptocop.c| 2 ++
 arch/ia64/kernel/err_inject.c | 2 +-
 arch/x86/mm/mpx.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c   | 2 +-
 drivers/gpu/drm/via/via_dmablit.c | 2 +-
 drivers/infiniband/core/umem.c| 2 +-
 drivers/infiniband/hw/mthca/mthca_memfree.c   | 3 ++-
 drivers/infiniband/hw/qib/qib_user_pages.c| 2 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c  | 2 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c | 2 +-
 drivers/misc/mic/scif/scif_rma.c  | 1 +
 drivers/misc/sgi-gru/grufault.c   | 3 ++-
 drivers/platform/goldfish/goldfish_pipe.c | 2 +-
 drivers/rapidio/devices/rio_mport_cdev.c  | 2 +-
 .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c| 3 ++-
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
 drivers/virt/fsl_hypervisor.c | 2 +-
 include/linux/mm.h| 2 +-
 mm/gup.c  | 8 
 mm/ksm.c  | 3 ++-
 mm/mempolicy.c| 2 +-
 mm/nommu.c| 4 ++--
 virt/kvm/kvm_main.c   | 4 ++--
 24 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/arch/cris/arch-v32/drivers/cryptocop.c 
b/arch/cris/arch-v32/drivers/cryptocop.c
index 0068fd4..7444b45 100644
--- a/arch/cris/arch-v32/drivers/cryptocop.c
+++ b/arch/cris/arch-v32/drivers/cryptocop.c
@@ -2723,6 +2723,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
 noinpages,
 0,  /* read access only for in data */
 inpages,
+NULL,
 NULL);
 
if (err < 0) {
@@ -2737,6 +2738,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
 nooutpages,
 FOLL_WRITE, /* write access for out data */
 outpages,
+NULL,
 NULL);
up_read(>mm->mmap_sem);
if (err < 0) {
diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
index 5ed0ea9..99f3fa2 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(virt_addr, 1, FOLL_WRITE, NULL, NULL, NULL);
if (ret<=0) {
 #ifdef ERR_INJ_DEBUG
printk("Virtual address %lx is not existing.\n",virt_addr);
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index e4f8009..b74a7b2 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -546,7 +546,7 @@ static int mpx_resolve_fault(long __user *addr, int write)
int nr_pages = 1;
 
gup_ret = get_user_pages((unsigned long)addr, nr_pages,
-   write ? FOLL_WRITE : 0, NULL, NULL);
+   write ? FOLL_WRITE : 0, NULL, NULL, NULL);
/*
 * get_user_pages() returns number of pages gotten.
 * 0 means we failed to fault in and get anything,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index dcaf691..3d9a195 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -584,7 +584,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct 
page **pages)
list_add(, >guptasks);
spin_unlock(>guptasklock);
 
-   r = get_user_pages(use

[PATCH 2/2] mm: remove get_user_pages_locked()

[Lorenzo Stoakes]

get_user_pages() now has an int *locked parameter which renders
get_user_pages_locked() redundant, so remove it.

This patch should not introduce any functional changes.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 include/linux/mm.h |  2 --
 mm/frame_vector.c  |  4 ++--
 mm/gup.c   | 56 +++---
 mm/nommu.c |  8 
 4 files changed, 22 insertions(+), 48 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 396984e..4db3147 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1278,8 +1278,6 @@ long get_user_pages_remote(struct task_struct *tsk, 
struct mm_struct *mm,
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas, int *locked);
-long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-   unsigned int gup_flags, 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,
   struct page **pages, unsigned int gup_flags);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index db77dcb..c5ce1b1 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -55,8 +55,8 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
vec->got_ref = true;
vec->is_pfns = false;
-   ret = get_user_pages_locked(start, nr_frames,
-   gup_flags, (struct page **)(vec->ptrs), );
+   ret = get_user_pages(start, nr_frames,
+   gup_flags, (struct page **)(vec->ptrs), NULL, );
goto out;
}
 
diff --git a/mm/gup.c b/mm/gup.c
index 6b5539e..6cec693 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -826,37 +826,6 @@ static __always_inline long __get_user_pages_locked(struct 
task_struct *tsk,
 }
 
 /*
- * We can leverage the VM_FAULT_RETRY functionality in the page fault
- * paths better by using either get_user_pages_locked() or
- * get_user_pages_unlocked().
- *
- * get_user_pages_locked() is suitable to replace the form:
- *
- *  down_read(>mmap_sem);
- *  do_something()
- *  get_user_pages(tsk, mm, ..., pages, NULL, NULL);
- *  up_read(>mmap_sem);
- *
- *  to:
- *
- *  int locked = 1;
- *  down_read(>mmap_sem);
- *  do_something()
- *  get_user_pages_locked(tsk, mm, ..., pages, );
- *  if (locked)
- *  up_read(>mmap_sem);
- */
-long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-  unsigned int gup_flags, struct page **pages,
-  int *locked)
-{
-   return __get_user_pages_locked(current, current->mm, start, nr_pages,
-  pages, NULL, locked, true,
-  gup_flags | FOLL_TOUCH);
-}
-EXPORT_SYMBOL(get_user_pages_locked);
-
-/*
  * Same as get_user_pages_unlocked(, FOLL_TOUCH) but it allows to
  * pass additional gup_flags as last parameter (like FOLL_HWPOISON).
  *
@@ -954,11 +923,6 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
  * use the correct cache flushing APIs.
  *
  * See also get_user_pages_fast, for performance critical applications.
- *
- * get_user_pages should be phased out in favor of
- * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
- * should use get_user_pages because it cannot pass
- * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
  */
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
@@ -976,6 +940,26 @@ EXPORT_SYMBOL(get_user_pages_remote);
  * less-flexible calling convention where we assume that the task
  * and mm being operated on are the current task's.  We also
  * obviously don't pass FOLL_REMOTE in here.
+ *
+ * We can leverage the VM_FAULT_RETRY functionality in the page fault
+ * paths better by using either get_user_pages()'s locked parameter or
+ * get_user_pages_unlocked().
+ *
+ * get_user_pages()'s locked parameter is suitable to replace the form:
+ *
+ *  down_read(>mmap_sem);
+ *  do_something()
+ *  get_user_pages(tsk, mm, ..., pages, NULL, NULL);
+ *  up_read(>mmap_sem);
+ *
+ *  to:
+ *
+ *  int locked = 1;
+ *  down_read(>mmap_sem);
+ *  do_something()
+ *  get_user_pages(tsk, mm, ..., pages, NULL, );
+ *  if (locked)
+ *  up_read(>mmap_sem);
  */
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
diff --git a/mm/nommu.c b/mm/nommu.c
index 82aaa33..3d38d40 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -168,14 +168,6 @@ long get_use

[PATCH 2/2] mm: remove get_user_pages_locked()

[Lorenzo Stoakes]

get_user_pages() now has an int *locked parameter which renders
get_user_pages_locked() redundant, so remove it.

This patch should not introduce any functional changes.

Signed-off-by: Lorenzo Stoakes 
---
 include/linux/mm.h |  2 --
 mm/frame_vector.c  |  4 ++--
 mm/gup.c   | 56 +++---
 mm/nommu.c |  8 
 4 files changed, 22 insertions(+), 48 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 396984e..4db3147 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1278,8 +1278,6 @@ long get_user_pages_remote(struct task_struct *tsk, 
struct mm_struct *mm,
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas, int *locked);
-long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-   unsigned int gup_flags, 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,
   struct page **pages, unsigned int gup_flags);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index db77dcb..c5ce1b1 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -55,8 +55,8 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
vec->got_ref = true;
vec->is_pfns = false;
-   ret = get_user_pages_locked(start, nr_frames,
-   gup_flags, (struct page **)(vec->ptrs), );
+   ret = get_user_pages(start, nr_frames,
+   gup_flags, (struct page **)(vec->ptrs), NULL, );
goto out;
}
 
diff --git a/mm/gup.c b/mm/gup.c
index 6b5539e..6cec693 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -826,37 +826,6 @@ static __always_inline long __get_user_pages_locked(struct 
task_struct *tsk,
 }
 
 /*
- * We can leverage the VM_FAULT_RETRY functionality in the page fault
- * paths better by using either get_user_pages_locked() or
- * get_user_pages_unlocked().
- *
- * get_user_pages_locked() is suitable to replace the form:
- *
- *  down_read(>mmap_sem);
- *  do_something()
- *  get_user_pages(tsk, mm, ..., pages, NULL, NULL);
- *  up_read(>mmap_sem);
- *
- *  to:
- *
- *  int locked = 1;
- *  down_read(>mmap_sem);
- *  do_something()
- *  get_user_pages_locked(tsk, mm, ..., pages, );
- *  if (locked)
- *  up_read(>mmap_sem);
- */
-long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-  unsigned int gup_flags, struct page **pages,
-  int *locked)
-{
-   return __get_user_pages_locked(current, current->mm, start, nr_pages,
-  pages, NULL, locked, true,
-  gup_flags | FOLL_TOUCH);
-}
-EXPORT_SYMBOL(get_user_pages_locked);
-
-/*
  * Same as get_user_pages_unlocked(, FOLL_TOUCH) but it allows to
  * pass additional gup_flags as last parameter (like FOLL_HWPOISON).
  *
@@ -954,11 +923,6 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
  * use the correct cache flushing APIs.
  *
  * See also get_user_pages_fast, for performance critical applications.
- *
- * get_user_pages should be phased out in favor of
- * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
- * should use get_user_pages because it cannot pass
- * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
  */
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
@@ -976,6 +940,26 @@ EXPORT_SYMBOL(get_user_pages_remote);
  * less-flexible calling convention where we assume that the task
  * and mm being operated on are the current task's.  We also
  * obviously don't pass FOLL_REMOTE in here.
+ *
+ * We can leverage the VM_FAULT_RETRY functionality in the page fault
+ * paths better by using either get_user_pages()'s locked parameter or
+ * get_user_pages_unlocked().
+ *
+ * get_user_pages()'s locked parameter is suitable to replace the form:
+ *
+ *  down_read(>mmap_sem);
+ *  do_something()
+ *  get_user_pages(tsk, mm, ..., pages, NULL, NULL);
+ *  up_read(>mmap_sem);
+ *
+ *  to:
+ *
+ *  int locked = 1;
+ *  down_read(>mmap_sem);
+ *  do_something()
+ *  get_user_pages(tsk, mm, ..., pages, NULL, );
+ *  if (locked)
+ *  up_read(>mmap_sem);
  */
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
diff --git a/mm/nommu.c b/mm/nommu.c
index 82aaa33..3d38d40 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -168,14 +168,6 @@ long get_user_pages(unsigned long sta

[PATCH 0/2] mm: remove get_user_pages_locked()

[Lorenzo Stoakes]

by adding an int *locked parameter to get_user_pages() callers to this function
can now utilise VM_FAULT_RETRY functionality.

Taken in conjunction with the patch series adding the same parameter to
get_user_pages_remote() this means all slow-path get_user_pages*() functions
will now have the ability to utilise VM_FAULT_RETRY.

Additionally get_user_pages() and get_user_pages_remote() previously mirrored
one another in functionality differing only in the ability to specify task/mm,
this patch series reinstates this relationship.

This patch series should not introduce any functional changes.

Lorenzo Stoakes (2):
  mm: add locked parameter to get_user_pages()
  mm: remove get_user_pages_locked()

 arch/cris/arch-v32/drivers/cryptocop.c |  2 +
 arch/ia64/kernel/err_inject.c  |  2 +-
 arch/x86/mm/mpx.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c|  2 +-
 drivers/gpu/drm/via/via_dmablit.c  |  2 +-
 drivers/infiniband/core/umem.c |  2 +-
 drivers/infiniband/hw/mthca/mthca_memfree.c|  3 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |  2 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |  2 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c  |  2 +-
 drivers/misc/mic/scif/scif_rma.c   |  1 +
 drivers/misc/sgi-gru/grufault.c|  3 +-
 drivers/platform/goldfish/goldfish_pipe.c  |  2 +-
 drivers/rapidio/devices/rio_mport_cdev.c   |  2 +-
 .../interface/vchiq_arm/vchiq_2835_arm.c   |  3 +-
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |  3 +-
 drivers/virt/fsl_hypervisor.c  |  2 +-
 include/linux/mm.h |  4 +-
 mm/frame_vector.c  |  4 +-
 mm/gup.c   | 62 --
 mm/ksm.c   |  3 +-
 mm/mempolicy.c |  2 +-
 mm/nommu.c | 10 +---
 virt/kvm/kvm_main.c|  4 +-
 25 files changed, 55 insertions(+), 73 deletions(-)

[PATCH 0/2] mm: remove get_user_pages_locked()

[Lorenzo Stoakes]

by adding an int *locked parameter to get_user_pages() callers to this function
can now utilise VM_FAULT_RETRY functionality.

Taken in conjunction with the patch series adding the same parameter to
get_user_pages_remote() this means all slow-path get_user_pages*() functions
will now have the ability to utilise VM_FAULT_RETRY.

Additionally get_user_pages() and get_user_pages_remote() previously mirrored
one another in functionality differing only in the ability to specify task/mm,
this patch series reinstates this relationship.

This patch series should not introduce any functional changes.

Lorenzo Stoakes (2):
  mm: add locked parameter to get_user_pages()
  mm: remove get_user_pages_locked()

 arch/cris/arch-v32/drivers/cryptocop.c |  2 +
 arch/ia64/kernel/err_inject.c  |  2 +-
 arch/x86/mm/mpx.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c|  2 +-
 drivers/gpu/drm/via/via_dmablit.c  |  2 +-
 drivers/infiniband/core/umem.c |  2 +-
 drivers/infiniband/hw/mthca/mthca_memfree.c|  3 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |  2 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |  2 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c  |  2 +-
 drivers/misc/mic/scif/scif_rma.c   |  1 +
 drivers/misc/sgi-gru/grufault.c|  3 +-
 drivers/platform/goldfish/goldfish_pipe.c  |  2 +-
 drivers/rapidio/devices/rio_mport_cdev.c   |  2 +-
 .../interface/vchiq_arm/vchiq_2835_arm.c   |  3 +-
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |  3 +-
 drivers/virt/fsl_hypervisor.c  |  2 +-
 include/linux/mm.h |  4 +-
 mm/frame_vector.c  |  4 +-
 mm/gup.c   | 62 --
 mm/ksm.c   |  3 +-
 mm/mempolicy.c |  2 +-
 mm/nommu.c | 10 +---
 virt/kvm/kvm_main.c|  4 +-
 25 files changed, 55 insertions(+), 73 deletions(-)

[PATCH v2 2/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch unexports the low-level __get_user_pages_unlocked() function and
replaces invocations with calls to more appropriate higher-level functions.

In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
get_user_pages_unlocked() since we can now pass gup_flags.

In async_pf_execute() and process_vm_rw_single_vec() we need to pass different
tsk, mm arguments so get_user_pages_remote() is the sane replacement in these
cases (having added manual acquisition and release of mmap_sem.)

Additionally get_user_pages_remote() reintroduces use of the FOLL_TOUCH
flag. However, this flag was originally silently dropped by 1e9877902dc7e
("mm/gup: Introduce get_user_pages_remote()"), so this appears to have been
unintentional and reintroducing it is therefore not an issue.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
v2: updated patch to apply against mainline rather than -mmots

 include/linux/mm.h |  3 ---
 mm/gup.c   |  8 
 mm/nommu.c |  7 +++
 mm/process_vm_access.c | 12 
 virt/kvm/async_pf.c| 10 +++---
 virt/kvm/kvm_main.c|  5 ++---
 6 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index cc15445..7b2d14e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1280,9 +1280,6 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
struct vm_area_struct **vmas);
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, 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,
-  struct page **pages, unsigned int gup_flags);
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
 int get_user_pages_fast(unsigned long start, int nr_pages, int write,
diff --git a/mm/gup.c b/mm/gup.c
index 0567851..8028af1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -866,9 +866,10 @@ EXPORT_SYMBOL(get_user_pages_locked);
  * according to the parameters "pages", "write", "force"
  * respectively.
  */
-__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)
+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;
@@ -880,7 +881,6 @@ __always_inline long __get_user_pages_unlocked(struct 
task_struct *tsk, struct m
up_read(>mmap_sem);
return ret;
 }
-EXPORT_SYMBOL(__get_user_pages_unlocked);

 /*
  * get_user_pages_unlocked() is suitable to replace the form:
diff --git a/mm/nommu.c b/mm/nommu.c
index 8b8faaf..669437b 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -176,9 +176,9 @@ long get_user_pages_locked(unsigned long start, unsigned 
long nr_pages,
 }
 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,
-  struct page **pages, unsigned int gup_flags)
+static 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;
down_read(>mmap_sem);
@@ -187,7 +187,6 @@ long __get_user_pages_unlocked(struct task_struct *tsk, 
struct mm_struct *mm,
up_read(>mmap_sem);
return ret;
 }
-EXPORT_SYMBOL(__get_user_pages_unlocked);

 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 struct page **pages, unsigned int gup_flags)
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index be8dc8d..84d0c7e 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -88,7 +88,7 @@ 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;
+   unsigned int flags = 0;

/* Work out address and page range required */
if (len == 0)
@@ -100,15 +100,19 @@ static int process_vm_rw_single_vec(unsigned long addr,

while (!rc && nr_pages && iov_iter_count(iter)) {
int pa

[PATCH v2 2/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch unexports the low-level __get_user_pages_unlocked() function and
replaces invocations with calls to more appropriate higher-level functions.

In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
get_user_pages_unlocked() since we can now pass gup_flags.

In async_pf_execute() and process_vm_rw_single_vec() we need to pass different
tsk, mm arguments so get_user_pages_remote() is the sane replacement in these
cases (having added manual acquisition and release of mmap_sem.)

Additionally get_user_pages_remote() reintroduces use of the FOLL_TOUCH
flag. However, this flag was originally silently dropped by 1e9877902dc7e
("mm/gup: Introduce get_user_pages_remote()"), so this appears to have been
unintentional and reintroducing it is therefore not an issue.

Signed-off-by: Lorenzo Stoakes 
---
v2: updated patch to apply against mainline rather than -mmots

 include/linux/mm.h |  3 ---
 mm/gup.c   |  8 
 mm/nommu.c |  7 +++
 mm/process_vm_access.c | 12 
 virt/kvm/async_pf.c| 10 +++---
 virt/kvm/kvm_main.c|  5 ++---
 6 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index cc15445..7b2d14e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1280,9 +1280,6 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
struct vm_area_struct **vmas);
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, 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,
-  struct page **pages, unsigned int gup_flags);
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
 int get_user_pages_fast(unsigned long start, int nr_pages, int write,
diff --git a/mm/gup.c b/mm/gup.c
index 0567851..8028af1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -866,9 +866,10 @@ EXPORT_SYMBOL(get_user_pages_locked);
  * according to the parameters "pages", "write", "force"
  * respectively.
  */
-__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)
+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;
@@ -880,7 +881,6 @@ __always_inline long __get_user_pages_unlocked(struct 
task_struct *tsk, struct m
up_read(>mmap_sem);
return ret;
 }
-EXPORT_SYMBOL(__get_user_pages_unlocked);

 /*
  * get_user_pages_unlocked() is suitable to replace the form:
diff --git a/mm/nommu.c b/mm/nommu.c
index 8b8faaf..669437b 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -176,9 +176,9 @@ long get_user_pages_locked(unsigned long start, unsigned 
long nr_pages,
 }
 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,
-  struct page **pages, unsigned int gup_flags)
+static 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;
down_read(>mmap_sem);
@@ -187,7 +187,6 @@ long __get_user_pages_unlocked(struct task_struct *tsk, 
struct mm_struct *mm,
up_read(>mmap_sem);
return ret;
 }
-EXPORT_SYMBOL(__get_user_pages_unlocked);

 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 struct page **pages, unsigned int gup_flags)
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index be8dc8d..84d0c7e 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -88,7 +88,7 @@ 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;
+   unsigned int flags = 0;

/* Work out address and page range required */
if (len == 0)
@@ -100,15 +100,19 @@ static int process_vm_rw_single_vec(unsigned long addr,

while (!rc && nr_pages && iov_iter_count(iter)) {
int pages = 

[PATCH v2 0/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch series continues the cleanup of get_user_pages*() functions taking
advantage of the fact we can now pass gup_flags as we please.

It firstly adds an additional 'locked' parameter to get_user_pages_remote() to
allow for its callers to utilise VM_FAULT_RETRY functionality. This is necessary
as the invocation of __get_user_pages_unlocked() in process_vm_rw_single_vec()
makes use of this and no other existing higher level function would allow it to
do so.

Secondly existing callers of __get_user_pages_unlocked() are replaced with the
appropriate higher-level replacement - get_user_pages_unlocked() if the current
task and memory descriptor are referenced, or get_user_pages_remote() if other
task/memory descriptors are referenced (having acquiring mmap_sem.)

Lorenzo Stoakes (2):
  mm: add locked parameter to get_user_pages_remote()
  mm: unexport __get_user_pages_unlocked()

 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/infiniband/core/umem_odp.c  |  2 +-
 fs/exec.c   |  2 +-
 include/linux/mm.h  |  5 +
 kernel/events/uprobes.c |  4 ++--
 mm/gup.c| 20 
 mm/memory.c |  2 +-
 mm/nommu.c  |  7 +++
 mm/process_vm_access.c  | 12 
 security/tomoyo/domain.c|  2 +-
 virt/kvm/async_pf.c | 10 +++---
 virt/kvm/kvm_main.c |  5 ++---
 13 files changed, 41 insertions(+), 34 deletions(-)

[PATCH v2 0/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch series continues the cleanup of get_user_pages*() functions taking
advantage of the fact we can now pass gup_flags as we please.

It firstly adds an additional 'locked' parameter to get_user_pages_remote() to
allow for its callers to utilise VM_FAULT_RETRY functionality. This is necessary
as the invocation of __get_user_pages_unlocked() in process_vm_rw_single_vec()
makes use of this and no other existing higher level function would allow it to
do so.

Secondly existing callers of __get_user_pages_unlocked() are replaced with the
appropriate higher-level replacement - get_user_pages_unlocked() if the current
task and memory descriptor are referenced, or get_user_pages_remote() if other
task/memory descriptors are referenced (having acquiring mmap_sem.)

Lorenzo Stoakes (2):
  mm: add locked parameter to get_user_pages_remote()
  mm: unexport __get_user_pages_unlocked()

 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/infiniband/core/umem_odp.c  |  2 +-
 fs/exec.c   |  2 +-
 include/linux/mm.h  |  5 +
 kernel/events/uprobes.c |  4 ++--
 mm/gup.c| 20 
 mm/memory.c |  2 +-
 mm/nommu.c  |  7 +++
 mm/process_vm_access.c  | 12 
 security/tomoyo/domain.c|  2 +-
 virt/kvm/async_pf.c | 10 +++---
 virt/kvm/kvm_main.c |  5 ++---
 13 files changed, 41 insertions(+), 34 deletions(-)

[PATCH v2 1/2] mm: add locked parameter to get_user_pages_remote()

[Lorenzo Stoakes]

This patch adds an int *locked parameter to get_user_pages_remote() to allow
VM_FAULT_RETRY faulting behaviour similar to get_user_pages_[un]locked().

It additionally clears the way for __get_user_pages_unlocked() to be unexported
as its sole remaining useful characteristic was to allow for VM_FAULT_RETRY
behaviour when faulting in pages.

It should not introduce any functional changes, however it does allow for
subsequent changes to get_user_pages_remote() callers to take advantage of
VM_FAULT_RETRY.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
v2: updated description

 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/infiniband/core/umem_odp.c  |  2 +-
 fs/exec.c   |  2 +-
 include/linux/mm.h  |  2 +-
 kernel/events/uprobes.c |  4 ++--
 mm/gup.c| 12 
 mm/memory.c |  2 +-
 security/tomoyo/domain.c|  2 +-
 9 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 0370b84..0c69a97f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -763,7 +763,7 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
down_read(>mmap_sem);
while (pinned < npages) {
ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
-   flags, pvec + pinned, NULL);
+   flags, pvec + pinned, NULL, NULL);
if (ret < 0)
break;

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index c6f780f..836b525 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -522,7 +522,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
 obj->userptr.ptr + pinned * PAGE_SIZE,
 npages - pinned,
 flags,
-pvec + pinned, NULL);
+pvec + pinned, NULL, NULL);
if (ret < 0)
break;

diff --git a/drivers/infiniband/core/umem_odp.c 
b/drivers/infiniband/core/umem_odp.c
index 1f0fe32..6b079a3 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -578,7 +578,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
user_virt, u64 bcnt,
 */
npages = get_user_pages_remote(owning_process, owning_mm,
user_virt, gup_num_pages,
-   flags, local_page_list, NULL);
+   flags, local_page_list, NULL, NULL);
up_read(_mm->mmap_sem);

if (npages < 0)
diff --git a/fs/exec.c b/fs/exec.c
index 4e497b9..2cf049d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -209,7 +209,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, 
unsigned long pos,
 * doing the exec and bprm->mm is the new process's mm.
 */
ret = get_user_pages_remote(current, bprm->mm, pos, 1, gup_flags,
-   , NULL);
+   , NULL, NULL);
if (ret <= 0)
return NULL;

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a92c8d7..cc15445 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1274,7 +1274,7 @@ extern int access_remote_vm(struct mm_struct *mm, 
unsigned long addr,
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
-   struct vm_area_struct **vmas);
+   struct vm_area_struct **vmas, int *locked);
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index f9ec9ad..215871b 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -301,7 +301,7 @@ int uprobe_write_opcode(struct mm_struct *mm, unsigned long 
vaddr,
 retry:
/* Read the page with vaddr into memory */
ret = get_user_pages_remote(NULL, mm, vaddr, 1, FOLL_FORCE, _page,
-   );
+   , NULL);
if (ret <= 0)
return ret;

@@ -1712,7 +1712,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned 
long vaddr)
 * es

[PATCH v2 1/2] mm: add locked parameter to get_user_pages_remote()

[Lorenzo Stoakes]

This patch adds an int *locked parameter to get_user_pages_remote() to allow
VM_FAULT_RETRY faulting behaviour similar to get_user_pages_[un]locked().

It additionally clears the way for __get_user_pages_unlocked() to be unexported
as its sole remaining useful characteristic was to allow for VM_FAULT_RETRY
behaviour when faulting in pages.

It should not introduce any functional changes, however it does allow for
subsequent changes to get_user_pages_remote() callers to take advantage of
VM_FAULT_RETRY.

Signed-off-by: Lorenzo Stoakes 
---
v2: updated description

 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/infiniband/core/umem_odp.c  |  2 +-
 fs/exec.c   |  2 +-
 include/linux/mm.h  |  2 +-
 kernel/events/uprobes.c |  4 ++--
 mm/gup.c| 12 
 mm/memory.c |  2 +-
 security/tomoyo/domain.c|  2 +-
 9 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 0370b84..0c69a97f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -763,7 +763,7 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
down_read(>mmap_sem);
while (pinned < npages) {
ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
-   flags, pvec + pinned, NULL);
+   flags, pvec + pinned, NULL, NULL);
if (ret < 0)
break;

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index c6f780f..836b525 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -522,7 +522,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
 obj->userptr.ptr + pinned * PAGE_SIZE,
 npages - pinned,
 flags,
-pvec + pinned, NULL);
+pvec + pinned, NULL, NULL);
if (ret < 0)
break;

diff --git a/drivers/infiniband/core/umem_odp.c 
b/drivers/infiniband/core/umem_odp.c
index 1f0fe32..6b079a3 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -578,7 +578,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
user_virt, u64 bcnt,
 */
npages = get_user_pages_remote(owning_process, owning_mm,
user_virt, gup_num_pages,
-   flags, local_page_list, NULL);
+   flags, local_page_list, NULL, NULL);
up_read(_mm->mmap_sem);

if (npages < 0)
diff --git a/fs/exec.c b/fs/exec.c
index 4e497b9..2cf049d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -209,7 +209,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, 
unsigned long pos,
 * doing the exec and bprm->mm is the new process's mm.
 */
ret = get_user_pages_remote(current, bprm->mm, pos, 1, gup_flags,
-   , NULL);
+   , NULL, NULL);
if (ret <= 0)
return NULL;

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a92c8d7..cc15445 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1274,7 +1274,7 @@ extern int access_remote_vm(struct mm_struct *mm, 
unsigned long addr,
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
-   struct vm_area_struct **vmas);
+   struct vm_area_struct **vmas, int *locked);
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index f9ec9ad..215871b 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -301,7 +301,7 @@ int uprobe_write_opcode(struct mm_struct *mm, unsigned long 
vaddr,
 retry:
/* Read the page with vaddr into memory */
ret = get_user_pages_remote(NULL, mm, vaddr, 1, FOLL_FORCE, _page,
-   );
+   , NULL);
if (ret <= 0)
return ret;

@@ -1712,7 +1712,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned 
long vaddr)
 * essentially a kernel access to the m

[PATCH 2/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch unexports the low-level __get_user_pages_unlocked() function and
replaces invocations with calls to more appropriate higher-level functions.

In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
get_user_pages_unlocked() since we can now pass gup_flags.

In async_pf_execute() and process_vm_rw_single_vec() we need to pass different
tsk, mm arguments so get_user_pages_remote() is the sane replacement in these
cases (having added manual acquisition and release of mmap_sem.)

Additionally get_user_pages_remote() reintroduces use of the FOLL_TOUCH
flag. However, this flag was originally silently dropped by 1e9877902dc7e
("mm/gup: Introduce get_user_pages_remote()"), so this appears to have been
unintentional and reintroducing it is therefore not an issue.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 include/linux/mm.h |  3 ---
 mm/gup.c   |  8 
 mm/nommu.c |  7 +++
 mm/process_vm_access.c | 12 
 virt/kvm/async_pf.c| 10 +++---
 virt/kvm/kvm_main.c|  5 ++---
 6 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index cc15445..7b2d14e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1280,9 +1280,6 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
struct vm_area_struct **vmas);
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, 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,
-  struct page **pages, unsigned int gup_flags);
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
 int get_user_pages_fast(unsigned long start, int nr_pages, int write,
diff --git a/mm/gup.c b/mm/gup.c
index 0567851..8028af1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -866,9 +866,10 @@ EXPORT_SYMBOL(get_user_pages_locked);
  * caller if required (just like with __get_user_pages). "FOLL_GET"
  * is set implicitly if "pages" is non-NULL.
  */
-__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)
+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;
@@ -880,7 +881,6 @@ __always_inline long __get_user_pages_unlocked(struct 
task_struct *tsk, struct m
up_read(>mmap_sem);
return ret;
 }
-EXPORT_SYMBOL(__get_user_pages_unlocked);

 /*
  * get_user_pages_unlocked() is suitable to replace the form:
diff --git a/mm/nommu.c b/mm/nommu.c
index 8b8faaf..669437b 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -176,9 +176,9 @@ long get_user_pages_locked(unsigned long start, unsigned 
long nr_pages,
 }
 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,
-  struct page **pages, unsigned int gup_flags)
+static 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;
down_read(>mmap_sem);
@@ -187,7 +187,6 @@ long __get_user_pages_unlocked(struct task_struct *tsk, 
struct mm_struct *mm,
up_read(>mmap_sem);
return ret;
 }
-EXPORT_SYMBOL(__get_user_pages_unlocked);

 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 struct page **pages, unsigned int gup_flags)
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index be8dc8d..84d0c7e 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -88,7 +88,7 @@ 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;
+   unsigned int flags = 0;

/* Work out address and page range required */
if (len == 0)
@@ -100,15 +100,19 @@ static int process_vm_rw_single_vec(unsigned long addr,

while (!rc && nr_pages && iov_iter_count(iter)) {
int pages = min(nr_pages, max_pages_per_

[PATCH 2/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch unexports the low-level __get_user_pages_unlocked() function and
replaces invocations with calls to more appropriate higher-level functions.

In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
get_user_pages_unlocked() since we can now pass gup_flags.

In async_pf_execute() and process_vm_rw_single_vec() we need to pass different
tsk, mm arguments so get_user_pages_remote() is the sane replacement in these
cases (having added manual acquisition and release of mmap_sem.)

Additionally get_user_pages_remote() reintroduces use of the FOLL_TOUCH
flag. However, this flag was originally silently dropped by 1e9877902dc7e
("mm/gup: Introduce get_user_pages_remote()"), so this appears to have been
unintentional and reintroducing it is therefore not an issue.

Signed-off-by: Lorenzo Stoakes 
---
 include/linux/mm.h |  3 ---
 mm/gup.c   |  8 
 mm/nommu.c |  7 +++
 mm/process_vm_access.c | 12 
 virt/kvm/async_pf.c| 10 +++---
 virt/kvm/kvm_main.c|  5 ++---
 6 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index cc15445..7b2d14e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1280,9 +1280,6 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
struct vm_area_struct **vmas);
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, 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,
-  struct page **pages, unsigned int gup_flags);
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
 int get_user_pages_fast(unsigned long start, int nr_pages, int write,
diff --git a/mm/gup.c b/mm/gup.c
index 0567851..8028af1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -866,9 +866,10 @@ EXPORT_SYMBOL(get_user_pages_locked);
  * caller if required (just like with __get_user_pages). "FOLL_GET"
  * is set implicitly if "pages" is non-NULL.
  */
-__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)
+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;
@@ -880,7 +881,6 @@ __always_inline long __get_user_pages_unlocked(struct 
task_struct *tsk, struct m
up_read(>mmap_sem);
return ret;
 }
-EXPORT_SYMBOL(__get_user_pages_unlocked);

 /*
  * get_user_pages_unlocked() is suitable to replace the form:
diff --git a/mm/nommu.c b/mm/nommu.c
index 8b8faaf..669437b 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -176,9 +176,9 @@ long get_user_pages_locked(unsigned long start, unsigned 
long nr_pages,
 }
 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,
-  struct page **pages, unsigned int gup_flags)
+static 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;
down_read(>mmap_sem);
@@ -187,7 +187,6 @@ long __get_user_pages_unlocked(struct task_struct *tsk, 
struct mm_struct *mm,
up_read(>mmap_sem);
return ret;
 }
-EXPORT_SYMBOL(__get_user_pages_unlocked);

 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 struct page **pages, unsigned int gup_flags)
diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index be8dc8d..84d0c7e 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -88,7 +88,7 @@ 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;
+   unsigned int flags = 0;

/* Work out address and page range required */
if (len == 0)
@@ -100,15 +100,19 @@ static int process_vm_rw_single_vec(unsigned long addr,

while (!rc && nr_pages && iov_iter_count(iter)) {
int pages = min(nr_pages, max_pages_per_loop);
+

[PATCH 0/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch series continues the cleanup of get_user_pages*() functions taking
advantage of the fact we can now pass gup_flags as we please.

It firstly adds an additional 'locked' parameter to get_user_pages_remote() to
allow for its callers to utilise VM_FAULT_RETRY functionality. This is necessary
as the invocation of __get_user_pages_unlocked() in process_vm_rw_single_vec()
makes use of this and no other existing higher level function would allow it to
do so.

Secondly existing callers of __get_user_pages_unlocked() are replaced with the
appropriate higher-level replacement - get_user_pages_unlocked() if the current
task and memory descriptor are referenced, or get_user_pages_remote() if other
task/memory descriptors are referenced (having acquiring mmap_sem.)

Lorenzo Stoakes (2):
  mm: add locked parameter to get_user_pages_remote()
  mm: unexport __get_user_pages_unlocked()

 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/infiniband/core/umem_odp.c  |  2 +-
 fs/exec.c   |  2 +-
 include/linux/mm.h  |  5 +
 kernel/events/uprobes.c |  4 ++--
 mm/gup.c| 20 
 mm/memory.c |  2 +-
 mm/nommu.c  |  7 +++
 mm/process_vm_access.c  | 12 
 security/tomoyo/domain.c|  2 +-
 virt/kvm/async_pf.c | 10 +++---
 virt/kvm/kvm_main.c |  5 ++---
 13 files changed, 41 insertions(+), 34 deletions(-)

[PATCH 0/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

This patch series continues the cleanup of get_user_pages*() functions taking
advantage of the fact we can now pass gup_flags as we please.

It firstly adds an additional 'locked' parameter to get_user_pages_remote() to
allow for its callers to utilise VM_FAULT_RETRY functionality. This is necessary
as the invocation of __get_user_pages_unlocked() in process_vm_rw_single_vec()
makes use of this and no other existing higher level function would allow it to
do so.

Secondly existing callers of __get_user_pages_unlocked() are replaced with the
appropriate higher-level replacement - get_user_pages_unlocked() if the current
task and memory descriptor are referenced, or get_user_pages_remote() if other
task/memory descriptors are referenced (having acquiring mmap_sem.)

Lorenzo Stoakes (2):
  mm: add locked parameter to get_user_pages_remote()
  mm: unexport __get_user_pages_unlocked()

 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/infiniband/core/umem_odp.c  |  2 +-
 fs/exec.c   |  2 +-
 include/linux/mm.h  |  5 +
 kernel/events/uprobes.c |  4 ++--
 mm/gup.c| 20 
 mm/memory.c |  2 +-
 mm/nommu.c  |  7 +++
 mm/process_vm_access.c  | 12 
 security/tomoyo/domain.c|  2 +-
 virt/kvm/async_pf.c | 10 +++---
 virt/kvm/kvm_main.c |  5 ++---
 13 files changed, 41 insertions(+), 34 deletions(-)

Re: [PATCH 0/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

On Thu, Oct 27, 2016 at 10:51:39AM +0100, Lorenzo Stoakes wrote:
> This patch series continues the cleanup of get_user_pages*() functions taking
> advantage of the fact we can now pass gup_flags as we please.

Note that this patch series has an unfortunate trivial dependency on my recent
'fix up get_user_pages* comments' patch which means this series applies against
-mmots but not mainline at this point in time.

Re: [PATCH 0/2] mm: unexport __get_user_pages_unlocked()

[Lorenzo Stoakes]

On Thu, Oct 27, 2016 at 10:51:39AM +0100, Lorenzo Stoakes wrote:
> This patch series continues the cleanup of get_user_pages*() functions taking
> advantage of the fact we can now pass gup_flags as we please.

Note that this patch series has an unfortunate trivial dependency on my recent
'fix up get_user_pages* comments' patch which means this series applies against
-mmots but not mainline at this point in time.

[PATCH 1/2] mm: add locked parameter to get_user_pages_remote()

[Lorenzo Stoakes]

This patch adds a int *locked parameter to get_user_pages_remote() to allow
VM_FAULT_RETRY faulting behaviour similar to get_user_pages_[un]locked().

Taking into account the previous adjustments to get_user_pages*() functions
allowing for the passing of gup_flags, we are now in a position where
__get_user_pages_unlocked() need only be exported for his ability to allow
VM_FAULT_RETRY behaviour, this adjustment allows us to subsequently unexport
__get_user_pages_unlocked() as well as allowing for future flexibility in the
use of get_user_pages_remote().

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/infiniband/core/umem_odp.c  |  2 +-
 fs/exec.c   |  2 +-
 include/linux/mm.h  |  2 +-
 kernel/events/uprobes.c |  4 ++--
 mm/gup.c| 12 
 mm/memory.c |  2 +-
 security/tomoyo/domain.c|  2 +-
 9 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 0370b84..0c69a97f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -763,7 +763,7 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
down_read(>mmap_sem);
while (pinned < npages) {
ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
-   flags, pvec + pinned, NULL);
+   flags, pvec + pinned, NULL, NULL);
if (ret < 0)
break;

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index c6f780f..836b525 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -522,7 +522,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
 obj->userptr.ptr + pinned * PAGE_SIZE,
 npages - pinned,
 flags,
-pvec + pinned, NULL);
+pvec + pinned, NULL, NULL);
if (ret < 0)
break;

diff --git a/drivers/infiniband/core/umem_odp.c 
b/drivers/infiniband/core/umem_odp.c
index 1f0fe32..6b079a3 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -578,7 +578,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
user_virt, u64 bcnt,
 */
npages = get_user_pages_remote(owning_process, owning_mm,
user_virt, gup_num_pages,
-   flags, local_page_list, NULL);
+   flags, local_page_list, NULL, NULL);
up_read(_mm->mmap_sem);

if (npages < 0)
diff --git a/fs/exec.c b/fs/exec.c
index 4e497b9..2cf049d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -209,7 +209,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, 
unsigned long pos,
 * doing the exec and bprm->mm is the new process's mm.
 */
ret = get_user_pages_remote(current, bprm->mm, pos, 1, gup_flags,
-   , NULL);
+   , NULL, NULL);
if (ret <= 0)
return NULL;

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a92c8d7..cc15445 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1274,7 +1274,7 @@ extern int access_remote_vm(struct mm_struct *mm, 
unsigned long addr,
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
-   struct vm_area_struct **vmas);
+   struct vm_area_struct **vmas, int *locked);
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index f9ec9ad..215871b 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -301,7 +301,7 @@ int uprobe_write_opcode(struct mm_struct *mm, unsigned long 
vaddr,
 retry:
/* Read the page with vaddr into memory */
ret = get_user_pages_remote(NULL, mm, vaddr, 1, FOLL_FORCE, _page,
-   );
+   , NULL);
if (ret <= 0)
return ret;

@@ -1712,7 +1712,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned 
long vaddr)

[PATCH 1/2] mm: add locked parameter to get_user_pages_remote()

[Lorenzo Stoakes]

This patch adds a int *locked parameter to get_user_pages_remote() to allow
VM_FAULT_RETRY faulting behaviour similar to get_user_pages_[un]locked().

Taking into account the previous adjustments to get_user_pages*() functions
allowing for the passing of gup_flags, we are now in a position where
__get_user_pages_unlocked() need only be exported for his ability to allow
VM_FAULT_RETRY behaviour, this adjustment allows us to subsequently unexport
__get_user_pages_unlocked() as well as allowing for future flexibility in the
use of get_user_pages_remote().

Signed-off-by: Lorenzo Stoakes 
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_userptr.c |  2 +-
 drivers/infiniband/core/umem_odp.c  |  2 +-
 fs/exec.c   |  2 +-
 include/linux/mm.h  |  2 +-
 kernel/events/uprobes.c |  4 ++--
 mm/gup.c| 12 
 mm/memory.c |  2 +-
 security/tomoyo/domain.c|  2 +-
 9 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 0370b84..0c69a97f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -763,7 +763,7 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
down_read(>mmap_sem);
while (pinned < npages) {
ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
-   flags, pvec + pinned, NULL);
+   flags, pvec + pinned, NULL, NULL);
if (ret < 0)
break;

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index c6f780f..836b525 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -522,7 +522,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
 obj->userptr.ptr + pinned * PAGE_SIZE,
 npages - pinned,
 flags,
-pvec + pinned, NULL);
+pvec + pinned, NULL, NULL);
if (ret < 0)
break;

diff --git a/drivers/infiniband/core/umem_odp.c 
b/drivers/infiniband/core/umem_odp.c
index 1f0fe32..6b079a3 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -578,7 +578,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
user_virt, u64 bcnt,
 */
npages = get_user_pages_remote(owning_process, owning_mm,
user_virt, gup_num_pages,
-   flags, local_page_list, NULL);
+   flags, local_page_list, NULL, NULL);
up_read(_mm->mmap_sem);

if (npages < 0)
diff --git a/fs/exec.c b/fs/exec.c
index 4e497b9..2cf049d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -209,7 +209,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, 
unsigned long pos,
 * doing the exec and bprm->mm is the new process's mm.
 */
ret = get_user_pages_remote(current, bprm->mm, pos, 1, gup_flags,
-   , NULL);
+   , NULL, NULL);
if (ret <= 0)
return NULL;

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a92c8d7..cc15445 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1274,7 +1274,7 @@ extern int access_remote_vm(struct mm_struct *mm, 
unsigned long addr,
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
-   struct vm_area_struct **vmas);
+   struct vm_area_struct **vmas, int *locked);
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas);
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index f9ec9ad..215871b 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -301,7 +301,7 @@ int uprobe_write_opcode(struct mm_struct *mm, unsigned long 
vaddr,
 retry:
/* Read the page with vaddr into memory */
ret = get_user_pages_remote(NULL, mm, vaddr, 1, FOLL_FORCE, _page,
-   );
+   , NULL);
if (ret <= 0)
return ret;

@@ -1712,7 +1712,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned 
long vaddr)
 * essentia

Re: [PATCH v2] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

On Thu, Oct 27, 2016 at 11:27:24AM +0200, Paolo Bonzini wrote:
>
>
> On 27/10/2016 02:12, Andrew Morton wrote:
> >
> >
> >> Subject: [PATCH v2] mm: remove unnecessary __get_user_pages_unlocked() 
> >> calls
> >
> > The patch is rather misidentified.
> >
> >>  virt/kvm/async_pf.c | 7 ---
> >>  virt/kvm/kvm_main.c | 5 ++---
> >>  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > It's a KVM patch and should have been called "kvm: remove ...".
> > Possibly the KVM maintainers will miss it for this reason.
>
> I noticed it, but I confused it with "mm: unexport __get_user_pages()".
>
> I'll merge this through the KVM tree for -rc3.

Actually Paolo could you hold off on this? As I think on reflection it'd make
more sense to batch this change up with a change to get_user_pages_remote() as
suggested by Michal.

Re: [PATCH v2] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

On Thu, Oct 27, 2016 at 11:27:24AM +0200, Paolo Bonzini wrote:
>
>
> On 27/10/2016 02:12, Andrew Morton wrote:
> >
> >
> >> Subject: [PATCH v2] mm: remove unnecessary __get_user_pages_unlocked() 
> >> calls
> >
> > The patch is rather misidentified.
> >
> >>  virt/kvm/async_pf.c | 7 ---
> >>  virt/kvm/kvm_main.c | 5 ++---
> >>  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > It's a KVM patch and should have been called "kvm: remove ...".
> > Possibly the KVM maintainers will miss it for this reason.
>
> I noticed it, but I confused it with "mm: unexport __get_user_pages()".
>
> I'll merge this through the KVM tree for -rc3.

Actually Paolo could you hold off on this? As I think on reflection it'd make
more sense to batch this change up with a change to get_user_pages_remote() as
suggested by Michal.

Re: [PATCH v2] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

On Wed, Oct 26, 2016 at 05:12:07PM -0700, Andrew Morton wrote:
> It's a KVM patch and should have been called "kvm: remove ...".
> Possibly the KVM maintainers will miss it for this reason.
>

Ah, indeed, however I think given my and Michal's discussion in this thread
regarding adjusting get_user_pages_remote() to allow for the unexporting of
__get_user_pages_unlocked() it would make more sense for me to batch up this
change with that change also (and then in fact actually be an mm patch.)

Re: [PATCH v2] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

On Wed, Oct 26, 2016 at 05:12:07PM -0700, Andrew Morton wrote:
> It's a KVM patch and should have been called "kvm: remove ...".
> Possibly the KVM maintainers will miss it for this reason.
>

Ah, indeed, however I think given my and Michal's discussion in this thread
regarding adjusting get_user_pages_remote() to allow for the unexporting of
__get_user_pages_unlocked() it would make more sense for me to batch up this
change with that change also (and then in fact actually be an mm patch.)

Re: [PATCH] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

On Wed, Oct 26, 2016 at 11:15:43AM +0200, Michal Hocko wrote:
> On Wed 26-10-16 00:46:31, Lorenzo Stoakes wrote:
> > The holdout for unexporting __get_user_pages_unlocked() is its invocation in
> > mm/process_vm_access.c: process_vm_rw_single_vec(), as this definitely 
> > _does_
> > seem to invoke VM_FAULT_RETRY behaviour which get_user_pages_remote() will 
> > not
> > trigger if we were to replace it with the latter.
>
> I am not sure I understand. Prior to 1e9877902dc7e this used
> get_user_pages_unlocked. What prevents us from reintroducing it with
> FOLL_REMOVE which was meant to be added by the above commit?
>
> Or am I missing your point?

The issue isn't the flags being passed, rather that in this case:

a. Replacing __get_user_pages_unlocked() with get_user_pages_unlocked() won't
   work as the latter assumes task = current and mm = current->mm but
   process_vm_rw_single_vec() needs to pass different task, mm.

b. Moving to get_user_pages_remote() _will_ allow us to pass different task, mm
   but won't however match existing behaviour precisely, since
   __get_user_pages_unlocked() acquires mmap_sem then passes a pointer to a
   local 'locked' variable to __get_user_pages_locked() which allows
   VM_FAULT_RETRY to trigger.

The main issue I had here was not being sure whether we care about the
VM_FAULT_RETRY functionality being used here or not, if we don't care then we
can just move to get_user_pages_remote(), otherwise perhaps this should be left
alone or maybe we need to consider adjusting the API to allow for remote access
with VM_FAULT_RETRY functionality.

Re: [PATCH] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

On Wed, Oct 26, 2016 at 11:15:43AM +0200, Michal Hocko wrote:
> On Wed 26-10-16 00:46:31, Lorenzo Stoakes wrote:
> > The holdout for unexporting __get_user_pages_unlocked() is its invocation in
> > mm/process_vm_access.c: process_vm_rw_single_vec(), as this definitely 
> > _does_
> > seem to invoke VM_FAULT_RETRY behaviour which get_user_pages_remote() will 
> > not
> > trigger if we were to replace it with the latter.
>
> I am not sure I understand. Prior to 1e9877902dc7e this used
> get_user_pages_unlocked. What prevents us from reintroducing it with
> FOLL_REMOVE which was meant to be added by the above commit?
>
> Or am I missing your point?

The issue isn't the flags being passed, rather that in this case:

a. Replacing __get_user_pages_unlocked() with get_user_pages_unlocked() won't
   work as the latter assumes task = current and mm = current->mm but
   process_vm_rw_single_vec() needs to pass different task, mm.

b. Moving to get_user_pages_remote() _will_ allow us to pass different task, mm
   but won't however match existing behaviour precisely, since
   __get_user_pages_unlocked() acquires mmap_sem then passes a pointer to a
   local 'locked' variable to __get_user_pages_locked() which allows
   VM_FAULT_RETRY to trigger.

The main issue I had here was not being sure whether we care about the
VM_FAULT_RETRY functionality being used here or not, if we don't care then we
can just move to get_user_pages_remote(), otherwise perhaps this should be left
alone or maybe we need to consider adjusting the API to allow for remote access
with VM_FAULT_RETRY functionality.

[PATCH v2] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
get_user_pages_unlocked() since we can now pass gup_flags.

In async_pf_execute() we need to pass different tsk, mm arguments so
get_user_pages_remote() is the sane replacement here (having added manual
acquisition and release of mmap_sem.)

Since we pass a NULL pages parameter the subsequent call to
__get_user_pages_locked() will have previously bailed any attempt at
VM_FAULT_RETRY, so we do not change this behaviour by using
get_user_pages_remote() which does not invoke VM_FAULT_RETRY logic at all.

Additionally get_user_pages_remote() reintroduces use of the FOLL_TOUCH
flag. However, this flag was originally silently dropped by 1e9877902dc7e
("mm/gup: Introduce get_user_pages_remote()"), so this appears to have been
unintentional and reintroducing it is therefore not an issue.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
v2: update description to reference dropped FOLL_TOUCH flag

 virt/kvm/async_pf.c | 7 ---
 virt/kvm/kvm_main.c | 5 ++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 8035cc1..e8c832c 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -82,10 +82,11 @@ static void async_pf_execute(struct work_struct *work)
/*
 * This work is run asynchromously to the task which owns
 * mm and might be done in another context, so we must
-* use FOLL_REMOTE.
+* access remotely.
 */
-   __get_user_pages_unlocked(NULL, mm, addr, 1, NULL,
-   FOLL_WRITE | FOLL_REMOTE);
+   down_read(>mmap_sem);
+   get_user_pages_remote(NULL, mm, addr, 1, FOLL_WRITE, NULL, NULL);
+   up_read(>mmap_sem);

kvm_async_page_present_sync(vcpu, apf);

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2907b7b..c45d951 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1415,13 +1415,12 @@ static int hva_to_pfn_slow(unsigned long addr, bool 
*async, bool write_fault,
npages = get_user_page_nowait(addr, write_fault, page);
up_read(>mm->mmap_sem);
} else {
-   unsigned int flags = FOLL_TOUCH | FOLL_HWPOISON;
+   unsigned int flags = FOLL_HWPOISON;

if (write_fault)
flags |= FOLL_WRITE;

-   npages = __get_user_pages_unlocked(current, current->mm, addr, 
1,
-  page, flags);
+   npages = get_user_pages_unlocked(addr, 1, page, flags);
}
if (npages != 1)
return npages;
--
2.10.1

[PATCH v2] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
get_user_pages_unlocked() since we can now pass gup_flags.

In async_pf_execute() we need to pass different tsk, mm arguments so
get_user_pages_remote() is the sane replacement here (having added manual
acquisition and release of mmap_sem.)

Since we pass a NULL pages parameter the subsequent call to
__get_user_pages_locked() will have previously bailed any attempt at
VM_FAULT_RETRY, so we do not change this behaviour by using
get_user_pages_remote() which does not invoke VM_FAULT_RETRY logic at all.

Additionally get_user_pages_remote() reintroduces use of the FOLL_TOUCH
flag. However, this flag was originally silently dropped by 1e9877902dc7e
("mm/gup: Introduce get_user_pages_remote()"), so this appears to have been
unintentional and reintroducing it is therefore not an issue.

Signed-off-by: Lorenzo Stoakes 
---
v2: update description to reference dropped FOLL_TOUCH flag

 virt/kvm/async_pf.c | 7 ---
 virt/kvm/kvm_main.c | 5 ++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 8035cc1..e8c832c 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -82,10 +82,11 @@ static void async_pf_execute(struct work_struct *work)
/*
 * This work is run asynchromously to the task which owns
 * mm and might be done in another context, so we must
-* use FOLL_REMOTE.
+* access remotely.
 */
-   __get_user_pages_unlocked(NULL, mm, addr, 1, NULL,
-   FOLL_WRITE | FOLL_REMOTE);
+   down_read(>mmap_sem);
+   get_user_pages_remote(NULL, mm, addr, 1, FOLL_WRITE, NULL, NULL);
+   up_read(>mmap_sem);

kvm_async_page_present_sync(vcpu, apf);

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2907b7b..c45d951 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1415,13 +1415,12 @@ static int hva_to_pfn_slow(unsigned long addr, bool 
*async, bool write_fault,
npages = get_user_page_nowait(addr, write_fault, page);
up_read(>mm->mmap_sem);
} else {
-   unsigned int flags = FOLL_TOUCH | FOLL_HWPOISON;
+   unsigned int flags = FOLL_HWPOISON;

if (write_fault)
flags |= FOLL_WRITE;

-   npages = __get_user_pages_unlocked(current, current->mm, addr, 
1,
-  page, flags);
+   npages = get_user_pages_unlocked(addr, 1, page, flags);
}
if (npages != 1)
return npages;
--
2.10.1

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Lorenzo Stoakes]

On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
>I am wondering whether we can go further. E.g. it is not really clear to
>me whether we need an explicit FOLL_REMOTE when we can in fact check
>mm != current->mm and imply that. Maybe there are some contexts which
>wouldn't work, I haven't checked.
>
>Then I am also wondering about FOLL_TOUCH behavior.
>__get_user_pages_unlocked has only few callers which used to be
>get_user_pages_unlocked before 1e9877902dc7e ("mm/gup: Introduce
>get_user_pages_remote()"). To me a dropped FOLL_TOUCH seems
>unintentional. Now that get_user_pages_unlocked has gup_flags argument I
>guess we might want to get rid of the __g-u-p-u version altogether, no?
>
>__get_user_pages is quite low level and imho shouldn't be exported. It's
>only user - kvm - should rather pull those two functions to gup instead
>and export them. There is nothing really KVM specific in them.

I believe I've attacked each of these, other than the use of explicit
FOLL_REMOTE which was explained by Dave.

> I also cannot say I would be entirely thrilled about get_user_pages_locked,
> we only have one user which can simply do lock g-u-p unlock AFAICS.

The principle difference here seems to be the availability of VM_FAULT_RETRY
behaviour (by passing a non-NULL locked argument), and indeed the comments in
gup.c recommends using get_user_pages_locked() if possible (though it seems not
to have been heeded too much :), so I'm not sure if this would be a fruitful
refactor, let me know!

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Lorenzo Stoakes]

On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
>I am wondering whether we can go further. E.g. it is not really clear to
>me whether we need an explicit FOLL_REMOTE when we can in fact check
>mm != current->mm and imply that. Maybe there are some contexts which
>wouldn't work, I haven't checked.
>
>Then I am also wondering about FOLL_TOUCH behavior.
>__get_user_pages_unlocked has only few callers which used to be
>get_user_pages_unlocked before 1e9877902dc7e ("mm/gup: Introduce
>get_user_pages_remote()"). To me a dropped FOLL_TOUCH seems
>unintentional. Now that get_user_pages_unlocked has gup_flags argument I
>guess we might want to get rid of the __g-u-p-u version altogether, no?
>
>__get_user_pages is quite low level and imho shouldn't be exported. It's
>only user - kvm - should rather pull those two functions to gup instead
>and export them. There is nothing really KVM specific in them.

I believe I've attacked each of these, other than the use of explicit
FOLL_REMOTE which was explained by Dave.

> I also cannot say I would be entirely thrilled about get_user_pages_locked,
> we only have one user which can simply do lock g-u-p unlock AFAICS.

The principle difference here seems to be the availability of VM_FAULT_RETRY
behaviour (by passing a non-NULL locked argument), and indeed the comments in
gup.c recommends using get_user_pages_locked() if possible (though it seems not
to have been heeded too much :), so I'm not sure if this would be a fruitful
refactor, let me know!

Re: [PATCH] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

On Wed, Oct 26, 2016 at 12:36:09AM +0100, Lorenzo Stoakes wrote:
> In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
> get_user_pages_unlocked() since we can now pass gup_flags.
>
> In async_pf_execute() we need to pass different tsk, mm arguments so
> get_user_pages_remote() is the sane replacement here (having added manual
> acquisition and release of mmap_sem.)
>
> Since we pass a NULL pages parameter the subsequent call to
> __get_user_pages_locked() will have previously bailed any attempt at
> VM_FAULT_RETRY, so we do not change this behaviour by using
> get_user_pages_remote() which does not invoke VM_FAULT_RETRY logic at all.
>
> Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>

Note that the use of get_user_pages_remote() in async_pf_execute() reintroduces
the use of the FOLL_TOUCH flag - I don't think this is a problem as this flag
was dropped by 1e987790 ("mm/gup: Introduce get_user_pages_remote()") which
states 'Without protection keys, this patch should not change any behavior', so
I don't think this was intentional.

Re: [PATCH] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

On Wed, Oct 26, 2016 at 12:36:09AM +0100, Lorenzo Stoakes wrote:
> In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
> get_user_pages_unlocked() since we can now pass gup_flags.
>
> In async_pf_execute() we need to pass different tsk, mm arguments so
> get_user_pages_remote() is the sane replacement here (having added manual
> acquisition and release of mmap_sem.)
>
> Since we pass a NULL pages parameter the subsequent call to
> __get_user_pages_locked() will have previously bailed any attempt at
> VM_FAULT_RETRY, so we do not change this behaviour by using
> get_user_pages_remote() which does not invoke VM_FAULT_RETRY logic at all.
>
> Signed-off-by: Lorenzo Stoakes 

Note that the use of get_user_pages_remote() in async_pf_execute() reintroduces
the use of the FOLL_TOUCH flag - I don't think this is a problem as this flag
was dropped by 1e987790 ("mm/gup: Introduce get_user_pages_remote()") which
states 'Without protection keys, this patch should not change any behavior', so
I don't think this was intentional.

Re: [PATCH] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

The holdout for unexporting __get_user_pages_unlocked() is its invocation in
mm/process_vm_access.c: process_vm_rw_single_vec(), as this definitely _does_
seem to invoke VM_FAULT_RETRY behaviour which get_user_pages_remote() will not
trigger if we were to replace it with the latter.

I'm not sure how to proceed in this case - get_user_pages_remote() invocations
assume mmap_sem is held so can't offer VM_FAULT_RETRY behaviour as the lock
can't be assumed to be safe to release, and get_user_pages_unlocked() assumes
tsk, mm are set to current, current->mm respectively so we can't use that here
either.

Is it important to retain VM_FAULT_RETRY behaviour here, does it matter? If it
isn't so important then we can just go ahead and replace with
get_user_pages_remote() and unexport away.

Of course the whole idea of unexporting __get_user_pages_unlocked() might be
bogus so let me know in that case also :)

Re: [PATCH] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

The holdout for unexporting __get_user_pages_unlocked() is its invocation in
mm/process_vm_access.c: process_vm_rw_single_vec(), as this definitely _does_
seem to invoke VM_FAULT_RETRY behaviour which get_user_pages_remote() will not
trigger if we were to replace it with the latter.

I'm not sure how to proceed in this case - get_user_pages_remote() invocations
assume mmap_sem is held so can't offer VM_FAULT_RETRY behaviour as the lock
can't be assumed to be safe to release, and get_user_pages_unlocked() assumes
tsk, mm are set to current, current->mm respectively so we can't use that here
either.

Is it important to retain VM_FAULT_RETRY behaviour here, does it matter? If it
isn't so important then we can just go ahead and replace with
get_user_pages_remote() and unexport away.

Of course the whole idea of unexporting __get_user_pages_unlocked() might be
bogus so let me know in that case also :)

[PATCH] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
get_user_pages_unlocked() since we can now pass gup_flags.

In async_pf_execute() we need to pass different tsk, mm arguments so
get_user_pages_remote() is the sane replacement here (having added manual
acquisition and release of mmap_sem.)

Since we pass a NULL pages parameter the subsequent call to
__get_user_pages_locked() will have previously bailed any attempt at
VM_FAULT_RETRY, so we do not change this behaviour by using
get_user_pages_remote() which does not invoke VM_FAULT_RETRY logic at all.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 virt/kvm/async_pf.c | 7 ---
 virt/kvm/kvm_main.c | 5 ++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 8035cc1..e8c832c 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -82,10 +82,11 @@ static void async_pf_execute(struct work_struct *work)
/*
 * This work is run asynchromously to the task which owns
 * mm and might be done in another context, so we must
-* use FOLL_REMOTE.
+* access remotely.
 */
-   __get_user_pages_unlocked(NULL, mm, addr, 1, NULL,
-   FOLL_WRITE | FOLL_REMOTE);
+   down_read(>mmap_sem);
+   get_user_pages_remote(NULL, mm, addr, 1, FOLL_WRITE, NULL, NULL);
+   up_read(>mmap_sem);

kvm_async_page_present_sync(vcpu, apf);

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2907b7b..c45d951 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1415,13 +1415,12 @@ static int hva_to_pfn_slow(unsigned long addr, bool 
*async, bool write_fault,
npages = get_user_page_nowait(addr, write_fault, page);
up_read(>mm->mmap_sem);
} else {
-   unsigned int flags = FOLL_TOUCH | FOLL_HWPOISON;
+   unsigned int flags = FOLL_HWPOISON;

if (write_fault)
flags |= FOLL_WRITE;

-   npages = __get_user_pages_unlocked(current, current->mm, addr, 
1,
-  page, flags);
+   npages = get_user_pages_unlocked(addr, 1, page, flags);
}
if (npages != 1)
return npages;
--
2.10.1

[PATCH] mm: remove unnecessary __get_user_pages_unlocked() calls

[Lorenzo Stoakes]

In hva_to_pfn_slow() we are able to replace __get_user_pages_unlocked() with
get_user_pages_unlocked() since we can now pass gup_flags.

In async_pf_execute() we need to pass different tsk, mm arguments so
get_user_pages_remote() is the sane replacement here (having added manual
acquisition and release of mmap_sem.)

Since we pass a NULL pages parameter the subsequent call to
__get_user_pages_locked() will have previously bailed any attempt at
VM_FAULT_RETRY, so we do not change this behaviour by using
get_user_pages_remote() which does not invoke VM_FAULT_RETRY logic at all.

Signed-off-by: Lorenzo Stoakes 
---
 virt/kvm/async_pf.c | 7 ---
 virt/kvm/kvm_main.c | 5 ++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 8035cc1..e8c832c 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -82,10 +82,11 @@ static void async_pf_execute(struct work_struct *work)
/*
 * This work is run asynchromously to the task which owns
 * mm and might be done in another context, so we must
-* use FOLL_REMOTE.
+* access remotely.
 */
-   __get_user_pages_unlocked(NULL, mm, addr, 1, NULL,
-   FOLL_WRITE | FOLL_REMOTE);
+   down_read(>mmap_sem);
+   get_user_pages_remote(NULL, mm, addr, 1, FOLL_WRITE, NULL, NULL);
+   up_read(>mmap_sem);

kvm_async_page_present_sync(vcpu, apf);

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2907b7b..c45d951 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1415,13 +1415,12 @@ static int hva_to_pfn_slow(unsigned long addr, bool 
*async, bool write_fault,
npages = get_user_page_nowait(addr, write_fault, page);
up_read(>mm->mmap_sem);
} else {
-   unsigned int flags = FOLL_TOUCH | FOLL_HWPOISON;
+   unsigned int flags = FOLL_HWPOISON;

if (write_fault)
flags |= FOLL_WRITE;

-   npages = __get_user_pages_unlocked(current, current->mm, addr, 
1,
-  page, flags);
+   npages = get_user_pages_unlocked(addr, 1, page, flags);
}
if (npages != 1)
return npages;
--
2.10.1

[PATCH] mm: fixup get_user_pages* comments

[Lorenzo Stoakes]

In the previous round of get_user_pages* changes comments attached to
__get_user_pages_unlocked() and get_user_pages_unlocked() were rendered
incorrect, this patch corrects them.

In addition the get_user_pages_unlocked() comment seems to have already been
outdated as it referred to tsk, mm parameters which were removed in c12d2da5
("mm/gup: Remove the macro overload API migration helpers from the get_user*()
APIs"), this patch fixes this also.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 mm/gup.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index ec4f827..e6147f1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -857,14 +857,12 @@ 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 to
- * pass additional gup_flags as last parameter (like FOLL_HWPOISON).
+ * 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",
- * "FOLL_WRITE" and "FOLL_FORCE" are set implicitly as needed
- * according to the parameters "pages", "write", "force"
- * respectively.
+ * caller if required (just like with __get_user_pages). "FOLL_GET"
+ * is set implicitly if "pages" is non-NULL.
  */
 __always_inline long __get_user_pages_unlocked(struct task_struct *tsk, struct 
mm_struct *mm,
   unsigned long start, unsigned 
long nr_pages,
@@ -894,10 +892,8 @@ EXPORT_SYMBOL(__get_user_pages_unlocked);
  *  get_user_pages_unlocked(tsk, mm, ..., pages);
  *
  * It is functionally equivalent to get_user_pages_fast so
- * get_user_pages_fast should be used instead, if the two parameters
- * "tsk" and "mm" are respectively equal to current and current->mm,
- * or if "force" shall be set to 1 (get_user_pages_fast misses the
- * "force" parameter).
+ * get_user_pages_fast should be used instead if specific gup_flags
+ * (e.g. FOLL_FORCE) are not required.
  */
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 struct page **pages, unsigned int gup_flags)
-- 
2.10.1

[PATCH] mm: fixup get_user_pages* comments

[Lorenzo Stoakes]

In the previous round of get_user_pages* changes comments attached to
__get_user_pages_unlocked() and get_user_pages_unlocked() were rendered
incorrect, this patch corrects them.

In addition the get_user_pages_unlocked() comment seems to have already been
outdated as it referred to tsk, mm parameters which were removed in c12d2da5
("mm/gup: Remove the macro overload API migration helpers from the get_user*()
APIs"), this patch fixes this also.

Signed-off-by: Lorenzo Stoakes 
---
 mm/gup.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index ec4f827..e6147f1 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -857,14 +857,12 @@ 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 to
- * pass additional gup_flags as last parameter (like FOLL_HWPOISON).
+ * 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",
- * "FOLL_WRITE" and "FOLL_FORCE" are set implicitly as needed
- * according to the parameters "pages", "write", "force"
- * respectively.
+ * caller if required (just like with __get_user_pages). "FOLL_GET"
+ * is set implicitly if "pages" is non-NULL.
  */
 __always_inline long __get_user_pages_unlocked(struct task_struct *tsk, struct 
mm_struct *mm,
   unsigned long start, unsigned 
long nr_pages,
@@ -894,10 +892,8 @@ EXPORT_SYMBOL(__get_user_pages_unlocked);
  *  get_user_pages_unlocked(tsk, mm, ..., pages);
  *
  * It is functionally equivalent to get_user_pages_fast so
- * get_user_pages_fast should be used instead, if the two parameters
- * "tsk" and "mm" are respectively equal to current and current->mm,
- * or if "force" shall be set to 1 (get_user_pages_fast misses the
- * "force" parameter).
+ * get_user_pages_fast should be used instead if specific gup_flags
+ * (e.g. FOLL_FORCE) are not required.
  */
 long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 struct page **pages, unsigned int gup_flags)
-- 
2.10.1

[PATCH] mm: unexport __get_user_pages()

[Lorenzo Stoakes]

This patch unexports the low-level __get_user_pages() function. Recent
refactoring of the get_user_pages* functions allow flags to be passed through
get_user_pages() which eliminates the need for access to this function from its
one user, kvm.

We can see that the 2 calls to get_user_pages() which replace __get_user_pages()
in kvm_main.c are equivalent by examining their call stacks:

get_user_page_nowait():
  get_user_pages(start, 1, flags, page, NULL)
  __get_user_pages_locked(current, current->mm, start, 1, page, NULL, NULL,
  false, flags | FOLL_TOUCH)
  __get_user_pages(current, current->mm, start, 1,
   flags | FOLL_TOUCH | FOLL_GET, page, NULL, NULL)

check_user_page_hwpoison():
  get_user_pages(addr, 1, flags, NULL, NULL)
  __get_user_pages_locked(current, current->mm, addr, 1, NULL, NULL, NULL,
  false, flags | FOLL_TOUCH)
  __get_user_pages(current, current->mm, addr, 1, flags | FOLL_TOUCH, NULL,
   NULL, NULL)

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 include/linux/mm.h  |  4 
 mm/gup.c|  3 +--
 mm/nommu.c  |  2 +-
 virt/kvm/kvm_main.c | 10 --
 4 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3a19185..a92c8d7 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1271,10 +1271,6 @@ extern int access_process_vm(struct task_struct *tsk, 
unsigned long addr, void *
 extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
void *buf, int len, unsigned int gup_flags);
 
-long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
- unsigned long start, unsigned long nr_pages,
- unsigned int foll_flags, struct page **pages,
- struct vm_area_struct **vmas, int *nonblocking);
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
diff --git a/mm/gup.c b/mm/gup.c
index 7aa113c..ec4f827 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -526,7 +526,7 @@ static int check_vma_flags(struct vm_area_struct *vma, 
unsigned long gup_flags)
  * instead of __get_user_pages. __get_user_pages should be used only if
  * you need some special @gup_flags.
  */
-long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
+static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas, int *nonblocking)
@@ -631,7 +631,6 @@ long __get_user_pages(struct task_struct *tsk, struct 
mm_struct *mm,
} while (nr_pages);
return i;
 }
-EXPORT_SYMBOL(__get_user_pages);
 
 bool vma_permits_fault(struct vm_area_struct *vma, unsigned int fault_flags)
 {
diff --git a/mm/nommu.c b/mm/nommu.c
index db5fd17..8b8faaf 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -109,7 +109,7 @@ unsigned int kobjsize(const void *objp)
return PAGE_SIZE << compound_order(page);
 }
 
-long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
+static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
  unsigned long start, unsigned long nr_pages,
  unsigned int foll_flags, struct page **pages,
  struct vm_area_struct **vmas, int *nonblocking)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 28510e7..2907b7b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1346,21 +1346,19 @@ unsigned long kvm_vcpu_gfn_to_hva_prot(struct kvm_vcpu 
*vcpu, gfn_t gfn, bool *w
 static int get_user_page_nowait(unsigned long start, int write,
struct page **page)
 {
-   int flags = FOLL_TOUCH | FOLL_NOWAIT | FOLL_HWPOISON | FOLL_GET;
+   int flags = FOLL_NOWAIT | FOLL_HWPOISON;
 
if (write)
flags |= FOLL_WRITE;
 
-   return __get_user_pages(current, current->mm, start, 1, flags, page,
-   NULL, NULL);
+   return get_user_pages(start, 1, flags, page, NULL);
 }
 
 static inline int check_user_page_hwpoison(unsigned long addr)
 {
-   int rc, flags = FOLL_TOUCH | FOLL_HWPOISON | FOLL_WRITE;
+   int rc, flags = FOLL_HWPOISON | FOLL_WRITE;
 
-   rc = __get_user_pages(current, current->mm, addr, 1,
- flags, NULL, NULL, NULL);
+   rc = get_user_pages(addr, 1, flags, NULL, NULL);
return rc == -EHWPOISON;
 }
 
-- 
2.10.1

[PATCH] mm: unexport __get_user_pages()

[Lorenzo Stoakes]

This patch unexports the low-level __get_user_pages() function. Recent
refactoring of the get_user_pages* functions allow flags to be passed through
get_user_pages() which eliminates the need for access to this function from its
one user, kvm.

We can see that the 2 calls to get_user_pages() which replace __get_user_pages()
in kvm_main.c are equivalent by examining their call stacks:

get_user_page_nowait():
  get_user_pages(start, 1, flags, page, NULL)
  __get_user_pages_locked(current, current->mm, start, 1, page, NULL, NULL,
  false, flags | FOLL_TOUCH)
  __get_user_pages(current, current->mm, start, 1,
   flags | FOLL_TOUCH | FOLL_GET, page, NULL, NULL)

check_user_page_hwpoison():
  get_user_pages(addr, 1, flags, NULL, NULL)
  __get_user_pages_locked(current, current->mm, addr, 1, NULL, NULL, NULL,
  false, flags | FOLL_TOUCH)
  __get_user_pages(current, current->mm, addr, 1, flags | FOLL_TOUCH, NULL,
   NULL, NULL)

Signed-off-by: Lorenzo Stoakes 
---
 include/linux/mm.h  |  4 
 mm/gup.c|  3 +--
 mm/nommu.c  |  2 +-
 virt/kvm/kvm_main.c | 10 --
 4 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3a19185..a92c8d7 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1271,10 +1271,6 @@ extern int access_process_vm(struct task_struct *tsk, 
unsigned long addr, void *
 extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
void *buf, int len, unsigned int gup_flags);
 
-long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
- unsigned long start, unsigned long nr_pages,
- unsigned int foll_flags, struct page **pages,
- struct vm_area_struct **vmas, int *nonblocking);
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
diff --git a/mm/gup.c b/mm/gup.c
index 7aa113c..ec4f827 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -526,7 +526,7 @@ static int check_vma_flags(struct vm_area_struct *vma, 
unsigned long gup_flags)
  * instead of __get_user_pages. __get_user_pages should be used only if
  * you need some special @gup_flags.
  */
-long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
+static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas, int *nonblocking)
@@ -631,7 +631,6 @@ long __get_user_pages(struct task_struct *tsk, struct 
mm_struct *mm,
} while (nr_pages);
return i;
 }
-EXPORT_SYMBOL(__get_user_pages);
 
 bool vma_permits_fault(struct vm_area_struct *vma, unsigned int fault_flags)
 {
diff --git a/mm/nommu.c b/mm/nommu.c
index db5fd17..8b8faaf 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -109,7 +109,7 @@ unsigned int kobjsize(const void *objp)
return PAGE_SIZE << compound_order(page);
 }
 
-long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
+static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
  unsigned long start, unsigned long nr_pages,
  unsigned int foll_flags, struct page **pages,
  struct vm_area_struct **vmas, int *nonblocking)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 28510e7..2907b7b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1346,21 +1346,19 @@ unsigned long kvm_vcpu_gfn_to_hva_prot(struct kvm_vcpu 
*vcpu, gfn_t gfn, bool *w
 static int get_user_page_nowait(unsigned long start, int write,
struct page **page)
 {
-   int flags = FOLL_TOUCH | FOLL_NOWAIT | FOLL_HWPOISON | FOLL_GET;
+   int flags = FOLL_NOWAIT | FOLL_HWPOISON;
 
if (write)
flags |= FOLL_WRITE;
 
-   return __get_user_pages(current, current->mm, start, 1, flags, page,
-   NULL, NULL);
+   return get_user_pages(start, 1, flags, page, NULL);
 }
 
 static inline int check_user_page_hwpoison(unsigned long addr)
 {
-   int rc, flags = FOLL_TOUCH | FOLL_HWPOISON | FOLL_WRITE;
+   int rc, flags = FOLL_HWPOISON | FOLL_WRITE;
 
-   rc = __get_user_pages(current, current->mm, addr, 1,
- flags, NULL, NULL, NULL);
+   rc = get_user_pages(addr, 1, flags, NULL, NULL);
return rc == -EHWPOISON;
 }
 
-- 
2.10.1

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Lorenzo Stoakes]

On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
> I am wondering whether we can go further. E.g. it is not really clear to
> me whether we need an explicit FOLL_REMOTE when we can in fact check
> mm != current->mm and imply that. Maybe there are some contexts which
> wouldn't work, I haven't checked.

This flag is set even when /proc/self/mem is used. I've not looked deeply into
this flag but perhaps accessing your own memory this way can be considered
'remote' since you're not accessing it directly. On the other hand, perhaps this
is just mistaken in this case?

> I guess there is more work in that area and I do not want to impose all
> that work on you, but I couldn't resist once I saw you playing in that
> area ;) Definitely a good start!

Thanks, I am more than happy to go as far down this rabbit hole as is helpful,
no imposition at all :)

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Lorenzo Stoakes]

On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
> I am wondering whether we can go further. E.g. it is not really clear to
> me whether we need an explicit FOLL_REMOTE when we can in fact check
> mm != current->mm and imply that. Maybe there are some contexts which
> wouldn't work, I haven't checked.

This flag is set even when /proc/self/mem is used. I've not looked deeply into
this flag but perhaps accessing your own memory this way can be considered
'remote' since you're not accessing it directly. On the other hand, perhaps this
is just mistaken in this case?

> I guess there is more work in that area and I do not want to impose all
> that work on you, but I couldn't resist once I saw you playing in that
> area ;) Definitely a good start!

Thanks, I am more than happy to go as far down this rabbit hole as is helpful,
no imposition at all :)

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

On Wed, Oct 19, 2016 at 10:52:05AM +0200, Michal Hocko wrote:
> yes this is the desirable and expected behavior.
>
> > wonder if this is desirable behaviour or whether this ought to be limited to
> > ptrace system calls. Regardless, by making the flag more visible it makes it
> > easier to see that this is happening.
>
> mem_open already enforces PTRACE_MODE_ATTACH

Ah I missed this, that makes a lot of sense, thanks!

I still wonder whether other invocations of access_remote_vm() in fs/proc/base.c
(the principle caller of this function) need FOLL_FORCE, for example the various
calls that simply read data from other processes, so I think the point stands
about keeping this explicit.

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

On Wed, Oct 19, 2016 at 10:52:05AM +0200, Michal Hocko wrote:
> yes this is the desirable and expected behavior.
>
> > wonder if this is desirable behaviour or whether this ought to be limited to
> > ptrace system calls. Regardless, by making the flag more visible it makes it
> > easier to see that this is happening.
>
> mem_open already enforces PTRACE_MODE_ATTACH

Ah I missed this, that makes a lot of sense, thanks!

I still wonder whether other invocations of access_remote_vm() in fs/proc/base.c
(the principle caller of this function) need FOLL_FORCE, for example the various
calls that simply read data from other processes, so I think the point stands
about keeping this explicit.

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

On Wed, Oct 19, 2016 at 10:13:52AM +0200, Michal Hocko wrote:
> On Wed 19-10-16 09:59:03, Jan Kara wrote:
> > On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> > > This patch removes the write parameter from __access_remote_vm() and 
> > > replaces it
> > > with a gup_flags parameter as use of this function previously _implied_
> > > FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> > >
> > > We make this explicit as use of FOLL_FORCE can result in surprising 
> > > behaviour
> > > (and hence bugs) within the mm subsystem.
> > >
> > > Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
> >
> > So I'm not convinced this (and the following two patches) is actually
> > helping much. By grepping for FOLL_FORCE we will easily see that any caller
> > of access_remote_vm() gets that semantics and can thus continue search
>
> I am really wondering. Is there anything inherent that would require
> FOLL_FORCE for access_remote_vm? I mean FOLL_FORCE is a really
> non-trivial thing. It doesn't obey vma permissions so we should really
> minimize its usage. Do all of those users really need FOLL_FORCE?

I wonder about this also, for example by accessing /proc/self/mem you trigger
access_remote_vm() and consequently get_user_pages_remote() meaning FOLL_FORCE
is implied and you can use /proc/self/mem to override any VMA permissions. I
wonder if this is desirable behaviour or whether this ought to be limited to
ptrace system calls. Regardless, by making the flag more visible it makes it
easier to see that this is happening.

Note that this /proc/self/mem behaviour is what triggered the bug that brought
about this discussion in the first place -
https://marc.info/?l=linux-mm=147363447105059 - as using /proc/self/mem this
way on PROT_NONE memory broke some assumptions.

On the other hand I see your point Jan in that you know any caller of these
functions will have FOLL_FORCE implied, and you have to decide to stop passing
the flag at _some_ point in the stack, however it seems fairly sane to have that
stopping point exist outside of exported gup functions so the gup interface
forces explicitness but callers can wrap things as they need.

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

On Wed, Oct 19, 2016 at 10:13:52AM +0200, Michal Hocko wrote:
> On Wed 19-10-16 09:59:03, Jan Kara wrote:
> > On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> > > This patch removes the write parameter from __access_remote_vm() and 
> > > replaces it
> > > with a gup_flags parameter as use of this function previously _implied_
> > > FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> > >
> > > We make this explicit as use of FOLL_FORCE can result in surprising 
> > > behaviour
> > > (and hence bugs) within the mm subsystem.
> > >
> > > Signed-off-by: Lorenzo Stoakes 
> >
> > So I'm not convinced this (and the following two patches) is actually
> > helping much. By grepping for FOLL_FORCE we will easily see that any caller
> > of access_remote_vm() gets that semantics and can thus continue search
>
> I am really wondering. Is there anything inherent that would require
> FOLL_FORCE for access_remote_vm? I mean FOLL_FORCE is a really
> non-trivial thing. It doesn't obey vma permissions so we should really
> minimize its usage. Do all of those users really need FOLL_FORCE?

I wonder about this also, for example by accessing /proc/self/mem you trigger
access_remote_vm() and consequently get_user_pages_remote() meaning FOLL_FORCE
is implied and you can use /proc/self/mem to override any VMA permissions. I
wonder if this is desirable behaviour or whether this ought to be limited to
ptrace system calls. Regardless, by making the flag more visible it makes it
easier to see that this is happening.

Note that this /proc/self/mem behaviour is what triggered the bug that brought
about this discussion in the first place -
https://marc.info/?l=linux-mm=147363447105059 - as using /proc/self/mem this
way on PROT_NONE memory broke some assumptions.

On the other hand I see your point Jan in that you know any caller of these
functions will have FOLL_FORCE implied, and you have to decide to stop passing
the flag at _some_ point in the stack, however it seems fairly sane to have that
stopping point exist outside of exported gup functions so the gup interface
forces explicitness but callers can wrap things as they need.

Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags

[Lorenzo Stoakes]

On Tue, Oct 18, 2016 at 02:54:25PM +0200, Jan Kara wrote:
> > @@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned 
> > long nr_pages,
> > int write, int force, struct page **pages,
> > struct vm_area_struct **vmas);
> >  long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > -   int write, int force, struct page **pages, int *locked);
> > +   unsigned int gup_flags, struct page **pages, int *locked);
>
> Hum, the prototype is inconsistent with e.g. __get_user_pages_unlocked()
> where gup_flags come after **pages argument. Actually it makes more sense
> to have it before **pages so that input arguments come first and output
> arguments second but I don't care that much. But it definitely should be
> consistent...

It was difficult to decide quite how to arrange parameters as there was
inconsitency with regards to parameter ordering already - for example
__get_user_pages() places its flags argument before pages whereas, as you note,
__get_user_pages_unlocked() puts them afterwards.

I ended up compromising by trying to match the existing ordering of the function
as much as I could by replacing write, force pairs with gup_flags in the same
location (with the exception of get_user_pages_unlocked() which I felt should
match __get_user_pages_unlocked() in signature) or if there was already a
gup_flags parameter as in the case of __get_user_pages_unlocked() I simply
removed the write, force pair and left the flags as the last parameter.

I am happy to rearrange parameters as needed, however I am not sure if it'd be
worthwhile for me to do so (I am keen to try to avoid adding too much noise here
:)

If we were to rearrange parameters for consistency I'd suggest adjusting
__get_user_pages_unlocked() to put gup_flags before pages and do the same with
get_user_pages_unlocked(), let me know what you think.

Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags

[Lorenzo Stoakes]

On Tue, Oct 18, 2016 at 02:54:25PM +0200, Jan Kara wrote:
> > @@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned 
> > long nr_pages,
> > int write, int force, struct page **pages,
> > struct vm_area_struct **vmas);
> >  long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > -   int write, int force, struct page **pages, int *locked);
> > +   unsigned int gup_flags, struct page **pages, int *locked);
>
> Hum, the prototype is inconsistent with e.g. __get_user_pages_unlocked()
> where gup_flags come after **pages argument. Actually it makes more sense
> to have it before **pages so that input arguments come first and output
> arguments second but I don't care that much. But it definitely should be
> consistent...

It was difficult to decide quite how to arrange parameters as there was
inconsitency with regards to parameter ordering already - for example
__get_user_pages() places its flags argument before pages whereas, as you note,
__get_user_pages_unlocked() puts them afterwards.

I ended up compromising by trying to match the existing ordering of the function
as much as I could by replacing write, force pairs with gup_flags in the same
location (with the exception of get_user_pages_unlocked() which I felt should
match __get_user_pages_unlocked() in signature) or if there was already a
gup_flags parameter as in the case of __get_user_pages_unlocked() I simply
removed the write, force pair and left the flags as the last parameter.

I am happy to rearrange parameters as needed, however I am not sure if it'd be
worthwhile for me to do so (I am keen to try to avoid adding too much noise here
:)

If we were to rearrange parameters for consistency I'd suggest adjusting
__get_user_pages_unlocked() to put gup_flags before pages and do the same with
get_user_pages_unlocked(), let me know what you think.

[PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags

[Lorenzo Stoakes]

This patch removes the write and force parameters from get_user_pages_locked()
and replaces them with a gup_flags parameter 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 <lstoa...@gmail.com>
---
 include/linux/mm.h |  2 +-
 mm/frame_vector.c  |  8 +++-
 mm/gup.c   | 12 +++-
 mm/nommu.c |  5 -
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6adc4bc..27ab538 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
int write, int force, struct page **pages,
struct vm_area_struct **vmas);
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-   int write, int force, struct page **pages, int *locked);
+   unsigned int gup_flags, 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,
   struct page **pages, unsigned int gup_flags);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 381bb07..81b6749 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -41,10 +41,16 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
int ret = 0;
int err;
int locked;
+   unsigned int gup_flags = 0;
 
if (nr_frames == 0)
return 0;
 
+   if (write)
+   gup_flags |= FOLL_WRITE;
+   if (force)
+   gup_flags |= FOLL_FORCE;
+
if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
nr_frames = vec->nr_allocated;
 
@@ -59,7 +65,7 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
vec->got_ref = true;
vec->is_pfns = false;
ret = get_user_pages_locked(start, nr_frames,
-   write, force, (struct page **)(vec->ptrs), );
+   gup_flags, (struct page **)(vec->ptrs), );
goto out;
}
 
diff --git a/mm/gup.c b/mm/gup.c
index cfcb014..7a0d033 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -838,18 +838,12 @@ static __always_inline long 
__get_user_pages_locked(struct task_struct *tsk,
  *  up_read(>mmap_sem);
  */
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-  int write, int force, struct page **pages,
+  unsigned int gup_flags, struct page **pages,
   int *locked)
 {
-   unsigned int flags = FOLL_TOUCH;
-
-   if (write)
-   flags |= FOLL_WRITE;
-   if (force)
-   flags |= FOLL_FORCE;
-
return __get_user_pages_locked(current, current->mm, start, nr_pages,
-  pages, NULL, locked, true, flags);
+  pages, NULL, locked, true,
+  gup_flags | FOLL_TOUCH);
 }
 EXPORT_SYMBOL(get_user_pages_locked);
 
diff --git a/mm/nommu.c b/mm/nommu.c
index 7e27add..842cfdd 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -176,9 +176,12 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
 EXPORT_SYMBOL(get_user_pages);
 
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-   int write, int force, struct page **pages,
+   unsigned int gup_flags, struct page **pages,
int *locked)
 {
+   int write = gup_flags & FOLL_WRITE;
+   int force = gup_flags & FOLL_FORCE;
+
return get_user_pages(start, nr_pages, write, force, pages, NULL);
 }
 EXPORT_SYMBOL(get_user_pages_locked);
-- 
2.10.0

[PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags

[Lorenzo Stoakes]

This patch removes the write and force parameters from get_user_pages_locked()
and replaces them with a gup_flags parameter 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 |  2 +-
 mm/frame_vector.c  |  8 +++-
 mm/gup.c   | 12 +++-
 mm/nommu.c |  5 -
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6adc4bc..27ab538 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
int write, int force, struct page **pages,
struct vm_area_struct **vmas);
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-   int write, int force, struct page **pages, int *locked);
+   unsigned int gup_flags, 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,
   struct page **pages, unsigned int gup_flags);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 381bb07..81b6749 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -41,10 +41,16 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
int ret = 0;
int err;
int locked;
+   unsigned int gup_flags = 0;
 
if (nr_frames == 0)
return 0;
 
+   if (write)
+   gup_flags |= FOLL_WRITE;
+   if (force)
+   gup_flags |= FOLL_FORCE;
+
if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
nr_frames = vec->nr_allocated;
 
@@ -59,7 +65,7 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
vec->got_ref = true;
vec->is_pfns = false;
ret = get_user_pages_locked(start, nr_frames,
-   write, force, (struct page **)(vec->ptrs), );
+   gup_flags, (struct page **)(vec->ptrs), );
goto out;
}
 
diff --git a/mm/gup.c b/mm/gup.c
index cfcb014..7a0d033 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -838,18 +838,12 @@ static __always_inline long 
__get_user_pages_locked(struct task_struct *tsk,
  *  up_read(>mmap_sem);
  */
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-  int write, int force, struct page **pages,
+  unsigned int gup_flags, struct page **pages,
   int *locked)
 {
-   unsigned int flags = FOLL_TOUCH;
-
-   if (write)
-   flags |= FOLL_WRITE;
-   if (force)
-   flags |= FOLL_FORCE;
-
return __get_user_pages_locked(current, current->mm, start, nr_pages,
-  pages, NULL, locked, true, flags);
+  pages, NULL, locked, true,
+  gup_flags | FOLL_TOUCH);
 }
 EXPORT_SYMBOL(get_user_pages_locked);
 
diff --git a/mm/nommu.c b/mm/nommu.c
index 7e27add..842cfdd 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -176,9 +176,12 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
 EXPORT_SYMBOL(get_user_pages);
 
 long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-   int write, int force, struct page **pages,
+   unsigned int gup_flags, struct page **pages,
int *locked)
 {
+   int write = gup_flags & FOLL_WRITE;
+   int force = gup_flags & FOLL_FORCE;
+
return get_user_pages(start, nr_pages, write, force, pages, NULL);
 }
 EXPORT_SYMBOL(get_user_pages_locked);
-- 
2.10.0

[PATCH 10/10] mm: replace access_process_vm() write parameter with gup_flags

[Lorenzo Stoakes]

This patch removes the write parameter from access_process_vm() and replaces it
with a gup_flags parameter as use of this function previously _implied_
FOLL_FORCE, whereas after this patch callers explicitly pass this flag.

We make this explicit as use of FOLL_FORCE can result in surprising behaviour
(and hence bugs) within the mm subsystem.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 arch/alpha/kernel/ptrace.c |  9 ++---
 arch/blackfin/kernel/ptrace.c  |  5 +++--
 arch/cris/arch-v32/kernel/ptrace.c |  4 ++--
 arch/ia64/kernel/ptrace.c  | 14 +-
 arch/m32r/kernel/ptrace.c  | 15 ++-
 arch/mips/kernel/ptrace32.c|  5 +++--
 arch/powerpc/kernel/ptrace32.c |  5 +++--
 arch/score/kernel/ptrace.c | 10 ++
 arch/sparc/kernel/ptrace_64.c  | 24 
 arch/x86/kernel/step.c |  3 ++-
 arch/x86/um/ptrace_32.c|  3 ++-
 arch/x86/um/ptrace_64.c|  3 ++-
 include/linux/mm.h |  3 ++-
 kernel/ptrace.c| 16 ++--
 mm/memory.c|  8 ++--
 mm/nommu.c |  6 +++---
 mm/util.c  |  5 +++--
 17 files changed, 84 insertions(+), 54 deletions(-)

diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
index d9ee817..940dfb4 100644
--- a/arch/alpha/kernel/ptrace.c
+++ b/arch/alpha/kernel/ptrace.c
@@ -157,14 +157,16 @@ put_reg(struct task_struct *task, unsigned long regno, 
unsigned long data)
 static inline int
 read_int(struct task_struct *task, unsigned long addr, int * data)
 {
-   int copied = access_process_vm(task, addr, data, sizeof(int), 0);
+   int copied = access_process_vm(task, addr, data, sizeof(int),
+   FOLL_FORCE);
return (copied == sizeof(int)) ? 0 : -EIO;
 }
 
 static inline int
 write_int(struct task_struct *task, unsigned long addr, int data)
 {
-   int copied = access_process_vm(task, addr, , sizeof(int), 1);
+   int copied = access_process_vm(task, addr, , sizeof(int),
+   FOLL_FORCE | FOLL_WRITE);
return (copied == sizeof(int)) ? 0 : -EIO;
 }
 
@@ -281,7 +283,8 @@ long arch_ptrace(struct task_struct *child, long request,
/* When I and D space are separate, these will need to be fixed.  */
case PTRACE_PEEKTEXT: /* read word at location addr. */
case PTRACE_PEEKDATA:
-   copied = access_process_vm(child, addr, , sizeof(tmp), 0);
+   copied = access_process_vm(child, addr, , sizeof(tmp),
+   FOLL_FORCE);
ret = -EIO;
if (copied != sizeof(tmp))
break;
diff --git a/arch/blackfin/kernel/ptrace.c b/arch/blackfin/kernel/ptrace.c
index 8b8fe67..8d79286 100644
--- a/arch/blackfin/kernel/ptrace.c
+++ b/arch/blackfin/kernel/ptrace.c
@@ -271,7 +271,7 @@ long arch_ptrace(struct task_struct *child, long request,
case BFIN_MEM_ACCESS_CORE:
case BFIN_MEM_ACCESS_CORE_ONLY:
copied = access_process_vm(child, addr, ,
-  to_copy, 0);
+  to_copy, FOLL_FORCE);
if (copied)
break;
 
@@ -324,7 +324,8 @@ long arch_ptrace(struct task_struct *child, long request,
case BFIN_MEM_ACCESS_CORE:
case BFIN_MEM_ACCESS_CORE_ONLY:
copied = access_process_vm(child, addr, ,
-  to_copy, 1);
+  to_copy,
+  FOLL_FORCE | 
FOLL_WRITE);
break;
case BFIN_MEM_ACCESS_DMA:
if (safe_dma_memcpy(paddr, , to_copy))
diff --git a/arch/cris/arch-v32/kernel/ptrace.c 
b/arch/cris/arch-v32/kernel/ptrace.c
index f085229..f0df654 100644
--- a/arch/cris/arch-v32/kernel/ptrace.c
+++ b/arch/cris/arch-v32/kernel/ptrace.c
@@ -147,7 +147,7 @@ long arch_ptrace(struct task_struct *child, long request,
/* The trampoline page is globally mapped, no 
page table to traverse.*/
tmp = *(unsigned long*)addr;
} else {
-   copied = access_process_vm(child, addr, , 
sizeof(tmp), 0);
+   copied = access_process_vm(child, addr, , 
sizeof(tmp), FOLL_FORCE);
 
if (copied != sizeof(tmp))
break;
@@ -279,7 +279,7 @@ static int insn_size(struct task_struct *child, unsigned 
long pc)
   int opsize = 0;
 
   /

[PATCH 10/10] mm: replace access_process_vm() write parameter with gup_flags

[Lorenzo Stoakes]

This patch removes the write parameter from access_process_vm() and replaces it
with a gup_flags parameter as use of this function previously _implied_
FOLL_FORCE, whereas after this patch callers explicitly pass this flag.

We make this explicit as use of FOLL_FORCE can result in surprising behaviour
(and hence bugs) within the mm subsystem.

Signed-off-by: Lorenzo Stoakes 
---
 arch/alpha/kernel/ptrace.c |  9 ++---
 arch/blackfin/kernel/ptrace.c  |  5 +++--
 arch/cris/arch-v32/kernel/ptrace.c |  4 ++--
 arch/ia64/kernel/ptrace.c  | 14 +-
 arch/m32r/kernel/ptrace.c  | 15 ++-
 arch/mips/kernel/ptrace32.c|  5 +++--
 arch/powerpc/kernel/ptrace32.c |  5 +++--
 arch/score/kernel/ptrace.c | 10 ++
 arch/sparc/kernel/ptrace_64.c  | 24 
 arch/x86/kernel/step.c |  3 ++-
 arch/x86/um/ptrace_32.c|  3 ++-
 arch/x86/um/ptrace_64.c|  3 ++-
 include/linux/mm.h |  3 ++-
 kernel/ptrace.c| 16 ++--
 mm/memory.c|  8 ++--
 mm/nommu.c |  6 +++---
 mm/util.c  |  5 +++--
 17 files changed, 84 insertions(+), 54 deletions(-)

diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
index d9ee817..940dfb4 100644
--- a/arch/alpha/kernel/ptrace.c
+++ b/arch/alpha/kernel/ptrace.c
@@ -157,14 +157,16 @@ put_reg(struct task_struct *task, unsigned long regno, 
unsigned long data)
 static inline int
 read_int(struct task_struct *task, unsigned long addr, int * data)
 {
-   int copied = access_process_vm(task, addr, data, sizeof(int), 0);
+   int copied = access_process_vm(task, addr, data, sizeof(int),
+   FOLL_FORCE);
return (copied == sizeof(int)) ? 0 : -EIO;
 }
 
 static inline int
 write_int(struct task_struct *task, unsigned long addr, int data)
 {
-   int copied = access_process_vm(task, addr, , sizeof(int), 1);
+   int copied = access_process_vm(task, addr, , sizeof(int),
+   FOLL_FORCE | FOLL_WRITE);
return (copied == sizeof(int)) ? 0 : -EIO;
 }
 
@@ -281,7 +283,8 @@ long arch_ptrace(struct task_struct *child, long request,
/* When I and D space are separate, these will need to be fixed.  */
case PTRACE_PEEKTEXT: /* read word at location addr. */
case PTRACE_PEEKDATA:
-   copied = access_process_vm(child, addr, , sizeof(tmp), 0);
+   copied = access_process_vm(child, addr, , sizeof(tmp),
+   FOLL_FORCE);
ret = -EIO;
if (copied != sizeof(tmp))
break;
diff --git a/arch/blackfin/kernel/ptrace.c b/arch/blackfin/kernel/ptrace.c
index 8b8fe67..8d79286 100644
--- a/arch/blackfin/kernel/ptrace.c
+++ b/arch/blackfin/kernel/ptrace.c
@@ -271,7 +271,7 @@ long arch_ptrace(struct task_struct *child, long request,
case BFIN_MEM_ACCESS_CORE:
case BFIN_MEM_ACCESS_CORE_ONLY:
copied = access_process_vm(child, addr, ,
-  to_copy, 0);
+  to_copy, FOLL_FORCE);
if (copied)
break;
 
@@ -324,7 +324,8 @@ long arch_ptrace(struct task_struct *child, long request,
case BFIN_MEM_ACCESS_CORE:
case BFIN_MEM_ACCESS_CORE_ONLY:
copied = access_process_vm(child, addr, ,
-  to_copy, 1);
+  to_copy,
+  FOLL_FORCE | 
FOLL_WRITE);
break;
case BFIN_MEM_ACCESS_DMA:
if (safe_dma_memcpy(paddr, , to_copy))
diff --git a/arch/cris/arch-v32/kernel/ptrace.c 
b/arch/cris/arch-v32/kernel/ptrace.c
index f085229..f0df654 100644
--- a/arch/cris/arch-v32/kernel/ptrace.c
+++ b/arch/cris/arch-v32/kernel/ptrace.c
@@ -147,7 +147,7 @@ long arch_ptrace(struct task_struct *child, long request,
/* The trampoline page is globally mapped, no 
page table to traverse.*/
tmp = *(unsigned long*)addr;
} else {
-   copied = access_process_vm(child, addr, , 
sizeof(tmp), 0);
+   copied = access_process_vm(child, addr, , 
sizeof(tmp), FOLL_FORCE);
 
if (copied != sizeof(tmp))
break;
@@ -279,7 +279,7 @@ static int insn_size(struct task_struct *child, unsigned 
long pc)
   int opsize = 0;
 
   /* Read the opcode at pc (do what

[PATCH 06/10] mm: replace get_user_pages() write/force parameters with gup_flags

[Lorenzo Stoakes]

This patch removes the write and force parameters from get_user_pages() and
replaces them with a gup_flags parameter 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 <lstoa...@gmail.com>
---
 arch/cris/arch-v32/drivers/cryptocop.c |  4 +---
 arch/ia64/kernel/err_inject.c  |  2 +-
 arch/x86/mm/mpx.c  |  5 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  7 +--
 drivers/gpu/drm/radeon/radeon_ttm.c|  3 ++-
 drivers/gpu/drm/via/via_dmablit.c  |  4 ++--
 drivers/infiniband/core/umem.c |  6 +-
 drivers/infiniband/hw/mthca/mthca_memfree.c|  2 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |  3 ++-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |  5 -
 drivers/media/v4l2-core/videobuf-dma-sg.c  |  7 +--
 drivers/misc/mic/scif/scif_rma.c   |  3 +--
 drivers/misc/sgi-gru/grufault.c|  2 +-
 drivers/platform/goldfish/goldfish_pipe.c  |  3 ++-
 drivers/rapidio/devices/rio_mport_cdev.c   |  3 ++-
 .../vc04_services/interface/vchiq_arm/vchiq_2835_arm.c |  3 +--
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |  3 +--
 drivers/virt/fsl_hypervisor.c  |  4 ++--
 include/linux/mm.h |  2 +-
 mm/gup.c   | 12 +++-
 mm/mempolicy.c |  2 +-
 mm/nommu.c | 18 --
 22 files changed, 49 insertions(+), 54 deletions(-)

diff --git a/arch/cris/arch-v32/drivers/cryptocop.c 
b/arch/cris/arch-v32/drivers/cryptocop.c
index b5698c8..099e170 100644
--- a/arch/cris/arch-v32/drivers/cryptocop.c
+++ b/arch/cris/arch-v32/drivers/cryptocop.c
@@ -2722,7 +2722,6 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
err = get_user_pages((unsigned long int)(oper.indata + prev_ix),
 noinpages,
 0,  /* read access only for in data */
-0, /* no force */
 inpages,
 NULL);
 
@@ -2736,8 +2735,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
if (oper.do_cipher){
err = get_user_pages((unsigned long int)oper.cipher_outdata,
 nooutpages,
-1, /* write access for out data */
-0, /* no force */
+FOLL_WRITE, /* write access for out data */
 outpages,
 NULL);
up_read(>mm->mmap_sem);
diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
index 09f8457..5ed0ea9 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, VM_READ, 0, NULL, NULL);
+   ret = get_user_pages(virt_addr, 1, FOLL_WRITE, NULL, NULL);
if (ret<=0) {
 #ifdef ERR_INJ_DEBUG
printk("Virtual address %lx is not existing.\n",virt_addr);
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index 8047687..e4f8009 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -544,10 +544,9 @@ static int mpx_resolve_fault(long __user *addr, int write)
 {
long gup_ret;
int nr_pages = 1;
-   int force = 0;
 
-   gup_ret = get_user_pages((unsigned long)addr, nr_pages, write,
-   force, NULL, NULL);
+   gup_ret = get_user_pages((unsigned long)addr, nr_pages,
+   write ? FOLL_WRITE : 0, NULL, NULL);
/*
 * get_user_pages() returns number of pages gotten.
 * 0 means we failed to fault in and get anything,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 887483b..dcaf691 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -555,10 +555,13 @@ struct amdgpu_ttm_tt {
 int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
 {
struct amdgpu_ttm_tt *gtt = (void *)ttm;
-   int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
+   unsigned int flags = 0;
unsigned pinned = 0;
int r;
 
+   if (!(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY))
+   flags |= F

[PATCH 06/10] mm: replace get_user_pages() write/force parameters with gup_flags

[Lorenzo Stoakes]

This patch removes the write and force parameters from get_user_pages() and
replaces them with a gup_flags parameter 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 
---
 arch/cris/arch-v32/drivers/cryptocop.c |  4 +---
 arch/ia64/kernel/err_inject.c  |  2 +-
 arch/x86/mm/mpx.c  |  5 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  7 +--
 drivers/gpu/drm/radeon/radeon_ttm.c|  3 ++-
 drivers/gpu/drm/via/via_dmablit.c  |  4 ++--
 drivers/infiniband/core/umem.c |  6 +-
 drivers/infiniband/hw/mthca/mthca_memfree.c|  2 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |  3 ++-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |  5 -
 drivers/media/v4l2-core/videobuf-dma-sg.c  |  7 +--
 drivers/misc/mic/scif/scif_rma.c   |  3 +--
 drivers/misc/sgi-gru/grufault.c|  2 +-
 drivers/platform/goldfish/goldfish_pipe.c  |  3 ++-
 drivers/rapidio/devices/rio_mport_cdev.c   |  3 ++-
 .../vc04_services/interface/vchiq_arm/vchiq_2835_arm.c |  3 +--
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |  3 +--
 drivers/virt/fsl_hypervisor.c  |  4 ++--
 include/linux/mm.h |  2 +-
 mm/gup.c   | 12 +++-
 mm/mempolicy.c |  2 +-
 mm/nommu.c | 18 --
 22 files changed, 49 insertions(+), 54 deletions(-)

diff --git a/arch/cris/arch-v32/drivers/cryptocop.c 
b/arch/cris/arch-v32/drivers/cryptocop.c
index b5698c8..099e170 100644
--- a/arch/cris/arch-v32/drivers/cryptocop.c
+++ b/arch/cris/arch-v32/drivers/cryptocop.c
@@ -2722,7 +2722,6 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
err = get_user_pages((unsigned long int)(oper.indata + prev_ix),
 noinpages,
 0,  /* read access only for in data */
-0, /* no force */
 inpages,
 NULL);
 
@@ -2736,8 +2735,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
if (oper.do_cipher){
err = get_user_pages((unsigned long int)oper.cipher_outdata,
 nooutpages,
-1, /* write access for out data */
-0, /* no force */
+FOLL_WRITE, /* write access for out data */
 outpages,
 NULL);
up_read(>mm->mmap_sem);
diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
index 09f8457..5ed0ea9 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, VM_READ, 0, NULL, NULL);
+   ret = get_user_pages(virt_addr, 1, FOLL_WRITE, NULL, NULL);
if (ret<=0) {
 #ifdef ERR_INJ_DEBUG
printk("Virtual address %lx is not existing.\n",virt_addr);
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index 8047687..e4f8009 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -544,10 +544,9 @@ static int mpx_resolve_fault(long __user *addr, int write)
 {
long gup_ret;
int nr_pages = 1;
-   int force = 0;
 
-   gup_ret = get_user_pages((unsigned long)addr, nr_pages, write,
-   force, NULL, NULL);
+   gup_ret = get_user_pages((unsigned long)addr, nr_pages,
+   write ? FOLL_WRITE : 0, NULL, NULL);
/*
 * get_user_pages() returns number of pages gotten.
 * 0 means we failed to fault in and get anything,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 887483b..dcaf691 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -555,10 +555,13 @@ struct amdgpu_ttm_tt {
 int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
 {
struct amdgpu_ttm_tt *gtt = (void *)ttm;
-   int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
+   unsigned int flags = 0;
unsigned pinned = 0;
int r;
 
+   if (!(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY))
+   flags |= FOLL_WRITE;
+
if (gtt->use

[PATCH 05/10] mm: replace get_vaddr_frames() write/force parameters with gup_flags

[Lorenzo Stoakes]

This patch removes the write and force parameters from get_vaddr_frames() and
replaces them with a gup_flags parameter 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 <lstoa...@gmail.com>
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c|  3 ++-
 drivers/media/platform/omap/omap_vout.c|  2 +-
 drivers/media/v4l2-core/videobuf2-memops.c |  6 +-
 include/linux/mm.h |  2 +-
 mm/frame_vector.c  | 13 ++---
 5 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index aa92dec..fbd13fa 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -488,7 +488,8 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct 
drm_device *drm_dev,
goto err_free;
}
 
-   ret = get_vaddr_frames(start, npages, true, true, g2d_userptr->vec);
+   ret = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
+   g2d_userptr->vec);
if (ret != npages) {
DRM_ERROR("failed to get user pages from userptr.\n");
if (ret < 0)
diff --git a/drivers/media/platform/omap/omap_vout.c 
b/drivers/media/platform/omap/omap_vout.c
index e668dde..a31b95c 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -214,7 +214,7 @@ static int omap_vout_get_userptr(struct videobuf_buffer 
*vb, u32 virtp,
if (!vec)
return -ENOMEM;
 
-   ret = get_vaddr_frames(virtp, 1, true, false, vec);
+   ret = get_vaddr_frames(virtp, 1, FOLL_WRITE, vec);
if (ret != 1) {
frame_vector_destroy(vec);
return -EINVAL;
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c 
b/drivers/media/v4l2-core/videobuf2-memops.c
index 3c3b517..1cd322e 100644
--- a/drivers/media/v4l2-core/videobuf2-memops.c
+++ b/drivers/media/v4l2-core/videobuf2-memops.c
@@ -42,6 +42,10 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
unsigned long first, last;
unsigned long nr;
struct frame_vector *vec;
+   unsigned int flags = FOLL_FORCE;
+
+   if (write)
+   flags |= FOLL_WRITE;
 
first = start >> PAGE_SHIFT;
last = (start + length - 1) >> PAGE_SHIFT;
@@ -49,7 +53,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
vec = frame_vector_create(nr);
if (!vec)
return ERR_PTR(-ENOMEM);
-   ret = get_vaddr_frames(start & PAGE_MASK, nr, write, true, vec);
+   ret = get_vaddr_frames(start & PAGE_MASK, nr, flags, vec);
if (ret < 0)
goto out_destroy;
/* We accept only complete set of PFNs */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ab538..5ff084f6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1305,7 +1305,7 @@ struct frame_vector {
 struct frame_vector *frame_vector_create(unsigned int nr_frames);
 void frame_vector_destroy(struct frame_vector *vec);
 int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
-bool write, bool force, struct frame_vector *vec);
+unsigned int gup_flags, struct frame_vector *vec);
 void put_vaddr_frames(struct frame_vector *vec);
 int frame_vector_to_pages(struct frame_vector *vec);
 void frame_vector_to_pfns(struct frame_vector *vec);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 81b6749..db77dcb 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -11,10 +11,7 @@
  * get_vaddr_frames() - map virtual addresses to pfns
  * @start: starting user address
  * @nr_frames: number of pages / pfns from start to map
- * @write: whether pages will be written to by the caller
- * @force: whether to force write access even if user mapping is
- * readonly. See description of the same argument of
-   get_user_pages().
+ * @gup_flags: flags modifying lookup behaviour
  * @vec:   structure which receives pages / pfns of the addresses mapped.
  * It should have space for at least nr_frames entries.
  *
@@ -34,23 +31,17 @@
  * This function takes care of grabbing mmap_sem as necessary.
  */
 int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
-bool write, bool force, struct frame_vector *vec)
+unsigned int gup_flags, struct frame_vector *vec)
 {
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
int ret = 0;
int err;
int locked;
-   unsigned int gup_flags = 0;
 
if (nr_frames == 0)
return 0;
 
-   if (write)
-   gup_flags |= FOLL_WRITE;
-   if (force)
-   

[PATCH 05/10] mm: replace get_vaddr_frames() write/force parameters with gup_flags

[Lorenzo Stoakes]

This patch removes the write and force parameters from get_vaddr_frames() and
replaces them with a gup_flags parameter 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 
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c|  3 ++-
 drivers/media/platform/omap/omap_vout.c|  2 +-
 drivers/media/v4l2-core/videobuf2-memops.c |  6 +-
 include/linux/mm.h |  2 +-
 mm/frame_vector.c  | 13 ++---
 5 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index aa92dec..fbd13fa 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -488,7 +488,8 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct 
drm_device *drm_dev,
goto err_free;
}
 
-   ret = get_vaddr_frames(start, npages, true, true, g2d_userptr->vec);
+   ret = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
+   g2d_userptr->vec);
if (ret != npages) {
DRM_ERROR("failed to get user pages from userptr.\n");
if (ret < 0)
diff --git a/drivers/media/platform/omap/omap_vout.c 
b/drivers/media/platform/omap/omap_vout.c
index e668dde..a31b95c 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -214,7 +214,7 @@ static int omap_vout_get_userptr(struct videobuf_buffer 
*vb, u32 virtp,
if (!vec)
return -ENOMEM;
 
-   ret = get_vaddr_frames(virtp, 1, true, false, vec);
+   ret = get_vaddr_frames(virtp, 1, FOLL_WRITE, vec);
if (ret != 1) {
frame_vector_destroy(vec);
return -EINVAL;
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c 
b/drivers/media/v4l2-core/videobuf2-memops.c
index 3c3b517..1cd322e 100644
--- a/drivers/media/v4l2-core/videobuf2-memops.c
+++ b/drivers/media/v4l2-core/videobuf2-memops.c
@@ -42,6 +42,10 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
unsigned long first, last;
unsigned long nr;
struct frame_vector *vec;
+   unsigned int flags = FOLL_FORCE;
+
+   if (write)
+   flags |= FOLL_WRITE;
 
first = start >> PAGE_SHIFT;
last = (start + length - 1) >> PAGE_SHIFT;
@@ -49,7 +53,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
vec = frame_vector_create(nr);
if (!vec)
return ERR_PTR(-ENOMEM);
-   ret = get_vaddr_frames(start & PAGE_MASK, nr, write, true, vec);
+   ret = get_vaddr_frames(start & PAGE_MASK, nr, flags, vec);
if (ret < 0)
goto out_destroy;
/* We accept only complete set of PFNs */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ab538..5ff084f6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1305,7 +1305,7 @@ struct frame_vector {
 struct frame_vector *frame_vector_create(unsigned int nr_frames);
 void frame_vector_destroy(struct frame_vector *vec);
 int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
-bool write, bool force, struct frame_vector *vec);
+unsigned int gup_flags, struct frame_vector *vec);
 void put_vaddr_frames(struct frame_vector *vec);
 int frame_vector_to_pages(struct frame_vector *vec);
 void frame_vector_to_pfns(struct frame_vector *vec);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 81b6749..db77dcb 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -11,10 +11,7 @@
  * get_vaddr_frames() - map virtual addresses to pfns
  * @start: starting user address
  * @nr_frames: number of pages / pfns from start to map
- * @write: whether pages will be written to by the caller
- * @force: whether to force write access even if user mapping is
- * readonly. See description of the same argument of
-   get_user_pages().
+ * @gup_flags: flags modifying lookup behaviour
  * @vec:   structure which receives pages / pfns of the addresses mapped.
  * It should have space for at least nr_frames entries.
  *
@@ -34,23 +31,17 @@
  * This function takes care of grabbing mmap_sem as necessary.
  */
 int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
-bool write, bool force, struct frame_vector *vec)
+unsigned int gup_flags, struct frame_vector *vec)
 {
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
int ret = 0;
int err;
int locked;
-   unsigned int gup_flags = 0;
 
if (nr_frames == 0)
return 0;
 
-   if (write)
-   gup_flags |= FOLL_WRITE;
-   if (force)
-   gup_flags |= FOLL_FORC

[PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

This patch removes the write parameter from __access_remote_vm() and replaces it
with a gup_flags parameter as use of this function previously _implied_
FOLL_FORCE, whereas after this patch callers explicitly pass this flag.

We make this explicit as use of FOLL_FORCE can result in surprising behaviour
(and hence bugs) within the mm subsystem.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 mm/memory.c | 23 +++
 mm/nommu.c  |  9 ++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 20a9adb..79ebed3 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3869,14 +3869,11 @@ EXPORT_SYMBOL_GPL(generic_access_phys);
  * given task for page fault accounting.
  */
 static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
-   unsigned long addr, void *buf, int len, int write)
+   unsigned long addr, void *buf, int len, unsigned int gup_flags)
 {
struct vm_area_struct *vma;
void *old_buf = buf;
-   unsigned int flags = FOLL_FORCE;
-
-   if (write)
-   flags |= FOLL_WRITE;
+   int write = gup_flags & FOLL_WRITE;
 
down_read(>mmap_sem);
/* ignore errors, just check how much was successfully transferred */
@@ -3886,7 +3883,7 @@ static int __access_remote_vm(struct task_struct *tsk, 
struct mm_struct *mm,
struct page *page = NULL;
 
ret = get_user_pages_remote(tsk, mm, addr, 1,
-   flags, , );
+   gup_flags, , );
if (ret <= 0) {
 #ifndef CONFIG_HAVE_IOREMAP_PROT
break;
@@ -3945,7 +3942,12 @@ static int __access_remote_vm(struct task_struct *tsk, 
struct mm_struct *mm,
 int access_remote_vm(struct mm_struct *mm, unsigned long addr,
void *buf, int len, int write)
 {
-   return __access_remote_vm(NULL, mm, addr, buf, len, write);
+   unsigned int flags = FOLL_FORCE;
+
+   if (write)
+   flags |= FOLL_WRITE;
+
+   return __access_remote_vm(NULL, mm, addr, buf, len, flags);
 }
 
 /*
@@ -3958,12 +3960,17 @@ int access_process_vm(struct task_struct *tsk, unsigned 
long addr,
 {
struct mm_struct *mm;
int ret;
+   unsigned int flags = FOLL_FORCE;
 
mm = get_task_mm(tsk);
if (!mm)
return 0;
 
-   ret = __access_remote_vm(tsk, mm, addr, buf, len, write);
+   if (write)
+   flags |= FOLL_WRITE;
+
+   ret = __access_remote_vm(tsk, mm, addr, buf, len, flags);
+
mmput(mm);
 
return ret;
diff --git a/mm/nommu.c b/mm/nommu.c
index 70cb844..bde7df3 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1809,9 +1809,10 @@ void filemap_map_pages(struct fault_env *fe,
 EXPORT_SYMBOL(filemap_map_pages);
 
 static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
-   unsigned long addr, void *buf, int len, int write)
+   unsigned long addr, void *buf, int len, unsigned int gup_flags)
 {
struct vm_area_struct *vma;
+   int write = gup_flags & FOLL_WRITE;
 
down_read(>mmap_sem);
 
@@ -1853,7 +1854,8 @@ static int __access_remote_vm(struct task_struct *tsk, 
struct mm_struct *mm,
 int access_remote_vm(struct mm_struct *mm, unsigned long addr,
void *buf, int len, int write)
 {
-   return __access_remote_vm(NULL, mm, addr, buf, len, write);
+   return __access_remote_vm(NULL, mm, addr, buf, len,
+   write ? FOLL_WRITE : 0);
 }
 
 /*
@@ -1871,7 +1873,8 @@ int access_process_vm(struct task_struct *tsk, unsigned 
long addr, void *buf, in
if (!mm)
return 0;
 
-   len = __access_remote_vm(tsk, mm, addr, buf, len, write);
+   len = __access_remote_vm(tsk, mm, addr, buf, len,
+   write ? FOLL_WRITE : 0);
 
mmput(mm);
return len;
-- 
2.10.0

[PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

This patch removes the write parameter from __access_remote_vm() and replaces it
with a gup_flags parameter as use of this function previously _implied_
FOLL_FORCE, whereas after this patch callers explicitly pass this flag.

We make this explicit as use of FOLL_FORCE can result in surprising behaviour
(and hence bugs) within the mm subsystem.

Signed-off-by: Lorenzo Stoakes 
---
 mm/memory.c | 23 +++
 mm/nommu.c  |  9 ++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 20a9adb..79ebed3 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3869,14 +3869,11 @@ EXPORT_SYMBOL_GPL(generic_access_phys);
  * given task for page fault accounting.
  */
 static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
-   unsigned long addr, void *buf, int len, int write)
+   unsigned long addr, void *buf, int len, unsigned int gup_flags)
 {
struct vm_area_struct *vma;
void *old_buf = buf;
-   unsigned int flags = FOLL_FORCE;
-
-   if (write)
-   flags |= FOLL_WRITE;
+   int write = gup_flags & FOLL_WRITE;
 
down_read(>mmap_sem);
/* ignore errors, just check how much was successfully transferred */
@@ -3886,7 +3883,7 @@ static int __access_remote_vm(struct task_struct *tsk, 
struct mm_struct *mm,
struct page *page = NULL;
 
ret = get_user_pages_remote(tsk, mm, addr, 1,
-   flags, , );
+   gup_flags, , );
if (ret <= 0) {
 #ifndef CONFIG_HAVE_IOREMAP_PROT
break;
@@ -3945,7 +3942,12 @@ static int __access_remote_vm(struct task_struct *tsk, 
struct mm_struct *mm,
 int access_remote_vm(struct mm_struct *mm, unsigned long addr,
void *buf, int len, int write)
 {
-   return __access_remote_vm(NULL, mm, addr, buf, len, write);
+   unsigned int flags = FOLL_FORCE;
+
+   if (write)
+   flags |= FOLL_WRITE;
+
+   return __access_remote_vm(NULL, mm, addr, buf, len, flags);
 }
 
 /*
@@ -3958,12 +3960,17 @@ int access_process_vm(struct task_struct *tsk, unsigned 
long addr,
 {
struct mm_struct *mm;
int ret;
+   unsigned int flags = FOLL_FORCE;
 
mm = get_task_mm(tsk);
if (!mm)
return 0;
 
-   ret = __access_remote_vm(tsk, mm, addr, buf, len, write);
+   if (write)
+   flags |= FOLL_WRITE;
+
+   ret = __access_remote_vm(tsk, mm, addr, buf, len, flags);
+
mmput(mm);
 
return ret;
diff --git a/mm/nommu.c b/mm/nommu.c
index 70cb844..bde7df3 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1809,9 +1809,10 @@ void filemap_map_pages(struct fault_env *fe,
 EXPORT_SYMBOL(filemap_map_pages);
 
 static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
-   unsigned long addr, void *buf, int len, int write)
+   unsigned long addr, void *buf, int len, unsigned int gup_flags)
 {
struct vm_area_struct *vma;
+   int write = gup_flags & FOLL_WRITE;
 
down_read(>mmap_sem);
 
@@ -1853,7 +1854,8 @@ static int __access_remote_vm(struct task_struct *tsk, 
struct mm_struct *mm,
 int access_remote_vm(struct mm_struct *mm, unsigned long addr,
void *buf, int len, int write)
 {
-   return __access_remote_vm(NULL, mm, addr, buf, len, write);
+   return __access_remote_vm(NULL, mm, addr, buf, len,
+   write ? FOLL_WRITE : 0);
 }
 
 /*
@@ -1871,7 +1873,8 @@ int access_process_vm(struct task_struct *tsk, unsigned 
long addr, void *buf, in
if (!mm)
return 0;
 
-   len = __access_remote_vm(tsk, mm, addr, buf, len, write);
+   len = __access_remote_vm(tsk, mm, addr, buf, len,
+   write ? FOLL_WRITE : 0);
 
mmput(mm);
return len;
-- 
2.10.0

[PATCH 09/10] mm: replace access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

This patch removes the write parameter from access_remote_vm() and replaces it
with a gup_flags parameter as use of this function previously _implied_
FOLL_FORCE, whereas after this patch callers explicitly pass this flag.

We make this explicit as use of FOLL_FORCE can result in surprising behaviour
(and hence bugs) within the mm subsystem.

Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com>
---
 fs/proc/base.c | 19 +--
 include/linux/mm.h |  2 +-
 mm/memory.c| 11 +++
 mm/nommu.c |  7 +++
 4 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index c2964d8..8e65446 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -252,7 +252,7 @@ static ssize_t proc_pid_cmdline_read(struct file *file, 
char __user *buf,
 * Inherently racy -- command line shares address space
 * with code and data.
 */
-   rv = access_remote_vm(mm, arg_end - 1, , 1, 0);
+   rv = access_remote_vm(mm, arg_end - 1, , 1, FOLL_FORCE);
if (rv <= 0)
goto out_free_page;
 
@@ -270,7 +270,8 @@ static ssize_t proc_pid_cmdline_read(struct file *file, 
char __user *buf,
int nr_read;
 
_count = min3(count, len, PAGE_SIZE);
-   nr_read = access_remote_vm(mm, p, page, _count, 0);
+   nr_read = access_remote_vm(mm, p, page, _count,
+   FOLL_FORCE);
if (nr_read < 0)
rv = nr_read;
if (nr_read <= 0)
@@ -305,7 +306,8 @@ static ssize_t proc_pid_cmdline_read(struct file *file, 
char __user *buf,
bool final;
 
_count = min3(count, len, PAGE_SIZE);
-   nr_read = access_remote_vm(mm, p, page, _count, 0);
+   nr_read = access_remote_vm(mm, p, page, _count,
+   FOLL_FORCE);
if (nr_read < 0)
rv = nr_read;
if (nr_read <= 0)
@@ -354,7 +356,8 @@ static ssize_t proc_pid_cmdline_read(struct file *file, 
char __user *buf,
bool final;
 
_count = min3(count, len, PAGE_SIZE);
-   nr_read = access_remote_vm(mm, p, page, _count, 0);
+   nr_read = access_remote_vm(mm, p, page, _count,
+   FOLL_FORCE);
if (nr_read < 0)
rv = nr_read;
if (nr_read <= 0)
@@ -832,6 +835,7 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
unsigned long addr = *ppos;
ssize_t copied;
char *page;
+   unsigned int flags = FOLL_FORCE;
 
if (!mm)
return 0;
@@ -844,6 +848,9 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
if (!atomic_inc_not_zero(>mm_users))
goto free;
 
+   if (write)
+   flags |= FOLL_WRITE;
+
while (count > 0) {
int this_len = min_t(int, count, PAGE_SIZE);
 
@@ -852,7 +859,7 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
break;
}
 
-   this_len = access_remote_vm(mm, addr, page, this_len, write);
+   this_len = access_remote_vm(mm, addr, page, this_len, flags);
if (!this_len) {
if (!copied)
copied = -EIO;
@@ -965,7 +972,7 @@ static ssize_t environ_read(struct file *file, char __user 
*buf,
this_len = min(max_len, this_len);
 
retval = access_remote_vm(mm, (env_start + src),
-   page, this_len, 0);
+   page, this_len, FOLL_FORCE);
 
if (retval <= 0) {
ret = retval;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 2a481d3..3e5234e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1268,7 +1268,7 @@ static inline int fixup_user_fault(struct task_struct 
*tsk,
 
 extern int access_process_vm(struct task_struct *tsk, unsigned long addr, void 
*buf, int len, int write);
 extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
-   void *buf, int len, int write);
+   void *buf, int len, unsigned int gup_flags);
 
 long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
  unsigned long start, unsigned long nr_pages,
diff --git a/mm/memory.c b/mm/memory.c
index 79ebed3..bac2d99 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3935,19 +3935,14 @@ static int __access_remote_vm(struct task_struct *tsk, 
struct mm_struct *mm,
  * @addr:  start address to access
  * @buf:   source or destination buffer
  * @len:   nu

[PATCH 09/10] mm: replace access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

This patch removes the write parameter from access_remote_vm() and replaces it
with a gup_flags parameter as use of this function previously _implied_
FOLL_FORCE, whereas after this patch callers explicitly pass this flag.

We make this explicit as use of FOLL_FORCE can result in surprising behaviour
(and hence bugs) within the mm subsystem.

Signed-off-by: Lorenzo Stoakes 
---
 fs/proc/base.c | 19 +--
 include/linux/mm.h |  2 +-
 mm/memory.c| 11 +++
 mm/nommu.c |  7 +++
 4 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index c2964d8..8e65446 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -252,7 +252,7 @@ static ssize_t proc_pid_cmdline_read(struct file *file, 
char __user *buf,
 * Inherently racy -- command line shares address space
 * with code and data.
 */
-   rv = access_remote_vm(mm, arg_end - 1, , 1, 0);
+   rv = access_remote_vm(mm, arg_end - 1, , 1, FOLL_FORCE);
if (rv <= 0)
goto out_free_page;
 
@@ -270,7 +270,8 @@ static ssize_t proc_pid_cmdline_read(struct file *file, 
char __user *buf,
int nr_read;
 
_count = min3(count, len, PAGE_SIZE);
-   nr_read = access_remote_vm(mm, p, page, _count, 0);
+   nr_read = access_remote_vm(mm, p, page, _count,
+   FOLL_FORCE);
if (nr_read < 0)
rv = nr_read;
if (nr_read <= 0)
@@ -305,7 +306,8 @@ static ssize_t proc_pid_cmdline_read(struct file *file, 
char __user *buf,
bool final;
 
_count = min3(count, len, PAGE_SIZE);
-   nr_read = access_remote_vm(mm, p, page, _count, 0);
+   nr_read = access_remote_vm(mm, p, page, _count,
+   FOLL_FORCE);
if (nr_read < 0)
rv = nr_read;
if (nr_read <= 0)
@@ -354,7 +356,8 @@ static ssize_t proc_pid_cmdline_read(struct file *file, 
char __user *buf,
bool final;
 
_count = min3(count, len, PAGE_SIZE);
-   nr_read = access_remote_vm(mm, p, page, _count, 0);
+   nr_read = access_remote_vm(mm, p, page, _count,
+   FOLL_FORCE);
if (nr_read < 0)
rv = nr_read;
if (nr_read <= 0)
@@ -832,6 +835,7 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
unsigned long addr = *ppos;
ssize_t copied;
char *page;
+   unsigned int flags = FOLL_FORCE;
 
if (!mm)
return 0;
@@ -844,6 +848,9 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
if (!atomic_inc_not_zero(>mm_users))
goto free;
 
+   if (write)
+   flags |= FOLL_WRITE;
+
while (count > 0) {
int this_len = min_t(int, count, PAGE_SIZE);
 
@@ -852,7 +859,7 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
break;
}
 
-   this_len = access_remote_vm(mm, addr, page, this_len, write);
+   this_len = access_remote_vm(mm, addr, page, this_len, flags);
if (!this_len) {
if (!copied)
copied = -EIO;
@@ -965,7 +972,7 @@ static ssize_t environ_read(struct file *file, char __user 
*buf,
this_len = min(max_len, this_len);
 
retval = access_remote_vm(mm, (env_start + src),
-   page, this_len, 0);
+   page, this_len, FOLL_FORCE);
 
if (retval <= 0) {
ret = retval;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 2a481d3..3e5234e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1268,7 +1268,7 @@ static inline int fixup_user_fault(struct task_struct 
*tsk,
 
 extern int access_process_vm(struct task_struct *tsk, unsigned long addr, void 
*buf, int len, int write);
 extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
-   void *buf, int len, int write);
+   void *buf, int len, unsigned int gup_flags);
 
 long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
  unsigned long start, unsigned long nr_pages,
diff --git a/mm/memory.c b/mm/memory.c
index 79ebed3..bac2d99 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3935,19 +3935,14 @@ static int __access_remote_vm(struct task_struct *tsk, 
struct mm_struct *mm,
  * @addr:  start address to access
  * @buf:   source or destination buffer
  * @len:   number of bytes to transfer
- * 

[PATCH 07/10] mm: replace get_user_pages_remote() write/force parameters with gup_flags

[Lorenzo Stoakes]

This patch removes the write and force parameters from get_user_pages_remote()
and replaces them with a gup_flags parameter 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 <lstoa...@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  7 +--
 drivers/gpu/drm/i915/i915_gem_userptr.c |  6 +-
 drivers/infiniband/core/umem_odp.c  |  7 +--
 fs/exec.c   |  9 +++--
 include/linux/mm.h  |  2 +-
 kernel/events/uprobes.c |  6 --
 mm/gup.c| 22 +++---
 mm/memory.c |  6 +-
 security/tomoyo/domain.c|  2 +-
 9 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 5ce3603..0370b84 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -748,19 +748,22 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
int ret = 0, pinned, npages = etnaviv_obj->base.size >> PAGE_SHIFT;
struct page **pvec;
uintptr_t ptr;
+   unsigned int flags = 0;
 
pvec = drm_malloc_ab(npages, sizeof(struct page *));
if (!pvec)
return ERR_PTR(-ENOMEM);
 
+   if (!etnaviv_obj->userptr.ro)
+   flags |= FOLL_WRITE;
+
pinned = 0;
ptr = etnaviv_obj->userptr.ptr;
 
down_read(>mmap_sem);
while (pinned < npages) {
ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
-   !etnaviv_obj->userptr.ro, 0,
-   pvec + pinned, NULL);
+   flags, pvec + pinned, NULL);
if (ret < 0)
break;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index e537930..c6f780f 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -508,6 +508,10 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
pvec = drm_malloc_gfp(npages, sizeof(struct page *), GFP_TEMPORARY);
if (pvec != NULL) {
struct mm_struct *mm = obj->userptr.mm->mm;
+   unsigned int flags = 0;
+
+   if (!obj->userptr.read_only)
+   flags |= FOLL_WRITE;
 
ret = -EFAULT;
if (atomic_inc_not_zero(>mm_users)) {
@@ -517,7 +521,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
(work->task, mm,
 obj->userptr.ptr + pinned * PAGE_SIZE,
 npages - pinned,
-!obj->userptr.read_only, 0,
+flags,
 pvec + pinned, NULL);
if (ret < 0)
break;
diff --git a/drivers/infiniband/core/umem_odp.c 
b/drivers/infiniband/core/umem_odp.c
index 75077a0..1f0fe32 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -527,6 +527,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
user_virt, u64 bcnt,
u64 off;
int j, k, ret = 0, start_idx, npages = 0;
u64 base_virt_addr;
+   unsigned int flags = 0;
 
if (access_mask == 0)
return -EINVAL;
@@ -556,6 +557,9 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
user_virt, u64 bcnt,
goto out_put_task;
}
 
+   if (access_mask & ODP_WRITE_ALLOWED_BIT)
+   flags |= FOLL_WRITE;
+
start_idx = (user_virt - ib_umem_start(umem)) >> PAGE_SHIFT;
k = start_idx;
 
@@ -574,8 +578,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
user_virt, u64 bcnt,
 */
npages = get_user_pages_remote(owning_process, owning_mm,
user_virt, gup_num_pages,
-   access_mask & ODP_WRITE_ALLOWED_BIT,
-   0, local_page_list, NULL);
+   flags, local_page_list, NULL);
up_read(_mm->mmap_sem);
 
if (npages < 0)
diff --git a/fs/exec.c b/fs/exec.c
index 6fcfb3f..4e497b9 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -191,6 +191,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, 
unsigned long pos,
 {
struct page *page;
int ret;
+   unsigned int gup_flags = FOLL_FORCE;
 
 #ifdef CONFIG_STACK_GROWSUP
if (write) {
@@ -199,12 +20