On 10/05/2018 18:15, vinayak menon wrote:

On Tue, Apr 17, 2018 at 8:03 PM, Laurent Dufour
[off-list ref] wrote:

quoted

pte_unmap_same() is making the assumption that the page table are still
around because the mmap_sem is held.
This is no more the case when running a speculative page fault and
additional check must be made to ensure that the final page table are still
there.

This is now done by calling pte_spinlock() to check for the VMA's
consistency while locking for the page tables.

This is requiring passing a vm_fault structure to pte_unmap_same() which is
containing all the needed parameters.

As pte_spinlock() may fail in the case of a speculative page fault, if the
VMA has been touched in our back, pte_unmap_same() should now return 3
cases :
        1. pte are the same (0)
        2. pte are different (VM_FAULT_PTNOTSAME)
        3. a VMA's changes has been detected (VM_FAULT_RETRY)

The case 2 is handled by the introduction of a new VM_FAULT flag named
VM_FAULT_PTNOTSAME which is then trapped in cow_user_page().
If VM_FAULT_RETRY is returned, it is passed up to the callers to retry the
page fault while holding the mmap_sem.

Acked-by: David Rientjes <rientjes@google.com>
Signed-off-by: Laurent Dufour <redacted>
---
 include/linux/mm.h |  1 +
 mm/memory.c        | 39 ++++++++++++++++++++++++++++-----------
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 4d1aff80669c..714da99d77a3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h

@@ -1208,6 +1208,7 @@ static inline void clear_page_pfmemalloc(struct page *page)
 #define VM_FAULT_NEEDDSYNC  0x2000     /* ->fault did not modify page tables
                                         * and needs fsync() to complete (for
                                         * synchronous page faults in DAX) */
+#define VM_FAULT_PTNOTSAME 0x4000      /* Page table entries have changed */

This has to be added to VM_FAULT_RESULT_TRACE ?

Indeed there is no chance that the macro VM_FAULT_RESULT_TRACE would have to
translate that code to a string since VM_FAULT_PTNOTSAME is currently only
returned by pte_unmap_same() and then converted by its only caller
do_swap_page() to return 0. So VM_FAULT_PTNOTSAME is not expected to be seen
outside of these services which are never using VM_FAULT_RESULT_TRACE().

This being said, this may be a good idea to add it in the case of future
potential usage.