On 2/3/22 07:18, Matthew Wilcox wrote:
...
This will also modify the error code returned from follow_page().
Er, but isn't that the whole point of this entire design? It is what
the commit that added it says:
commit 1027e4436b6a5c413c95d95e50d0f26348a602ac
Author: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
Date: Fri Sep 4 15:47:55 2015 -0700
mm: make GUP handle pfn mapping unless FOLL_GET is requested
With DAX, pfn mapping becoming more common. The patch adjusts GUP code to
cover pfn mapping for cases when we don't need struct page to proceed.
To make it possible, let's change follow_page() code to return -EEXIST
error code if proper page table entry exists, but no corresponding struct
page. __get_user_page() would ignore the error code and move to the next
page frame.
The immediate effect of the change is working MAP_POPULATE and mlock() on
DAX mappings.
A quick audit shows that at least the user in mm/migrate.c will
propagate this error code to userspace and I'm not sure the change
in error code will not break something... EEXIST is a bit strange
error code to get from move_pages(2).
That makes sense, maybe move_pages should squash the return codes to
EEXIST?
I think EFAULT is the closest:
This is a zero page or the memory area is not mapped by the
process.
EBUSY implies it can be tried again later.
OK. I definitely need to rework the commit description now, but the diffs are
looking like this:
diff --git a/mm/gup.c b/mm/gup.c
index 65575ae3602f..cad3f28492e3 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -439,10 +439,6 @@ static struct page *no_page_table(struct vm_area_struct *vma,
static int follow_pfn_pte(struct vm_area_struct *vma, unsigned long address,
pte_t *pte, unsigned int flags)
{
- /* No page to get reference */
- if (flags & (FOLL_GET | FOLL_PIN))
- return -EFAULT;
-
if (flags & FOLL_TOUCH) {
pte_t entry = *pte;
@@ -1180,8 +1176,13 @@ static long __get_user_pages(struct mm_struct *mm,
} else if (PTR_ERR(page) == -EEXIST) {
/*
* Proper page table entry exists, but no corresponding
- * struct page.
+ * struct page. If the caller expects **pages to be
+ * filled in, bail out now, because that can't be done
+ * for this page.
*/
+ if (pages)
+ goto out;
+
goto next_page;
} else if (IS_ERR(page)) {
ret = PTR_ERR(page);
diff --git a/mm/migrate.c b/mm/migrate.c
index c7da064b4781..be0d5ae36dc1 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1761,6 +1761,13 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
continue;
}
+ /*
+ * The move_pages() man page does not have an -EEXIST choice, so
+ * use -EFAULT instead.
+ */
+ if (err == -EEXIST)
+ err = -EFAULT;
+
/*
* If the page is already on the target node (!err), store the
* node, otherwise, store the err.
thanks,
--
John Hubbard
NVIDIA
