linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/20] Speculative page faults
@ 2017-09-08 18:06 Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 01/20] mm: Dont assume page-table invariance during faults Laurent Dufour
                   ` (20 more replies)
  0 siblings, 21 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

This is a port on kernel 4.13 of the work done by Peter Zijlstra to
handle page fault without holding the mm semaphore [1].

The idea is to try to handle user space page faults without holding the
mmap_sem. This should allow better concurrency for massively threaded
process since the page fault handler will not wait for other threads memory
layout change to be done, assuming that this change is done in another part
of the process's memory space. This type page fault is named speculative
page fault. If the speculative page fault fails because of a concurrency is
detected or because underlying PMD or PTE tables are not yet allocating, it
is failing its processing and a classic page fault is then tried.

The speculative page fault (SPF) has to look for the VMA matching the fault
address without holding the mmap_sem, so the VMA list is now managed using
SRCU allowing lockless walking. The only impact would be the deferred file
derefencing in the case of a file mapping, since the file pointer is
released once the SRCU cleaning is done.  This patch relies on the change
done recently by Paul McKenney in SRCU which now runs a callback per CPU
instead of per SRCU structure [1].

The VMA's attributes checked during the speculative page fault processing
have to be protected against parallel changes. This is done by using a per
VMA sequence lock. This sequence lock allows the speculative page fault
handler to fast check for parallel changes in progress and to abort the
speculative page fault in that case.

Once the VMA is found, the speculative page fault handler would check for
the VMA's attributes to verify that the page fault has to be handled
correctly or not. Thus the VMA is protected through a sequence lock which
allows fast detection of concurrent VMA changes. If such a change is
detected, the speculative page fault is aborted and a *classic* page fault
is tried.  VMA sequence locks are added when VMA attributes which are
checked during the page fault are modified.

When the PTE is fetched, the VMA is checked to see if it has been changed,
so once the page table is locked, the VMA is valid, so any other changes
leading to touching this PTE will need to lock the page table, so no
parallel change is possible at this time.

Compared to the Peter's initial work, this series introduces a spin_trylock
when dealing with speculative page fault. This is required to avoid dead
lock when handling a page fault while a TLB invalidate is requested by an
other CPU holding the PTE. Another change due to a lock dependency issue
with mapping->i_mmap_rwsem.

In addition some VMA field values which are used once the PTE is unlocked
at the end the page fault path are saved into the vm_fault structure to
used the values matching the VMA at the time the PTE was locked.

This series only support VMA with no vm_ops define, so huge page and mapped
file are not managed with the speculative path. In addition transparent
huge page are not supported. Once this series will be accepted upstream
I'll extend the support to mapped files, and transparent huge pages.

This series builds on top of v4.13.9-mm1 and is functional on x86 and
PowerPC.

Tests have been made using a large commercial in-memory database on a
PowerPC system with 752 CPU using RFC v5 using a previous version of this
series. The results are very encouraging since the loading of the 2TB
database was faster by 14% with the speculative page fault.

Using ebizzy test [3], which spreads a lot of threads, the result are good
when running on both a large or a small system. When using kernbench, the
result are quite similar which expected as not so much multithreaded
processes are involved. But there is no performance degradation neither
which is good.

------------------
Benchmarks results

Note these test have been made on top of 4.13.0-mm1.

Ebizzy:
-------
The test is counting the number of records per second it can manage, the
higher is the best. I run it like this 'ebizzy -mTRp'. To get consistent
result I repeated the test 100 times and measure the average result, mean
deviation, max and min.

- 16 CPUs x86 VM
Records/s	4.13.0-mm1	4.13.0-mm1-spf	delta
Average		13217.90 	65765.94	+397.55%
Mean deviation	690.37		2609.36		+277.97%
Max		16726		77675		+364.40%
Min		12194		616340		+405.45%
		
- 80 CPUs Power 8 node:
Records/s	4.13.0-mm1	4.13.0-mm1-spf	delta
Average		38175.40	67635.55	77.17% 
Mean deviation	600.09	 	2349.66		291.55%
Max		39563		74292		87.78% 
Min		35846		62657		74.79% 

The number of record per second is far better with the speculative page
fault. 
The mean deviation is higher with the speculative page fault, may be
because sometime the fault are not handled in a speculative way leading to
more variation.
The numbers for the x86 guest are really insane for the SPF case, but I
did the test several times and this leads each time this delta. I did again
the test using the previous version of the patch and I got similar
numbers. It happens that the host running the VM is far less loaded now
leading to better results as more threads are eligible to run.
Test on Power are done on a badly balanced node where the memory is only
attached to one core.

Kernbench:
----------
This test is building a 4.12 kernel using platform default config. The
build has been run 5 times each time.

- 16 CPUs x86 VM
Average Half load -j 8 Run (std deviation)
 		 4.13.0-mm1		4.13.0-mm1-spf		delta %
Elapsed Time     145.968 (0.402206)	145.654 (0.533601)	-0.22
User Time        1006.58 (2.74729)	1003.7 (4.11294)	-0.29
System Time      108.464 (0.177567)	111.034 (0.718213)	+2.37
Percent CPU 	 763.4 (1.34164)	764.8 (1.30384)		+0.18
Context Switches 46599.6 (412.013)	63771 (1049.95)		+36.85
Sleeps           85313.2 (514.456)	85532.2 (681.199)	-0.26

Average Optimal load -j 16 Run (std deviation)
 		 4.13.0-mm1		4.13.0-mm1-spf		delta %
Elapsed Time     74.292 (0.75998)	74.484 (0.723035)	+0.26
User Time        959.949 (49.2036)	956.057 (50.2993)	-0.41
System Time      100.203 (8.7119)	101.984 (9.56099)	+1.78
Percent CPU 	 1058 (310.661)		1054.3 (305.263)	-0.35
Context Switches 65713.8 (20161.7)	86619.4 (24095.4)	+31.81
Sleeps           90344.9 (5364.74)	90877.4 (5655.87)	-0.59

The elapsed time are similar, but the impact less important since there are
less multithreaded processes involved here. 

- 80 CPUs Power 8 node:
Average Half load -j 40 Run (std deviation)
		 4.13.0-mm1		4.13.0-mm1-spf		delta %
Elapsed Time 	 115.342 (0.321668)	115.786 (0.427118)	+0.38
User Time 	 4355.08 (10.1778)	4371.77 (14.9715)	+0.38
System Time 	 127.612 (0.882083)	130.048 (1.06258)	+1.91
Percent CPU 	 3885.8 (11.606)	3887.4 (8.04984)	+0.04
Context Switches 80907.8 (657.481)	81936.4 (729.538)	+1.27
Sleeps		 162109 (793.331)	162057 (1414.08)	+0.03

Average Optimal load -j 80 Run (std deviation)
 		 4.13.0-mm1		4.13.0-mm1-spf
Elapsed Time 	 110.308 (0.725445)	109.78 (0.826862)	-0.48
User Time 	 5893.12 (1621.33)	5923.19 (1635.48)	+0.51
System Time 	 162.168 (36.4347)	166.533 (38.4695)	+2.69
Percent CPU 	 5400.2 (1596.89)	5440.4 (1637.71)	+0.74
Context Switches 129372 (51088.2)	144529 (65985.5)	+11.72
Sleeps		 157312 (5113.57)	158696 (4301.48)	-0.87

Here the elapsed time are similar the SPF release, but we remain in the error
margin. It has to be noted that this system is not correctly balanced on
the NUMA point of view as all the available memory is attached to one core.

------------------------
Changes since v2:
 - Perf event is renamed in PERF_COUNT_SW_SPF
 - On Power handle do_page_fault()'s cleaning
 - On Power if the VM_FAULT_ERROR is returned by
 handle_speculative_fault(), do not retry but jump to the error path
 - If VMA's flags are not matching the fault, directly returns
 VM_FAULT_SIGSEGV and not VM_FAULT_RETRY
 - Check for pud_trans_huge() to avoid speculative path
 - Handles _vm_normal_page()'s introduced by 6f16211df3bf
 ("mm/device-public-memory: device memory cache coherent with CPU")
 - add and review few comments in the code
Changes since v1:
 - Remove PERF_COUNT_SW_SPF_FAILED perf event.
 - Add tracing events to details speculative page fault failures.
 - Cache VMA fields values which are used once the PTE is unlocked at the
 end of the page fault events.
 - Ensure that fields read during the speculative path are written and read
 using WRITE_ONCE and READ_ONCE.
 - Add checks at the beginning of the speculative path to abort it if the
 VMA is known to not be supported.
Changes since RFC V5 [5]
 - Port to 4.13 kernel
 - Merging patch fixing lock dependency into the original patch
 - Replace the 2 parameters of vma_has_changed() with the vmf pointer
 - In patch 7, don't call __do_fault() in the speculative path as it may
 want to unlock the mmap_sem.
 - In patch 11-12, don't check for vma boundaries when
 page_add_new_anon_rmap() is called during the spf path and protect against
 anon_vma pointer's update.
 - In patch 13-16, add performance events to report number of successful
 and failed speculative events. 

[1] http://linux-kernel.2935.n7.nabble.com/RFC-PATCH-0-6-Another-go-at-speculative-page-faults-tt965642.html#none
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=da915ad5cf25b5f5d358dd3670c3378d8ae8c03e
[3] http://ebizzy.sourceforge.net/
[4] http://ck.kolivas.org/apps/kernbench/kernbench-0.50/
[5] https://lwn.net/Articles/725607/

Laurent Dufour (14):
  mm: Introduce pte_spinlock for FAULT_FLAG_SPECULATIVE
  mm: Protect VMA modifications using VMA sequence count
  mm: Cache some VMA fields in the vm_fault structure
  mm: Protect SPF handler against anon_vma changes
  mm/migrate: Pass vm_fault pointer to migrate_misplaced_page()
  mm: Introduce __lru_cache_add_active_or_unevictable
  mm: Introduce __maybe_mkwrite()
  mm: Introduce __vm_normal_page()
  mm: Introduce __page_add_new_anon_rmap()
  mm: Try spin lock in speculative path
  mm: Adding speculative page fault failure trace events
  perf: Add a speculative page fault sw event
  perf tools: Add support for the SPF perf event
  powerpc/mm: Add speculative page fault

Peter Zijlstra (6):
  mm: Dont assume page-table invariance during faults
  mm: Prepare for FAULT_FLAG_SPECULATIVE
  mm: VMA sequence count
  mm: RCU free VMAs
  mm: Provide speculative fault infrastructure
  x86/mm: Add speculative pagefault handling

 arch/powerpc/include/asm/book3s/64/pgtable.h |   5 +
 arch/powerpc/mm/fault.c                      |  15 +
 arch/x86/include/asm/pgtable_types.h         |   7 +
 arch/x86/mm/fault.c                          |  19 ++
 fs/proc/task_mmu.c                           |   5 +-
 fs/userfaultfd.c                             |  17 +-
 include/linux/hugetlb_inline.h               |   2 +-
 include/linux/migrate.h                      |   4 +-
 include/linux/mm.h                           |  28 +-
 include/linux/mm_types.h                     |   3 +
 include/linux/pagemap.h                      |   4 +-
 include/linux/rmap.h                         |  12 +-
 include/linux/swap.h                         |  11 +-
 include/trace/events/pagefault.h             |  87 +++++
 include/uapi/linux/perf_event.h              |   1 +
 kernel/fork.c                                |   1 +
 mm/hugetlb.c                                 |   2 +
 mm/init-mm.c                                 |   1 +
 mm/internal.h                                |  19 ++
 mm/khugepaged.c                              |   5 +
 mm/madvise.c                                 |   6 +-
 mm/memory.c                                  | 478 ++++++++++++++++++++++-----
 mm/mempolicy.c                               |  51 ++-
 mm/migrate.c                                 |   4 +-
 mm/mlock.c                                   |  13 +-
 mm/mmap.c                                    | 138 ++++++--
 mm/mprotect.c                                |   4 +-
 mm/mremap.c                                  |   7 +
 mm/rmap.c                                    |   5 +-
 mm/swap.c                                    |  12 +-
 tools/include/uapi/linux/perf_event.h        |   1 +
 tools/perf/util/evsel.c                      |   1 +
 tools/perf/util/parse-events.c               |   4 +
 tools/perf/util/parse-events.l               |   1 +
 tools/perf/util/python.c                     |   1 +
 35 files changed, 796 insertions(+), 178 deletions(-)
 create mode 100644 include/trace/events/pagefault.h

-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 01/20] mm: Dont assume page-table invariance during faults
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 02/20] mm: Prepare for FAULT_FLAG_SPECULATIVE Laurent Dufour
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

From: Peter Zijlstra <peterz@infradead.org>

One of the side effects of speculating on faults (without holding
mmap_sem) is that we can race with free_pgtables() and therefore we
cannot assume the page-tables will stick around.

Remove the reliance on the pte pointer.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 mm/memory.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index ec4e15494901..30bccfa00630 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2270,30 +2270,6 @@ int apply_to_page_range(struct mm_struct *mm, unsigned long addr,
 }
 EXPORT_SYMBOL_GPL(apply_to_page_range);
 
-/*
- * handle_pte_fault chooses page fault handler according to an entry which was
- * read non-atomically.  Before making any commitment, on those architectures
- * or configurations (e.g. i386 with PAE) which might give a mix of unmatched
- * parts, do_swap_page must check under lock before unmapping the pte and
- * proceeding (but do_wp_page is only called after already making such a check;
- * and do_anonymous_page can safely check later on).
- */
-static inline int pte_unmap_same(struct mm_struct *mm, pmd_t *pmd,
-				pte_t *page_table, pte_t orig_pte)
-{
-	int same = 1;
-#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
-	if (sizeof(pte_t) > sizeof(unsigned long)) {
-		spinlock_t *ptl = pte_lockptr(mm, pmd);
-		spin_lock(ptl);
-		same = pte_same(*page_table, orig_pte);
-		spin_unlock(ptl);
-	}
-#endif
-	pte_unmap(page_table);
-	return same;
-}
-
 static inline void cow_user_page(struct page *dst, struct page *src, unsigned long va, struct vm_area_struct *vma)
 {
 	debug_dma_assert_idle(src);
@@ -2854,11 +2830,6 @@ int do_swap_page(struct vm_fault *vmf)
 
 	if (vma_readahead)
 		page = swap_readahead_detect(vmf, &swap_ra);
-	if (!pte_unmap_same(vma->vm_mm, vmf->pmd, vmf->pte, vmf->orig_pte)) {
-		if (page)
-			put_page(page);
-		goto out;
-	}
 
 	entry = pte_to_swp_entry(vmf->orig_pte);
 	if (unlikely(non_swap_entry(entry))) {
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 02/20] mm: Prepare for FAULT_FLAG_SPECULATIVE
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 01/20] mm: Dont assume page-table invariance during faults Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 03/20] mm: Introduce pte_spinlock " Laurent Dufour
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

From: Peter Zijlstra <peterz@infradead.org>

When speculating faults (without holding mmap_sem) we need to validate
that the vma against which we loaded pages is still valid when we're
ready to install the new PTE.

Therefore, replace the pte_offset_map_lock() calls that (re)take the
PTL with pte_map_lock() which can fail in case we find the VMA changed
since we started the fault.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[Port to 4.12 kernel]
[Remove the comment about the fault_env structure which has been
 implemented as the vm_fault structure in the kernel]
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/mm.h |  1 +
 mm/memory.c        | 55 ++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 68be41b31ad0..46e769a5a7ab 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -291,6 +291,7 @@ extern pgprot_t protection_map[16];
 #define FAULT_FLAG_USER		0x40	/* The fault originated in userspace */
 #define FAULT_FLAG_REMOTE	0x80	/* faulting for non current tsk/mm */
 #define FAULT_FLAG_INSTRUCTION  0x100	/* The fault was during an instruction fetch */
+#define FAULT_FLAG_SPECULATIVE	0x200	/* Speculative fault, not holding mmap_sem */
 
 #define FAULT_FLAG_TRACE \
 	{ FAULT_FLAG_WRITE,		"WRITE" }, \
diff --git a/mm/memory.c b/mm/memory.c
index 30bccfa00630..13c8c3c8b5e4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2408,6 +2408,12 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
 	pte_unmap_unlock(vmf->pte, vmf->ptl);
 }
 
+static bool pte_map_lock(struct vm_fault *vmf)
+{
+	vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl);
+	return true;
+}
+
 /*
  * Handle the case of a page which we actually need to copy to a new page.
  *
@@ -2435,6 +2441,7 @@ static int wp_page_copy(struct vm_fault *vmf)
 	const unsigned long mmun_start = vmf->address & PAGE_MASK;
 	const unsigned long mmun_end = mmun_start + PAGE_SIZE;
 	struct mem_cgroup *memcg;
+	int ret = VM_FAULT_OOM;
 
 	if (unlikely(anon_vma_prepare(vma)))
 		goto oom;
@@ -2462,7 +2469,11 @@ static int wp_page_copy(struct vm_fault *vmf)
 	/*
 	 * Re-check the pte - we dropped the lock
 	 */
-	vmf->pte = pte_offset_map_lock(mm, vmf->pmd, vmf->address, &vmf->ptl);
+	if (!pte_map_lock(vmf)) {
+		mem_cgroup_cancel_charge(new_page, memcg, false);
+		ret = VM_FAULT_RETRY;
+		goto oom_free_new;
+	}
 	if (likely(pte_same(*vmf->pte, vmf->orig_pte))) {
 		if (old_page) {
 			if (!PageAnon(old_page)) {
@@ -2550,7 +2561,7 @@ static int wp_page_copy(struct vm_fault *vmf)
 oom:
 	if (old_page)
 		put_page(old_page);
-	return VM_FAULT_OOM;
+	return ret;
 }
 
 /**
@@ -2571,8 +2582,8 @@ static int wp_page_copy(struct vm_fault *vmf)
 int finish_mkwrite_fault(struct vm_fault *vmf)
 {
 	WARN_ON_ONCE(!(vmf->vma->vm_flags & VM_SHARED));
-	vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd, vmf->address,
-				       &vmf->ptl);
+	if (!pte_map_lock(vmf))
+		return VM_FAULT_RETRY;
 	/*
 	 * We might have raced with another page fault while we released the
 	 * pte_offset_map_lock.
@@ -2690,8 +2701,11 @@ static int do_wp_page(struct vm_fault *vmf)
 			get_page(vmf->page);
 			pte_unmap_unlock(vmf->pte, vmf->ptl);
 			lock_page(vmf->page);
-			vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-					vmf->address, &vmf->ptl);
+			if (!pte_map_lock(vmf)) {
+				unlock_page(vmf->page);
+				put_page(vmf->page);
+				return VM_FAULT_RETRY;
+			}
 			if (!pte_same(*vmf->pte, vmf->orig_pte)) {
 				unlock_page(vmf->page);
 				pte_unmap_unlock(vmf->pte, vmf->ptl);
@@ -2868,8 +2882,10 @@ int do_swap_page(struct vm_fault *vmf)
 			 * Back out if somebody else faulted in this pte
 			 * while we released the pte lock.
 			 */
-			vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-					vmf->address, &vmf->ptl);
+			if (!pte_map_lock(vmf)) {
+				delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
+				return VM_FAULT_RETRY;
+			}
 			if (likely(pte_same(*vmf->pte, vmf->orig_pte)))
 				ret = VM_FAULT_OOM;
 			delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
@@ -2925,8 +2941,11 @@ int do_swap_page(struct vm_fault *vmf)
 	/*
 	 * Back out if somebody else already faulted in this pte.
 	 */
-	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
-			&vmf->ptl);
+	if (!pte_map_lock(vmf)) {
+		ret = VM_FAULT_RETRY;
+		mem_cgroup_cancel_charge(page, memcg, false);
+		goto out_page;
+	}
 	if (unlikely(!pte_same(*vmf->pte, vmf->orig_pte)))
 		goto out_nomap;
 
@@ -3053,8 +3072,8 @@ static int do_anonymous_page(struct vm_fault *vmf)
 			!mm_forbids_zeropage(vma->vm_mm)) {
 		entry = pte_mkspecial(pfn_pte(my_zero_pfn(vmf->address),
 						vma->vm_page_prot));
-		vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-				vmf->address, &vmf->ptl);
+		if (!pte_map_lock(vmf))
+			return VM_FAULT_RETRY;
 		if (!pte_none(*vmf->pte))
 			goto unlock;
 		ret = check_stable_address_space(vma->vm_mm);
@@ -3089,8 +3108,11 @@ static int do_anonymous_page(struct vm_fault *vmf)
 	if (vma->vm_flags & VM_WRITE)
 		entry = pte_mkwrite(pte_mkdirty(entry));
 
-	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
-			&vmf->ptl);
+	if (!pte_map_lock(vmf)) {
+		mem_cgroup_cancel_charge(page, memcg, false);
+		put_page(page);
+		return VM_FAULT_RETRY;
+	}
 	if (!pte_none(*vmf->pte))
 		goto release;
 
@@ -3214,8 +3236,9 @@ static int pte_alloc_one_map(struct vm_fault *vmf)
 	 * pte_none() under vmf->ptl protection when we return to
 	 * alloc_set_pte().
 	 */
-	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
-			&vmf->ptl);
+	if (!pte_map_lock(vmf))
+		return VM_FAULT_RETRY;
+
 	return 0;
 }
 
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 03/20] mm: Introduce pte_spinlock for FAULT_FLAG_SPECULATIVE
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 01/20] mm: Dont assume page-table invariance during faults Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 02/20] mm: Prepare for FAULT_FLAG_SPECULATIVE Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 04/20] mm: VMA sequence count Laurent Dufour
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

When handling page fault without holding the mmap_sem the fetch of the
pte lock pointer and the locking will have to be done while ensuring
that the VMA is not touched in our back.

So move the fetch and locking operations in a dedicated function.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 mm/memory.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 13c8c3c8b5e4..530d887ca885 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2408,6 +2408,13 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
 	pte_unmap_unlock(vmf->pte, vmf->ptl);
 }
 
+static bool pte_spinlock(struct vm_fault *vmf)
+{
+	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
+	spin_lock(vmf->ptl);
+	return true;
+}
+
 static bool pte_map_lock(struct vm_fault *vmf)
 {
 	vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl);
@@ -3717,8 +3724,8 @@ static int do_numa_page(struct vm_fault *vmf)
 	 * validation through pte_unmap_same(). It's of NUMA type but
 	 * the pfn may be screwed if the read is non atomic.
 	 */
-	vmf->ptl = pte_lockptr(vma->vm_mm, vmf->pmd);
-	spin_lock(vmf->ptl);
+	if (!pte_spinlock(vmf))
+		return VM_FAULT_RETRY;
 	if (unlikely(!pte_same(*vmf->pte, vmf->orig_pte))) {
 		pte_unmap_unlock(vmf->pte, vmf->ptl);
 		goto out;
@@ -3910,8 +3917,8 @@ static int handle_pte_fault(struct vm_fault *vmf)
 	if (pte_protnone(vmf->orig_pte) && vma_is_accessible(vmf->vma))
 		return do_numa_page(vmf);
 
-	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
-	spin_lock(vmf->ptl);
+	if (!pte_spinlock(vmf))
+		return VM_FAULT_RETRY;
 	entry = vmf->orig_pte;
 	if (unlikely(!pte_same(*vmf->pte, entry)))
 		goto unlock;
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 04/20] mm: VMA sequence count
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (2 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 03/20] mm: Introduce pte_spinlock " Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-13 11:53   ` Sergey Senozhatsky
  2017-09-08 18:06 ` [PATCH v3 05/20] mm: Protect VMA modifications using " Laurent Dufour
                   ` (16 subsequent siblings)
  20 siblings, 1 reply; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

From: Peter Zijlstra <peterz@infradead.org>

Wrap the VMA modifications (vma_adjust/unmap_page_range) with sequence
counts such that we can easily test if a VMA is changed.

The unmap_page_range() one allows us to make assumptions about
page-tables; when we find the seqcount hasn't changed we can assume
page-tables are still valid.

The flip side is that we cannot distinguish between a vma_adjust() and
the unmap_page_range() -- where with the former we could have
re-checked the vma bounds against the address.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[Port to 4.12 kernel]
[Fix lock dependency between mapping->i_mmap_rwsem and vma->vm_sequence]
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/mm_types.h |  1 +
 mm/memory.c              |  2 ++
 mm/mmap.c                | 21 ++++++++++++++++++---
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 46f4ecf5479a..df9a530c8ca1 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -344,6 +344,7 @@ struct vm_area_struct {
 	struct mempolicy *vm_policy;	/* NUMA policy for the VMA */
 #endif
 	struct vm_userfaultfd_ctx vm_userfaultfd_ctx;
+	seqcount_t vm_sequence;
 } __randomize_layout;
 
 struct core_thread {
diff --git a/mm/memory.c b/mm/memory.c
index 530d887ca885..f250e7c92948 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1499,6 +1499,7 @@ void unmap_page_range(struct mmu_gather *tlb,
 	unsigned long next;
 
 	BUG_ON(addr >= end);
+	write_seqcount_begin(&vma->vm_sequence);
 	tlb_start_vma(tlb, vma);
 	pgd = pgd_offset(vma->vm_mm, addr);
 	do {
@@ -1508,6 +1509,7 @@ void unmap_page_range(struct mmu_gather *tlb,
 		next = zap_p4d_range(tlb, vma, pgd, addr, next, details);
 	} while (pgd++, addr = next, addr != end);
 	tlb_end_vma(tlb, vma);
+	write_seqcount_end(&vma->vm_sequence);
 }
 
 
diff --git a/mm/mmap.c b/mm/mmap.c
index 680506faceae..0a0012c7e50c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -558,6 +558,8 @@ void __vma_link_rb(struct mm_struct *mm, struct vm_area_struct *vma,
 	else
 		mm->highest_vm_end = vm_end_gap(vma);
 
+	seqcount_init(&vma->vm_sequence);
+
 	/*
 	 * vma->vm_prev wasn't known when we followed the rbtree to find the
 	 * correct insertion point for that vma. As a result, we could not
@@ -799,6 +801,11 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 		}
 	}
 
+	write_seqcount_begin(&vma->vm_sequence);
+	if (next && next != vma)
+		write_seqcount_begin_nested(&next->vm_sequence,
+					    SINGLE_DEPTH_NESTING);
+
 	anon_vma = vma->anon_vma;
 	if (!anon_vma && adjust_next)
 		anon_vma = next->anon_vma;
@@ -903,6 +910,7 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 		mm->map_count--;
 		mpol_put(vma_policy(next));
 		kmem_cache_free(vm_area_cachep, next);
+		write_seqcount_end(&next->vm_sequence);
 		/*
 		 * In mprotect's case 6 (see comments on vma_merge),
 		 * we must remove another next too. It would clutter
@@ -932,11 +940,14 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 		if (remove_next == 2) {
 			remove_next = 1;
 			end = next->vm_end;
+			write_seqcount_end(&vma->vm_sequence);
 			goto again;
-		}
-		else if (next)
+		} else if (next) {
+			if (next != vma)
+				write_seqcount_begin_nested(&next->vm_sequence,
+							    SINGLE_DEPTH_NESTING);
 			vma_gap_update(next);
-		else {
+		} else {
 			/*
 			 * If remove_next == 2 we obviously can't
 			 * reach this path.
@@ -962,6 +973,10 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 	if (insert && file)
 		uprobe_mmap(insert);
 
+	if (next && next != vma)
+		write_seqcount_end(&next->vm_sequence);
+	write_seqcount_end(&vma->vm_sequence);
+
 	validate_mm(mm);
 
 	return 0;
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 05/20] mm: Protect VMA modifications using VMA sequence count
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (3 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 04/20] mm: VMA sequence count Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 06/20] mm: RCU free VMAs Laurent Dufour
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

The VMA sequence count has been introduced to allow fast detection of
VMA modification when running a page fault handler without holding
the mmap_sem.

This patch provides protection against the VMA modification done in :
	- madvise()
	- mremap()
	- mpol_rebind_policy()
	- vma_replace_policy()
	- change_prot_numa()
	- mlock(), munlock()
	- mprotect()
	- mmap_region()
	- collapse_huge_page()
	- userfaultd registering services

In addition, VMA fields which will be read during the speculative fault
path needs to be written using WRITE_ONCE to prevent write to be split
and intermediate values to be pushed to other CPUs.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 fs/proc/task_mmu.c |  5 ++++-
 fs/userfaultfd.c   | 17 +++++++++++++----
 mm/khugepaged.c    |  3 +++
 mm/madvise.c       |  6 +++++-
 mm/mempolicy.c     | 51 ++++++++++++++++++++++++++++++++++-----------------
 mm/mlock.c         | 13 ++++++++-----
 mm/mmap.c          | 17 ++++++++++-------
 mm/mprotect.c      |  4 +++-
 mm/mremap.c        |  7 +++++++
 9 files changed, 87 insertions(+), 36 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 5589b4bd4b85..550bbc852143 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1152,8 +1152,11 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
 					goto out_mm;
 				}
 				for (vma = mm->mmap; vma; vma = vma->vm_next) {
-					vma->vm_flags &= ~VM_SOFTDIRTY;
+					write_seqcount_begin(&vma->vm_sequence);
+					WRITE_ONCE(vma->vm_flags,
+						   vma->vm_flags & ~VM_SOFTDIRTY);
 					vma_set_page_prot(vma);
+					write_seqcount_end(&vma->vm_sequence);
 				}
 				downgrade_write(&mm->mmap_sem);
 				break;
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index ef4b48d1ea42..856570f327c3 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -634,8 +634,11 @@ int dup_userfaultfd(struct vm_area_struct *vma, struct list_head *fcs)
 
 	octx = vma->vm_userfaultfd_ctx.ctx;
 	if (!octx || !(octx->features & UFFD_FEATURE_EVENT_FORK)) {
+		write_seqcount_begin(&vma->vm_sequence);
 		vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
-		vma->vm_flags &= ~(VM_UFFD_WP | VM_UFFD_MISSING);
+		WRITE_ONCE(vma->vm_flags,
+			   vma->vm_flags & ~(VM_UFFD_WP | VM_UFFD_MISSING));
+		write_seqcount_end(&vma->vm_sequence);
 		return 0;
 	}
 
@@ -860,8 +863,10 @@ static int userfaultfd_release(struct inode *inode, struct file *file)
 			vma = prev;
 		else
 			prev = vma;
-		vma->vm_flags = new_flags;
+		write_seqcount_begin(&vma->vm_sequence);
+		WRITE_ONCE(vma->vm_flags, new_flags);
 		vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
+		write_seqcount_end(&vma->vm_sequence);
 	}
 	up_write(&mm->mmap_sem);
 	mmput(mm);
@@ -1379,8 +1384,10 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
 		 * the next vma was merged into the current one and
 		 * the current one has not been updated yet.
 		 */
-		vma->vm_flags = new_flags;
+		write_seqcount_begin(&vma->vm_sequence);
+		WRITE_ONCE(vma->vm_flags, new_flags);
 		vma->vm_userfaultfd_ctx.ctx = ctx;
+		write_seqcount_end(&vma->vm_sequence);
 
 	skip:
 		prev = vma;
@@ -1537,8 +1544,10 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
 		 * the next vma was merged into the current one and
 		 * the current one has not been updated yet.
 		 */
-		vma->vm_flags = new_flags;
+		write_seqcount_begin(&vma->vm_sequence);
+		WRITE_ONCE(vma->vm_flags, new_flags);
 		vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
+		write_seqcount_end(&vma->vm_sequence);
 
 	skip:
 		prev = vma;
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index c01f177a1120..56dd994c05d0 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1005,6 +1005,7 @@ static void collapse_huge_page(struct mm_struct *mm,
 	if (mm_find_pmd(mm, address) != pmd)
 		goto out;
 
+	write_seqcount_begin(&vma->vm_sequence);
 	anon_vma_lock_write(vma->anon_vma);
 
 	pte = pte_offset_map(pmd, address);
@@ -1040,6 +1041,7 @@ static void collapse_huge_page(struct mm_struct *mm,
 		pmd_populate(mm, pmd, pmd_pgtable(_pmd));
 		spin_unlock(pmd_ptl);
 		anon_vma_unlock_write(vma->anon_vma);
+		write_seqcount_end(&vma->vm_sequence);
 		result = SCAN_FAIL;
 		goto out;
 	}
@@ -1074,6 +1076,7 @@ static void collapse_huge_page(struct mm_struct *mm,
 	set_pmd_at(mm, address, pmd, _pmd);
 	update_mmu_cache_pmd(vma, address, pmd);
 	spin_unlock(pmd_ptl);
+	write_seqcount_end(&vma->vm_sequence);
 
 	*hpage = NULL;
 
diff --git a/mm/madvise.c b/mm/madvise.c
index 21261ff0466f..bedb0ec25c77 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -183,7 +183,9 @@ static long madvise_behavior(struct vm_area_struct *vma,
 	/*
 	 * vm_flags is protected by the mmap_sem held in write mode.
 	 */
-	vma->vm_flags = new_flags;
+	write_seqcount_begin(&vma->vm_sequence);
+	WRITE_ONCE(vma->vm_flags, new_flags);
+	write_seqcount_end(&vma->vm_sequence);
 out:
 	return error;
 }
@@ -451,9 +453,11 @@ static void madvise_free_page_range(struct mmu_gather *tlb,
 		.private = tlb,
 	};
 
+	write_seqcount_begin(&vma->vm_sequence);
 	tlb_start_vma(tlb, vma);
 	walk_page_range(addr, end, &free_walk);
 	tlb_end_vma(tlb, vma);
+	write_seqcount_end(&vma->vm_sequence);
 }
 
 static int madvise_free_single_vma(struct vm_area_struct *vma,
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 006ba625c0b8..ac1096b1be21 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -379,8 +379,11 @@ void mpol_rebind_mm(struct mm_struct *mm, nodemask_t *new)
 	struct vm_area_struct *vma;
 
 	down_write(&mm->mmap_sem);
-	for (vma = mm->mmap; vma; vma = vma->vm_next)
+	for (vma = mm->mmap; vma; vma = vma->vm_next) {
+		write_seqcount_begin(&vma->vm_sequence);
 		mpol_rebind_policy(vma->vm_policy, new);
+		write_seqcount_end(&vma->vm_sequence);
+	}
 	up_write(&mm->mmap_sem);
 }
 
@@ -578,9 +581,11 @@ unsigned long change_prot_numa(struct vm_area_struct *vma,
 {
 	int nr_updated;
 
+	write_seqcount_begin(&vma->vm_sequence);
 	nr_updated = change_protection(vma, addr, end, PAGE_NONE, 0, 1);
 	if (nr_updated)
 		count_vm_numa_events(NUMA_PTE_UPDATES, nr_updated);
+	write_seqcount_end(&vma->vm_sequence);
 
 	return nr_updated;
 }
@@ -681,6 +686,7 @@ static int vma_replace_policy(struct vm_area_struct *vma,
 	if (IS_ERR(new))
 		return PTR_ERR(new);
 
+	write_seqcount_begin(&vma->vm_sequence);
 	if (vma->vm_ops && vma->vm_ops->set_policy) {
 		err = vma->vm_ops->set_policy(vma, new);
 		if (err)
@@ -688,11 +694,17 @@ static int vma_replace_policy(struct vm_area_struct *vma,
 	}
 
 	old = vma->vm_policy;
-	vma->vm_policy = new; /* protected by mmap_sem */
+	/*
+	 * The speculative page fault handler access this field without
+	 * hodling the mmap_sem.
+	 */
+	WRITE_ONCE(vma->vm_policy,  new);
+	write_seqcount_end(&vma->vm_sequence);
 	mpol_put(old);
 
 	return 0;
  err_out:
+	write_seqcount_end(&vma->vm_sequence);
 	mpol_put(new);
 	return err;
 }
@@ -1562,23 +1574,28 @@ COMPAT_SYSCALL_DEFINE6(mbind, compat_ulong_t, start, compat_ulong_t, len,
 struct mempolicy *__get_vma_policy(struct vm_area_struct *vma,
 						unsigned long addr)
 {
-	struct mempolicy *pol = NULL;
+	struct mempolicy *pol;
 
-	if (vma) {
-		if (vma->vm_ops && vma->vm_ops->get_policy) {
-			pol = vma->vm_ops->get_policy(vma, addr);
-		} else if (vma->vm_policy) {
-			pol = vma->vm_policy;
+	if (!vma)
+		return NULL;
 
-			/*
-			 * shmem_alloc_page() passes MPOL_F_SHARED policy with
-			 * a pseudo vma whose vma->vm_ops=NULL. Take a reference
-			 * count on these policies which will be dropped by
-			 * mpol_cond_put() later
-			 */
-			if (mpol_needs_cond_ref(pol))
-				mpol_get(pol);
-		}
+	if (vma->vm_ops && vma->vm_ops->get_policy)
+		return vma->vm_ops->get_policy(vma, addr);
+
+	/*
+	 * This could be called without holding the mmap_sem in the
+	 * speculative page fault handler's path.
+	 */
+	pol = READ_ONCE(vma->vm_policy);
+	if (pol) {
+		/*
+		 * shmem_alloc_page() passes MPOL_F_SHARED policy with
+		 * a pseudo vma whose vma->vm_ops=NULL. Take a reference
+		 * count on these policies which will be dropped by
+		 * mpol_cond_put() later
+		 */
+		if (mpol_needs_cond_ref(pol))
+			mpol_get(pol);
 	}
 
 	return pol;
diff --git a/mm/mlock.c b/mm/mlock.c
index dfc6f1912176..4793a96cbc35 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -438,7 +438,9 @@ static unsigned long __munlock_pagevec_fill(struct pagevec *pvec,
 void munlock_vma_pages_range(struct vm_area_struct *vma,
 			     unsigned long start, unsigned long end)
 {
-	vma->vm_flags &= VM_LOCKED_CLEAR_MASK;
+	write_seqcount_begin(&vma->vm_sequence);
+	WRITE_ONCE(vma->vm_flags, vma->vm_flags & VM_LOCKED_CLEAR_MASK);
+	write_seqcount_end(&vma->vm_sequence);
 
 	while (start < end) {
 		struct page *page;
@@ -561,10 +563,11 @@ static int mlock_fixup(struct vm_area_struct *vma, struct vm_area_struct **prev,
 	 * It's okay if try_to_unmap_one unmaps a page just after we
 	 * set VM_LOCKED, populate_vma_page_range will bring it back.
 	 */
-
-	if (lock)
-		vma->vm_flags = newflags;
-	else
+	if (lock) {
+		write_seqcount_begin(&vma->vm_sequence);
+		WRITE_ONCE(vma->vm_flags, newflags);
+		write_seqcount_end(&vma->vm_sequence);
+	} else
 		munlock_vma_pages_range(vma, start, end);
 
 out:
diff --git a/mm/mmap.c b/mm/mmap.c
index 0a0012c7e50c..04e72314274d 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -826,17 +826,18 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 	}
 
 	if (start != vma->vm_start) {
-		vma->vm_start = start;
+		WRITE_ONCE(vma->vm_start, start);
 		start_changed = true;
 	}
 	if (end != vma->vm_end) {
-		vma->vm_end = end;
+		WRITE_ONCE(vma->vm_end, end);
 		end_changed = true;
 	}
-	vma->vm_pgoff = pgoff;
+	WRITE_ONCE(vma->vm_pgoff, pgoff);
 	if (adjust_next) {
-		next->vm_start += adjust_next << PAGE_SHIFT;
-		next->vm_pgoff += adjust_next;
+		WRITE_ONCE(next->vm_start,
+			   next->vm_start + (adjust_next << PAGE_SHIFT));
+		WRITE_ONCE(next->vm_pgoff, next->vm_pgoff + adjust_next);
 	}
 
 	if (root) {
@@ -1735,6 +1736,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 out:
 	perf_event_mmap(vma);
 
+	write_seqcount_begin(&vma->vm_sequence);
 	vm_stat_account(mm, vm_flags, len >> PAGE_SHIFT);
 	if (vm_flags & VM_LOCKED) {
 		if (!((vm_flags & VM_SPECIAL) || is_vm_hugetlb_page(vma) ||
@@ -1757,6 +1759,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 	vma->vm_flags |= VM_SOFTDIRTY;
 
 	vma_set_page_prot(vma);
+	write_seqcount_end(&vma->vm_sequence);
 
 	return addr;
 
@@ -2385,8 +2388,8 @@ int expand_downwards(struct vm_area_struct *vma,
 					mm->locked_vm += grow;
 				vm_stat_account(mm, vma->vm_flags, grow);
 				anon_vma_interval_tree_pre_update_vma(vma);
-				vma->vm_start = address;
-				vma->vm_pgoff -= grow;
+				WRITE_ONCE(vma->vm_start, address);
+				WRITE_ONCE(vma->vm_pgoff, vma->vm_pgoff - grow);
 				anon_vma_interval_tree_post_update_vma(vma);
 				vma_gap_update(vma);
 				spin_unlock(&mm->page_table_lock);
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 6d3e2f082290..0c9aa0b1a74e 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -358,7 +358,8 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 	 * vm_flags and vm_page_prot are protected by the mmap_sem
 	 * held in write mode.
 	 */
-	vma->vm_flags = newflags;
+	write_seqcount_begin(&vma->vm_sequence);
+	WRITE_ONCE(vma->vm_flags, newflags);
 	dirty_accountable = vma_wants_writenotify(vma, vma->vm_page_prot);
 	vma_set_page_prot(vma);
 
@@ -373,6 +374,7 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 			(newflags & VM_WRITE)) {
 		populate_vma_page_range(vma, start, end, NULL);
 	}
+	write_seqcount_end(&vma->vm_sequence);
 
 	vm_stat_account(mm, oldflags, -nrpages);
 	vm_stat_account(mm, newflags, nrpages);
diff --git a/mm/mremap.c b/mm/mremap.c
index cfec004c4ff9..240618950215 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -301,6 +301,10 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 	if (!new_vma)
 		return -ENOMEM;
 
+	write_seqcount_begin(&vma->vm_sequence);
+	write_seqcount_begin_nested(&new_vma->vm_sequence,
+				    SINGLE_DEPTH_NESTING);
+
 	moved_len = move_page_tables(vma, old_addr, new_vma, new_addr, old_len,
 				     need_rmap_locks);
 	if (moved_len < old_len) {
@@ -317,6 +321,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 		 */
 		move_page_tables(new_vma, new_addr, vma, old_addr, moved_len,
 				 true);
+		write_seqcount_end(&vma->vm_sequence);
 		vma = new_vma;
 		old_len = new_len;
 		old_addr = new_addr;
@@ -325,7 +330,9 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 		mremap_userfaultfd_prep(new_vma, uf);
 		arch_remap(mm, old_addr, old_addr + old_len,
 			   new_addr, new_addr + new_len);
+		write_seqcount_end(&vma->vm_sequence);
 	}
+	write_seqcount_end(&new_vma->vm_sequence);
 
 	/* Conceal VM_ACCOUNT so old reservation is not undone */
 	if (vm_flags & VM_ACCOUNT) {
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 06/20] mm: RCU free VMAs
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (4 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 05/20] mm: Protect VMA modifications using " Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 07/20] mm: Cache some VMA fields in the vm_fault structure Laurent Dufour
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

From: Peter Zijlstra <peterz@infradead.org>

Manage the VMAs with SRCU such that we can do a lockless VMA lookup.

We put the fput(vma->vm_file) in the SRCU callback, this keeps files
valid during speculative faults, this is possible due to the delayed
fput work by Al Viro -- do we need srcu_barrier() in unmount
someplace?

We guard the mm_rb tree with a seqlock (this could be a seqcount but
we'd have to disable preemption around the write side in order to make
the retry loop in __read_seqcount_begin() work) such that we can know
if the rb tree walk was correct. We cannot trust the restult of a
lockless tree walk in the face of concurrent tree rotations; although
we can trust on the termination of such walks -- tree rotations
guarantee the end result is a tree again after all.

Furthermore, we rely on the WMB implied by the
write_seqlock/count_begin() to separate the VMA initialization and the
publishing stores, analogous to the RELEASE in rcu_assign_pointer().
We also rely on the RMB from read_seqretry() to separate the vma load
from further loads like the smp_read_barrier_depends() in regular
RCU.

We must not touch the vmacache while doing SRCU lookups as that is not
properly serialized against changes. We update gap information after
publishing the VMA, but A) we don't use that and B) the seqlock
read side would fix that anyhow.

We clear vma->vm_rb for nodes removed from the vma tree such that we
can easily detect such 'dead' nodes, we rely on the WMB from
write_sequnlock() to separate the tree removal and clearing the node.

Provide find_vma_srcu() which wraps the required magic.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[Remove the warnings in description about the SRCU global lock which
 has been removed now]
[Rename vma_is_dead() to vma_has_changed() and move its adding to the next
 patch]
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/mm_types.h |   2 +
 kernel/fork.c            |   1 +
 mm/init-mm.c             |   1 +
 mm/internal.h            |   5 +++
 mm/mmap.c                | 100 +++++++++++++++++++++++++++++++++++------------
 5 files changed, 83 insertions(+), 26 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index df9a530c8ca1..19ed70f79873 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -345,6 +345,7 @@ struct vm_area_struct {
 #endif
 	struct vm_userfaultfd_ctx vm_userfaultfd_ctx;
 	seqcount_t vm_sequence;
+	struct rcu_head vm_rcu_head;
 } __randomize_layout;
 
 struct core_thread {
@@ -362,6 +363,7 @@ struct kioctx_table;
 struct mm_struct {
 	struct vm_area_struct *mmap;		/* list of VMAs */
 	struct rb_root mm_rb;
+	seqlock_t mm_seq;
 	u32 vmacache_seqnum;                   /* per-thread vmacache */
 #ifdef CONFIG_MMU
 	unsigned long (*get_unmapped_area) (struct file *filp,
diff --git a/kernel/fork.c b/kernel/fork.c
index 59c7a8775dd7..bb8205faf3c4 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -808,6 +808,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
 	mm->mmap = NULL;
 	mm->mm_rb = RB_ROOT;
 	mm->vmacache_seqnum = 0;
+	seqlock_init(&mm->mm_seq);
 	atomic_set(&mm->mm_users, 1);
 	atomic_set(&mm->mm_count, 1);
 	init_rwsem(&mm->mmap_sem);
diff --git a/mm/init-mm.c b/mm/init-mm.c
index 975e49f00f34..2b1fa061684f 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -16,6 +16,7 @@
 
 struct mm_struct init_mm = {
 	.mm_rb		= RB_ROOT,
+	.mm_seq		= __SEQLOCK_UNLOCKED(init_mm.mm_seq),
 	.pgd		= swapper_pg_dir,
 	.mm_users	= ATOMIC_INIT(2),
 	.mm_count	= ATOMIC_INIT(1),
diff --git a/mm/internal.h b/mm/internal.h
index 0aaa05af7833..84360184eafd 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -40,6 +40,11 @@ void page_writeback_init(void);
 
 int do_swap_page(struct vm_fault *vmf);
 
+extern struct srcu_struct vma_srcu;
+
+extern struct vm_area_struct *find_vma_srcu(struct mm_struct *mm,
+					    unsigned long addr);
+
 void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
 		unsigned long floor, unsigned long ceiling);
 
diff --git a/mm/mmap.c b/mm/mmap.c
index 04e72314274d..ad125d8c2e14 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -160,6 +160,23 @@ void unlink_file_vma(struct vm_area_struct *vma)
 	}
 }
 
+DEFINE_SRCU(vma_srcu);
+
+static void __free_vma(struct rcu_head *head)
+{
+	struct vm_area_struct *vma =
+		container_of(head, struct vm_area_struct, vm_rcu_head);
+
+	if (vma->vm_file)
+		fput(vma->vm_file);
+	kmem_cache_free(vm_area_cachep, vma);
+}
+
+static void free_vma(struct vm_area_struct *vma)
+{
+	call_srcu(&vma_srcu, &vma->vm_rcu_head, __free_vma);
+}
+
 /*
  * Close a vm structure and free it, returning the next.
  */
@@ -170,10 +187,8 @@ static struct vm_area_struct *remove_vma(struct vm_area_struct *vma)
 	might_sleep();
 	if (vma->vm_ops && vma->vm_ops->close)
 		vma->vm_ops->close(vma);
-	if (vma->vm_file)
-		fput(vma->vm_file);
 	mpol_put(vma_policy(vma));
-	kmem_cache_free(vm_area_cachep, vma);
+	free_vma(vma);
 	return next;
 }
 
@@ -411,26 +426,37 @@ static void vma_gap_update(struct vm_area_struct *vma)
 }
 
 static inline void vma_rb_insert(struct vm_area_struct *vma,
-				 struct rb_root *root)
+				 struct mm_struct *mm)
 {
+	struct rb_root *root = &mm->mm_rb;
+
 	/* All rb_subtree_gap values must be consistent prior to insertion */
 	validate_mm_rb(root, NULL);
 
 	rb_insert_augmented(&vma->vm_rb, root, &vma_gap_callbacks);
 }
 
-static void __vma_rb_erase(struct vm_area_struct *vma, struct rb_root *root)
+static void __vma_rb_erase(struct vm_area_struct *vma, struct mm_struct *mm)
 {
+	struct rb_root *root = &mm->mm_rb;
 	/*
 	 * Note rb_erase_augmented is a fairly large inline function,
 	 * so make sure we instantiate it only once with our desired
 	 * augmented rbtree callbacks.
 	 */
+	write_seqlock(&mm->mm_seq);
 	rb_erase_augmented(&vma->vm_rb, root, &vma_gap_callbacks);
+	write_sequnlock(&mm->mm_seq); /* wmb */
+
+	/*
+	 * Ensure the removal is complete before clearing the node.
+	 * Matched by vma_has_changed()/handle_speculative_fault().
+	 */
+	RB_CLEAR_NODE(&vma->vm_rb);
 }
 
 static __always_inline void vma_rb_erase_ignore(struct vm_area_struct *vma,
-						struct rb_root *root,
+						struct mm_struct *mm,
 						struct vm_area_struct *ignore)
 {
 	/*
@@ -438,21 +464,21 @@ static __always_inline void vma_rb_erase_ignore(struct vm_area_struct *vma,
 	 * with the possible exception of the "next" vma being erased if
 	 * next->vm_start was reduced.
 	 */
-	validate_mm_rb(root, ignore);
+	validate_mm_rb(&mm->mm_rb, ignore);
 
-	__vma_rb_erase(vma, root);
+	__vma_rb_erase(vma, mm);
 }
 
 static __always_inline void vma_rb_erase(struct vm_area_struct *vma,
-					 struct rb_root *root)
+					 struct mm_struct *mm)
 {
 	/*
 	 * All rb_subtree_gap values must be consistent prior to erase,
 	 * with the possible exception of the vma being erased.
 	 */
-	validate_mm_rb(root, vma);
+	validate_mm_rb(&mm->mm_rb, vma);
 
-	__vma_rb_erase(vma, root);
+	__vma_rb_erase(vma, mm);
 }
 
 /*
@@ -569,10 +595,12 @@ void __vma_link_rb(struct mm_struct *mm, struct vm_area_struct *vma,
 	 * immediately update the gap to the correct value. Finally we
 	 * rebalance the rbtree after all augmented values have been set.
 	 */
+	write_seqlock(&mm->mm_seq);
 	rb_link_node(&vma->vm_rb, rb_parent, rb_link);
 	vma->rb_subtree_gap = 0;
 	vma_gap_update(vma);
-	vma_rb_insert(vma, &mm->mm_rb);
+	vma_rb_insert(vma, mm);
+	write_sequnlock(&mm->mm_seq);
 }
 
 static void __vma_link_file(struct vm_area_struct *vma)
@@ -648,7 +676,7 @@ static __always_inline void __vma_unlink_common(struct mm_struct *mm,
 {
 	struct vm_area_struct *next;
 
-	vma_rb_erase_ignore(vma, &mm->mm_rb, ignore);
+	vma_rb_erase_ignore(vma, mm, ignore);
 	next = vma->vm_next;
 	if (has_prev)
 		prev->vm_next = next;
@@ -902,15 +930,13 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 	}
 
 	if (remove_next) {
-		if (file) {
+		if (file)
 			uprobe_munmap(next, next->vm_start, next->vm_end);
-			fput(file);
-		}
 		if (next->anon_vma)
 			anon_vma_merge(vma, next);
 		mm->map_count--;
 		mpol_put(vma_policy(next));
-		kmem_cache_free(vm_area_cachep, next);
+		free_vma(next);
 		write_seqcount_end(&next->vm_sequence);
 		/*
 		 * In mprotect's case 6 (see comments on vma_merge),
@@ -2131,15 +2157,10 @@ get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
 EXPORT_SYMBOL(get_unmapped_area);
 
 /* Look up the first VMA which satisfies  addr < vm_end,  NULL if none. */
-struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
+static struct vm_area_struct *__find_vma(struct mm_struct *mm, unsigned long addr)
 {
 	struct rb_node *rb_node;
-	struct vm_area_struct *vma;
-
-	/* Check the cache first. */
-	vma = vmacache_find(mm, addr);
-	if (likely(vma))
-		return vma;
+	struct vm_area_struct *vma = NULL;
 
 	rb_node = mm->mm_rb.rb_node;
 
@@ -2157,13 +2178,40 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
 			rb_node = rb_node->rb_right;
 	}
 
+	return vma;
+}
+
+struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
+{
+	struct vm_area_struct *vma;
+
+	/* Check the cache first. */
+	vma = vmacache_find(mm, addr);
+	if (likely(vma))
+		return vma;
+
+	vma = __find_vma(mm, addr);
 	if (vma)
 		vmacache_update(addr, vma);
 	return vma;
 }
-
 EXPORT_SYMBOL(find_vma);
 
+struct vm_area_struct *find_vma_srcu(struct mm_struct *mm, unsigned long addr)
+{
+	struct vm_area_struct *vma;
+	unsigned int seq;
+
+	WARN_ON_ONCE(!srcu_read_lock_held(&vma_srcu));
+
+	do {
+		seq = read_seqbegin(&mm->mm_seq);
+		vma = __find_vma(mm, addr);
+	} while (read_seqretry(&mm->mm_seq, seq));
+
+	return vma;
+}
+
 /*
  * Same as find_vma, but also return a pointer to the previous VMA in *pprev.
  */
@@ -2531,7 +2579,7 @@ detach_vmas_to_be_unmapped(struct mm_struct *mm, struct vm_area_struct *vma,
 	insertion_point = (prev ? &prev->vm_next : &mm->mmap);
 	vma->vm_prev = NULL;
 	do {
-		vma_rb_erase(vma, &mm->mm_rb);
+		vma_rb_erase(vma, mm);
 		mm->map_count--;
 		tail_vma = vma;
 		vma = vma->vm_next;
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 07/20] mm: Cache some VMA fields in the vm_fault structure
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (5 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 06/20] mm: RCU free VMAs Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 08/20] mm: Protect SPF handler against anon_vma changes Laurent Dufour
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

When handling speculative page fault, the vma->vm_flags and
vma->vm_page_prot fields are read once the page table lock is released. So
there is no more guarantee that these fields would not change in our back.
They will be saved in the vm_fault structure before the VMA is checked for
changes.

This patch also set the fields in hugetlb_no_page() and
__collapse_huge_page_swapin even if it is not need for the callee.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/mm.h |  6 ++++++
 mm/hugetlb.c       |  2 ++
 mm/khugepaged.c    |  2 ++
 mm/memory.c        | 38 ++++++++++++++++++++------------------
 4 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 46e769a5a7ab..5fd90ac31317 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -350,6 +350,12 @@ struct vm_fault {
 					 * page table to avoid allocation from
 					 * atomic context.
 					 */
+	/*
+	 * These entries are required when handling speculative page fault.
+	 * This way the page handling is done using consistent field values.
+	 */
+	unsigned long vma_flags;
+	pgprot_t vma_page_prot;
 };
 
 /* page entry size for vm->huge_fault() */
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 424b0ef08a60..da82a86a4761 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3683,6 +3683,8 @@ static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
 				.vma = vma,
 				.address = address,
 				.flags = flags,
+				.vma_flags = vma->vm_flags,
+				.vma_page_prot = vma->vm_page_prot,
 				/*
 				 * Hard to debug if it ends up being
 				 * used by a callee that assumes
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 56dd994c05d0..0525a0e74535 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -881,6 +881,8 @@ static bool __collapse_huge_page_swapin(struct mm_struct *mm,
 		.flags = FAULT_FLAG_ALLOW_RETRY,
 		.pmd = pmd,
 		.pgoff = linear_page_index(vma, address),
+		.vma_flags = vma->vm_flags,
+		.vma_page_prot = vma->vm_page_prot,
 	};
 
 	/* we only decide to swapin, if there is enough young ptes */
diff --git a/mm/memory.c b/mm/memory.c
index f250e7c92948..f008042ab24e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2556,7 +2556,7 @@ static int wp_page_copy(struct vm_fault *vmf)
 		 * Don't let another task, with possibly unlocked vma,
 		 * keep the mlocked page.
 		 */
-		if (page_copied && (vma->vm_flags & VM_LOCKED)) {
+		if (page_copied && (vmf->vma_flags & VM_LOCKED)) {
 			lock_page(old_page);	/* LRU manipulation */
 			if (PageMlocked(old_page))
 				munlock_vma_page(old_page);
@@ -2590,7 +2590,7 @@ static int wp_page_copy(struct vm_fault *vmf)
  */
 int finish_mkwrite_fault(struct vm_fault *vmf)
 {
-	WARN_ON_ONCE(!(vmf->vma->vm_flags & VM_SHARED));
+	WARN_ON_ONCE(!(vmf->vma_flags & VM_SHARED));
 	if (!pte_map_lock(vmf))
 		return VM_FAULT_RETRY;
 	/*
@@ -2692,7 +2692,7 @@ static int do_wp_page(struct vm_fault *vmf)
 		 * We should not cow pages in a shared writeable mapping.
 		 * Just mark the pages writable and/or call ops->pfn_mkwrite.
 		 */
-		if ((vma->vm_flags & (VM_WRITE|VM_SHARED)) ==
+		if ((vmf->vma_flags & (VM_WRITE|VM_SHARED)) ==
 				     (VM_WRITE|VM_SHARED))
 			return wp_pfn_shared(vmf);
 
@@ -2739,7 +2739,7 @@ static int do_wp_page(struct vm_fault *vmf)
 			return VM_FAULT_WRITE;
 		}
 		unlock_page(vmf->page);
-	} else if (unlikely((vma->vm_flags & (VM_WRITE|VM_SHARED)) ==
+	} else if (unlikely((vmf->vma_flags & (VM_WRITE|VM_SHARED)) ==
 					(VM_WRITE|VM_SHARED))) {
 		return wp_page_shared(vmf);
 	}
@@ -2975,7 +2975,7 @@ int do_swap_page(struct vm_fault *vmf)
 
 	inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
 	dec_mm_counter_fast(vma->vm_mm, MM_SWAPENTS);
-	pte = mk_pte(page, vma->vm_page_prot);
+	pte = mk_pte(page, vmf->vma_page_prot);
 	if ((vmf->flags & FAULT_FLAG_WRITE) && reuse_swap_page(page, NULL)) {
 		pte = maybe_mkwrite(pte_mkdirty(pte), vma);
 		vmf->flags &= ~FAULT_FLAG_WRITE;
@@ -2999,7 +2999,7 @@ int do_swap_page(struct vm_fault *vmf)
 
 	swap_free(entry);
 	if (mem_cgroup_swap_full(page) ||
-	    (vma->vm_flags & VM_LOCKED) || PageMlocked(page))
+	    (vmf->vma_flags & VM_LOCKED) || PageMlocked(page))
 		try_to_free_swap(page);
 	unlock_page(page);
 	if (page != swapcache) {
@@ -3056,7 +3056,7 @@ static int do_anonymous_page(struct vm_fault *vmf)
 	pte_t entry;
 
 	/* File mapping without ->vm_ops ? */
-	if (vma->vm_flags & VM_SHARED)
+	if (vmf->vma_flags & VM_SHARED)
 		return VM_FAULT_SIGBUS;
 
 	/*
@@ -3080,7 +3080,7 @@ static int do_anonymous_page(struct vm_fault *vmf)
 	if (!(vmf->flags & FAULT_FLAG_WRITE) &&
 			!mm_forbids_zeropage(vma->vm_mm)) {
 		entry = pte_mkspecial(pfn_pte(my_zero_pfn(vmf->address),
-						vma->vm_page_prot));
+						vmf->vma_page_prot));
 		if (!pte_map_lock(vmf))
 			return VM_FAULT_RETRY;
 		if (!pte_none(*vmf->pte))
@@ -3113,8 +3113,8 @@ static int do_anonymous_page(struct vm_fault *vmf)
 	 */
 	__SetPageUptodate(page);
 
-	entry = mk_pte(page, vma->vm_page_prot);
-	if (vma->vm_flags & VM_WRITE)
+	entry = mk_pte(page, vmf->vma_page_prot);
+	if (vmf->vma_flags & VM_WRITE)
 		entry = pte_mkwrite(pte_mkdirty(entry));
 
 	if (!pte_map_lock(vmf)) {
@@ -3310,7 +3310,7 @@ static int do_set_pmd(struct vm_fault *vmf, struct page *page)
 	for (i = 0; i < HPAGE_PMD_NR; i++)
 		flush_icache_page(vma, page + i);
 
-	entry = mk_huge_pmd(page, vma->vm_page_prot);
+	entry = mk_huge_pmd(page, vmf->vma_page_prot);
 	if (write)
 		entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
 
@@ -3384,11 +3384,11 @@ int alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
 		return VM_FAULT_NOPAGE;
 
 	flush_icache_page(vma, page);
-	entry = mk_pte(page, vma->vm_page_prot);
+	entry = mk_pte(page, vmf->vma_page_prot);
 	if (write)
 		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
 	/* copy-on-write page */
-	if (write && !(vma->vm_flags & VM_SHARED)) {
+	if (write && !(vmf->vma_flags & VM_SHARED)) {
 		inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
 		page_add_new_anon_rmap(page, vma, vmf->address, false);
 		mem_cgroup_commit_charge(page, memcg, false, false);
@@ -3427,7 +3427,7 @@ int finish_fault(struct vm_fault *vmf)
 
 	/* Did we COW the page? */
 	if ((vmf->flags & FAULT_FLAG_WRITE) &&
-	    !(vmf->vma->vm_flags & VM_SHARED))
+	    !(vmf->vma_flags & VM_SHARED))
 		page = vmf->cow_page;
 	else
 		page = vmf->page;
@@ -3681,7 +3681,7 @@ static int do_fault(struct vm_fault *vmf)
 		ret = VM_FAULT_SIGBUS;
 	else if (!(vmf->flags & FAULT_FLAG_WRITE))
 		ret = do_read_fault(vmf);
-	else if (!(vma->vm_flags & VM_SHARED))
+	else if (!(vmf->vma_flags & VM_SHARED))
 		ret = do_cow_fault(vmf);
 	else
 		ret = do_shared_fault(vmf);
@@ -3738,7 +3738,7 @@ static int do_numa_page(struct vm_fault *vmf)
 	 * accessible ptes, some can allow access by kernel mode.
 	 */
 	pte = ptep_modify_prot_start(vma->vm_mm, vmf->address, vmf->pte);
-	pte = pte_modify(pte, vma->vm_page_prot);
+	pte = pte_modify(pte, vmf->vma_page_prot);
 	pte = pte_mkyoung(pte);
 	if (was_writable)
 		pte = pte_mkwrite(pte);
@@ -3772,7 +3772,7 @@ static int do_numa_page(struct vm_fault *vmf)
 	 * Flag if the page is shared between multiple address spaces. This
 	 * is later used when determining whether to group tasks together
 	 */
-	if (page_mapcount(page) > 1 && (vma->vm_flags & VM_SHARED))
+	if (page_mapcount(page) > 1 && (vmf->vma_flags & VM_SHARED))
 		flags |= TNF_SHARED;
 
 	last_cpupid = page_cpupid_last(page);
@@ -3816,7 +3816,7 @@ static int wp_huge_pmd(struct vm_fault *vmf, pmd_t orig_pmd)
 		return vmf->vma->vm_ops->huge_fault(vmf, PE_SIZE_PMD);
 
 	/* COW handled on pte level: split pmd */
-	VM_BUG_ON_VMA(vmf->vma->vm_flags & VM_SHARED, vmf->vma);
+	VM_BUG_ON_VMA(vmf->vma_flags & VM_SHARED, vmf->vma);
 	__split_huge_pmd(vmf->vma, vmf->pmd, vmf->address, false, NULL);
 
 	return VM_FAULT_FALLBACK;
@@ -3963,6 +3963,8 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 		.flags = flags,
 		.pgoff = linear_page_index(vma, address),
 		.gfp_mask = __get_fault_gfp_mask(vma),
+		.vma_flags = vma->vm_flags,
+		.vma_page_prot = vma->vm_page_prot,
 	};
 	unsigned int dirty = flags & FAULT_FLAG_WRITE;
 	struct mm_struct *mm = vma->vm_mm;
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 08/20] mm: Protect SPF handler against anon_vma changes
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (6 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 07/20] mm: Cache some VMA fields in the vm_fault structure Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 09/20] mm/migrate: Pass vm_fault pointer to migrate_misplaced_page() Laurent Dufour
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

The speculative page fault handler must be protected against anon_vma
changes. This is because page_add_new_anon_rmap() is called during the
speculative path.

In addition, don't try speculative page fault if the VMA don't have an
anon_vma structure allocated because its allocation should be
protected by the mmap_sem.

In __vma_adjust() when importer->anon_vma is set, there is no need to
protect against speculative page faults since speculative page fault
is aborted if the vma->anon_vma is not set.

When calling page_add_new_anon_rmap() vma->anon_vma is necessarily
valid since we checked for it when locking the pte and the anon_vma is
removed once the pte is unlocked. So even if the speculative page
fault handler is running concurrently with do_unmap(), as the pte is
locked in unmap_region() - through unmap_vmas() - and the anon_vma
unlinked later, because we check for the vma sequence counter which is
updated in unmap_page_range() before locking the pte, and then in
free_pgtables() so when locking the pte the change will be detected.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 mm/memory.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index f008042ab24e..401b13cbfc3c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -617,7 +617,9 @@ void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma,
 		 * Hide vma from rmap and truncate_pagecache before freeing
 		 * pgtables
 		 */
+		write_seqcount_begin(&vma->vm_sequence);
 		unlink_anon_vmas(vma);
+		write_seqcount_end(&vma->vm_sequence);
 		unlink_file_vma(vma);
 
 		if (is_vm_hugetlb_page(vma)) {
@@ -631,7 +633,9 @@ void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma,
 			       && !is_vm_hugetlb_page(next)) {
 				vma = next;
 				next = vma->vm_next;
+				write_seqcount_begin(&vma->vm_sequence);
 				unlink_anon_vmas(vma);
+				write_seqcount_end(&vma->vm_sequence);
 				unlink_file_vma(vma);
 			}
 			free_pgd_range(tlb, addr, vma->vm_end,
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 09/20] mm/migrate: Pass vm_fault pointer to migrate_misplaced_page()
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (7 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 08/20] mm: Protect SPF handler against anon_vma changes Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 10/20] mm: Introduce __lru_cache_add_active_or_unevictable Laurent Dufour
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

migrate_misplaced_page() is only called during the page fault handling so
it's better to pass the pointer to the struct vm_fault instead of the vma.

This way during the speculative page fault path the saved vma->vm_flags
could be used.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/migrate.h | 4 ++--
 mm/memory.c             | 2 +-
 mm/migrate.c            | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 643c7ae7d7b4..a815ed6838b3 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -126,14 +126,14 @@ static inline void __ClearPageMovable(struct page *page)
 #ifdef CONFIG_NUMA_BALANCING
 extern bool pmd_trans_migrating(pmd_t pmd);
 extern int migrate_misplaced_page(struct page *page,
-				  struct vm_area_struct *vma, int node);
+				  struct vm_fault *vmf, int node);
 #else
 static inline bool pmd_trans_migrating(pmd_t pmd)
 {
 	return false;
 }
 static inline int migrate_misplaced_page(struct page *page,
-					 struct vm_area_struct *vma, int node)
+					 struct vm_fault *vmf, int node)
 {
 	return -EAGAIN; /* can't migrate now */
 }
diff --git a/mm/memory.c b/mm/memory.c
index 401b13cbfc3c..a4982917c16e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3790,7 +3790,7 @@ static int do_numa_page(struct vm_fault *vmf)
 	}
 
 	/* Migrate to the requested node */
-	migrated = migrate_misplaced_page(page, vma, target_nid);
+	migrated = migrate_misplaced_page(page, vmf, target_nid);
 	if (migrated) {
 		page_nid = target_nid;
 		flags |= TNF_MIGRATED;
diff --git a/mm/migrate.c b/mm/migrate.c
index 6954c1435833..4e8e22b9df8f 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1939,7 +1939,7 @@ bool pmd_trans_migrating(pmd_t pmd)
  * node. Caller is expected to have an elevated reference count on
  * the page that will be dropped by this function before returning.
  */
-int migrate_misplaced_page(struct page *page, struct vm_area_struct *vma,
+int migrate_misplaced_page(struct page *page, struct vm_fault *vmf,
 			   int node)
 {
 	pg_data_t *pgdat = NODE_DATA(node);
@@ -1952,7 +1952,7 @@ int migrate_misplaced_page(struct page *page, struct vm_area_struct *vma,
 	 * with execute permissions as they are probably shared libraries.
 	 */
 	if (page_mapcount(page) != 1 && page_is_file_cache(page) &&
-	    (vma->vm_flags & VM_EXEC))
+	    (vmf->vma_flags & VM_EXEC))
 		goto out;
 
 	/*
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 10/20] mm: Introduce __lru_cache_add_active_or_unevictable
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (8 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 09/20] mm/migrate: Pass vm_fault pointer to migrate_misplaced_page() Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 11/20] mm: Introduce __maybe_mkwrite() Laurent Dufour
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

The speculative page fault handler which is run without holding the
mmap_sem is calling lru_cache_add_active_or_unevictable() but the vm_flags
is not guaranteed to remain constant.
Introducing __lru_cache_add_active_or_unevictable() which has the vma flags
value parameter instead of the vma pointer.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/swap.h | 11 +++++++++--
 mm/memory.c          |  8 ++++----
 mm/swap.c            | 12 ++++++------
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 8a807292037f..9b4dbb98af89 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -323,8 +323,15 @@ extern void swap_setup(void);
 
 extern void add_page_to_unevictable_list(struct page *page);
 
-extern void lru_cache_add_active_or_unevictable(struct page *page,
-						struct vm_area_struct *vma);
+extern void __lru_cache_add_active_or_unevictable(struct page *page,
+						unsigned long vma_flags);
+
+static inline void lru_cache_add_active_or_unevictable(struct page *page,
+						struct vm_area_struct *vma)
+{
+	return __lru_cache_add_active_or_unevictable(page, vma->vm_flags);
+}
+
 
 /* linux/mm/vmscan.c */
 extern unsigned long zone_reclaimable_pages(struct zone *zone);
diff --git a/mm/memory.c b/mm/memory.c
index a4982917c16e..4583f354be94 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2509,7 +2509,7 @@ static int wp_page_copy(struct vm_fault *vmf)
 		ptep_clear_flush_notify(vma, vmf->address, vmf->pte);
 		page_add_new_anon_rmap(new_page, vma, vmf->address, false);
 		mem_cgroup_commit_charge(new_page, memcg, false, false);
-		lru_cache_add_active_or_unevictable(new_page, vma);
+		__lru_cache_add_active_or_unevictable(new_page, vmf->vma_flags);
 		/*
 		 * We call the notify macro here because, when using secondary
 		 * mmu page tables (such as kvm shadow page tables), we want the
@@ -2998,7 +2998,7 @@ int do_swap_page(struct vm_fault *vmf)
 	} else { /* ksm created a completely new copy */
 		page_add_new_anon_rmap(page, vma, vmf->address, false);
 		mem_cgroup_commit_charge(page, memcg, false, false);
-		lru_cache_add_active_or_unevictable(page, vma);
+		__lru_cache_add_active_or_unevictable(page, vmf->vma_flags);
 	}
 
 	swap_free(entry);
@@ -3144,7 +3144,7 @@ static int do_anonymous_page(struct vm_fault *vmf)
 	inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
 	page_add_new_anon_rmap(page, vma, vmf->address, false);
 	mem_cgroup_commit_charge(page, memcg, false, false);
-	lru_cache_add_active_or_unevictable(page, vma);
+	__lru_cache_add_active_or_unevictable(page, vmf->vma_flags);
 setpte:
 	set_pte_at(vma->vm_mm, vmf->address, vmf->pte, entry);
 
@@ -3396,7 +3396,7 @@ int alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
 		inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
 		page_add_new_anon_rmap(page, vma, vmf->address, false);
 		mem_cgroup_commit_charge(page, memcg, false, false);
-		lru_cache_add_active_or_unevictable(page, vma);
+		__lru_cache_add_active_or_unevictable(page, vmf->vma_flags);
 	} else {
 		inc_mm_counter_fast(vma->vm_mm, mm_counter_file(page));
 		page_add_file_rmap(page, false);
diff --git a/mm/swap.c b/mm/swap.c
index 9295ae960d66..b084bb16d769 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -470,21 +470,21 @@ void add_page_to_unevictable_list(struct page *page)
 }
 
 /**
- * lru_cache_add_active_or_unevictable
- * @page:  the page to be added to LRU
- * @vma:   vma in which page is mapped for determining reclaimability
+ * __lru_cache_add_active_or_unevictable
+ * @page:	the page to be added to LRU
+ * @vma_flags:  vma in which page is mapped for determining reclaimability
  *
  * Place @page on the active or unevictable LRU list, depending on its
  * evictability.  Note that if the page is not evictable, it goes
  * directly back onto it's zone's unevictable list, it does NOT use a
  * per cpu pagevec.
  */
-void lru_cache_add_active_or_unevictable(struct page *page,
-					 struct vm_area_struct *vma)
+void __lru_cache_add_active_or_unevictable(struct page *page,
+					   unsigned long vma_flags)
 {
 	VM_BUG_ON_PAGE(PageLRU(page), page);
 
-	if (likely((vma->vm_flags & (VM_LOCKED | VM_SPECIAL)) != VM_LOCKED)) {
+	if (likely((vma_flags & (VM_LOCKED | VM_SPECIAL)) != VM_LOCKED)) {
 		SetPageActive(page);
 		lru_cache_add(page);
 		return;
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 11/20] mm: Introduce __maybe_mkwrite()
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (9 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 10/20] mm: Introduce __lru_cache_add_active_or_unevictable Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 12/20] mm: Introduce __vm_normal_page() Laurent Dufour
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

The current maybe_mkwrite() is getting passed the pointer to the vma
structure to fetch the vm_flags field.

When dealing with the speculative page fault handler, it will be better to
rely on the cached vm_flags value stored in the vm_fault structure.

This patch introduce a __maybe_mkwrite() service which can be called by
passing the value of the vm_flags field.

There is no change functional changes expected for the other callers of
maybe_mkwrite().

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/mm.h | 9 +++++++--
 mm/memory.c        | 6 +++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5fd90ac31317..bb0c87f1c725 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -673,13 +673,18 @@ void free_compound_page(struct page *page);
  * pte_mkwrite.  But get_user_pages can cause write faults for mappings
  * that do not have writing enabled, when used by access_process_vm.
  */
-static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma)
+static inline pte_t __maybe_mkwrite(pte_t pte, unsigned long vma_flags)
 {
-	if (likely(vma->vm_flags & VM_WRITE))
+	if (likely(vma_flags & VM_WRITE))
 		pte = pte_mkwrite(pte);
 	return pte;
 }
 
+static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma)
+{
+	return __maybe_mkwrite(pte, vma->vm_flags);
+}
+
 int alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
 		struct page *page);
 int finish_fault(struct vm_fault *vmf);
diff --git a/mm/memory.c b/mm/memory.c
index 4583f354be94..c306f1f64c9e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2408,7 +2408,7 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
 
 	flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
 	entry = pte_mkyoung(vmf->orig_pte);
-	entry = maybe_mkwrite(pte_mkdirty(entry), vma);
+	entry = __maybe_mkwrite(pte_mkdirty(entry), vmf->vma_flags);
 	if (ptep_set_access_flags(vma, vmf->address, vmf->pte, entry, 1))
 		update_mmu_cache(vma, vmf->address, vmf->pte);
 	pte_unmap_unlock(vmf->pte, vmf->ptl);
@@ -2498,8 +2498,8 @@ static int wp_page_copy(struct vm_fault *vmf)
 			inc_mm_counter_fast(mm, MM_ANONPAGES);
 		}
 		flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
-		entry = mk_pte(new_page, vma->vm_page_prot);
-		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
+		entry = mk_pte(new_page, vmf->vma_page_prot);
+		entry = __maybe_mkwrite(pte_mkdirty(entry), vmf->vma_flags);
 		/*
 		 * Clear the pte entry and flush it first, before updating the
 		 * pte with the new entry. This will avoid a race condition
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 12/20] mm: Introduce __vm_normal_page()
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (10 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 11/20] mm: Introduce __maybe_mkwrite() Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 13/20] mm: Introduce __page_add_new_anon_rmap() Laurent Dufour
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

When dealing with the speculative fault path we should use the VMA's field
cached value stored in the vm_fault structure.

Currently vm_normal_page() is using the pointer to the VMA to fetch the
vm_flags value. This patch provides a new __vm_normal_page() which is
receiving the vm_flags flags value as parameter.

Note: The speculative path is turned on for architecture providing support
for special PTE flag. So only the first block of vm_normal_page is used
during the speculative path.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/mm.h |  7 +++++--
 mm/memory.c        | 18 ++++++++++--------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index bb0c87f1c725..a2857aaa03f1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1246,8 +1246,11 @@ struct zap_details {
 	pgoff_t last_index;			/* Highest page->index to unmap */
 };
 
-struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
-			     pte_t pte, bool with_public_device);
+struct page *__vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
+			      pte_t pte, bool with_public_device,
+			      unsigned long vma_flags);
+#define _vm_normal_page(vma, addr, pte, with_public_device) \
+	__vm_normal_page(vma, addr, pte, with_public_device, (vma)->vm_flags);
 #define vm_normal_page(vma, addr, pte) _vm_normal_page(vma, addr, pte, false)
 
 struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
diff --git a/mm/memory.c b/mm/memory.c
index c306f1f64c9e..a5b5fe833ed3 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -822,8 +822,9 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
 #else
 # define HAVE_PTE_SPECIAL 0
 #endif
-struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
-			     pte_t pte, bool with_public_device)
+struct page *__vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
+			      pte_t pte, bool with_public_device,
+			      unsigned long vma_flags)
 {
 	unsigned long pfn = pte_pfn(pte);
 
@@ -832,7 +833,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
 			goto check_pfn;
 		if (vma->vm_ops && vma->vm_ops->find_special_page)
 			return vma->vm_ops->find_special_page(vma, addr);
-		if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
+		if (vma_flags & (VM_PFNMAP | VM_MIXEDMAP))
 			return NULL;
 		if (is_zero_pfn(pfn))
 			return NULL;
@@ -864,8 +865,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
 
 	/* !HAVE_PTE_SPECIAL case follows: */
 
-	if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) {
-		if (vma->vm_flags & VM_MIXEDMAP) {
+	if (unlikely(vma_flags & (VM_PFNMAP|VM_MIXEDMAP))) {
+		if (vma_flags & VM_MIXEDMAP) {
 			if (!pfn_valid(pfn))
 				return NULL;
 			goto out;
@@ -874,7 +875,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
 			off = (addr - vma->vm_start) >> PAGE_SHIFT;
 			if (pfn == vma->vm_pgoff + off)
 				return NULL;
-			if (!is_cow_mapping(vma->vm_flags))
+			if (!is_cow_mapping(vma_flags))
 				return NULL;
 		}
 	}
@@ -2687,7 +2688,8 @@ static int do_wp_page(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 
-	vmf->page = vm_normal_page(vma, vmf->address, vmf->orig_pte);
+	vmf->page = __vm_normal_page(vma, vmf->address, vmf->orig_pte, false,
+				     vmf->vma_flags);
 	if (!vmf->page) {
 		/*
 		 * VM_MIXEDMAP !pfn_valid() case, or VM_SOFTDIRTY clear on a
@@ -3749,7 +3751,7 @@ static int do_numa_page(struct vm_fault *vmf)
 	ptep_modify_prot_commit(vma->vm_mm, vmf->address, vmf->pte, pte);
 	update_mmu_cache(vma, vmf->address, vmf->pte);
 
-	page = vm_normal_page(vma, vmf->address, pte);
+	page = __vm_normal_page(vma, vmf->address, pte, false, vmf->vma_flags);
 	if (!page) {
 		pte_unmap_unlock(vmf->pte, vmf->ptl);
 		return 0;
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 13/20] mm: Introduce __page_add_new_anon_rmap()
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (11 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 12/20] mm: Introduce __vm_normal_page() Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 14/20] mm: Provide speculative fault infrastructure Laurent Dufour
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

When dealing with speculative page fault handler, we may race with VMA
being split or merged. In this case the vma->vm_start and vm->vm_end
fields may not match the address the page fault is occurring.

This can only happens when the VMA is split but in that case, the
anon_vma pointer of the new VMA will be the same as the original one,
because in __split_vma the new->anon_vma is set to src->anon_vma when
*new = *vma.

So even if the VMA boundaries are not correct, the anon_vma pointer is
still valid.

If the VMA has been merged, then the VMA in which it has been merged
must have the same anon_vma pointer otherwise the merge can't be done.

So in all the case we know that the anon_vma is valid, since we have
checked before starting the speculative page fault that the anon_vma
pointer is valid for this VMA and since there is an anon_vma this
means that at one time a page has been backed and that before the VMA
is cleaned, the page table lock would have to be grab to clean the
PTE, and the anon_vma field is checked once the PTE is locked.

This patch introduce a new __page_add_new_anon_rmap() service which
doesn't check for the VMA boundaries, and create a new inline one
which do the check.

When called from a page fault handler, if this is not a speculative one,
there is a guarantee that vm_start and vm_end match the faulting address,
so this check is useless. In the context of the speculative page fault
handler, this check may be wrong but anon_vma is still valid as explained
above.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/rmap.h | 12 ++++++++++--
 mm/memory.c          |  8 ++++----
 mm/rmap.c            |  5 ++---
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 733d3d8181e2..d91be69c1c60 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -173,8 +173,16 @@ void page_add_anon_rmap(struct page *, struct vm_area_struct *,
 		unsigned long, bool);
 void do_page_add_anon_rmap(struct page *, struct vm_area_struct *,
 			   unsigned long, int);
-void page_add_new_anon_rmap(struct page *, struct vm_area_struct *,
-		unsigned long, bool);
+void __page_add_new_anon_rmap(struct page *, struct vm_area_struct *,
+			      unsigned long, bool);
+static inline void page_add_new_anon_rmap(struct page *page,
+					  struct vm_area_struct *vma,
+					  unsigned long address, bool compound)
+{
+	VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma);
+	__page_add_new_anon_rmap(page, vma, address, compound);
+}
+
 void page_add_file_rmap(struct page *, bool);
 void page_remove_rmap(struct page *, bool);
 
diff --git a/mm/memory.c b/mm/memory.c
index a5b5fe833ed3..479b47a8ed7c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2508,7 +2508,7 @@ static int wp_page_copy(struct vm_fault *vmf)
 		 * thread doing COW.
 		 */
 		ptep_clear_flush_notify(vma, vmf->address, vmf->pte);
-		page_add_new_anon_rmap(new_page, vma, vmf->address, false);
+		__page_add_new_anon_rmap(new_page, vma, vmf->address, false);
 		mem_cgroup_commit_charge(new_page, memcg, false, false);
 		__lru_cache_add_active_or_unevictable(new_page, vmf->vma_flags);
 		/*
@@ -2998,7 +2998,7 @@ int do_swap_page(struct vm_fault *vmf)
 		mem_cgroup_commit_charge(page, memcg, true, false);
 		activate_page(page);
 	} else { /* ksm created a completely new copy */
-		page_add_new_anon_rmap(page, vma, vmf->address, false);
+		__page_add_new_anon_rmap(page, vma, vmf->address, false);
 		mem_cgroup_commit_charge(page, memcg, false, false);
 		__lru_cache_add_active_or_unevictable(page, vmf->vma_flags);
 	}
@@ -3144,7 +3144,7 @@ static int do_anonymous_page(struct vm_fault *vmf)
 	}
 
 	inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
-	page_add_new_anon_rmap(page, vma, vmf->address, false);
+	__page_add_new_anon_rmap(page, vma, vmf->address, false);
 	mem_cgroup_commit_charge(page, memcg, false, false);
 	__lru_cache_add_active_or_unevictable(page, vmf->vma_flags);
 setpte:
@@ -3396,7 +3396,7 @@ int alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
 	/* copy-on-write page */
 	if (write && !(vmf->vma_flags & VM_SHARED)) {
 		inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
-		page_add_new_anon_rmap(page, vma, vmf->address, false);
+		__page_add_new_anon_rmap(page, vma, vmf->address, false);
 		mem_cgroup_commit_charge(page, memcg, false, false);
 		__lru_cache_add_active_or_unevictable(page, vmf->vma_flags);
 	} else {
diff --git a/mm/rmap.c b/mm/rmap.c
index b874c4761e84..5d657329191e 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1133,7 +1133,7 @@ void do_page_add_anon_rmap(struct page *page,
 }
 
 /**
- * page_add_new_anon_rmap - add pte mapping to a new anonymous page
+ * __page_add_new_anon_rmap - add pte mapping to a new anonymous page
  * @page:	the page to add the mapping to
  * @vma:	the vm area in which the mapping is added
  * @address:	the user virtual address mapped
@@ -1143,12 +1143,11 @@ void do_page_add_anon_rmap(struct page *page,
  * This means the inc-and-test can be bypassed.
  * Page does not have to be locked.
  */
-void page_add_new_anon_rmap(struct page *page,
+void __page_add_new_anon_rmap(struct page *page,
 	struct vm_area_struct *vma, unsigned long address, bool compound)
 {
 	int nr = compound ? hpage_nr_pages(page) : 1;
 
-	VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma);
 	__SetPageSwapBacked(page);
 	if (compound) {
 		VM_BUG_ON_PAGE(!PageTransHuge(page), page);
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 14/20] mm: Provide speculative fault infrastructure
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (12 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 13/20] mm: Introduce __page_add_new_anon_rmap() Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:06 ` [PATCH v3 15/20] mm: Try spin lock in speculative path Laurent Dufour
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

From: Peter Zijlstra <peterz@infradead.org>

Provide infrastructure to do a speculative fault (not holding
mmap_sem).

The not holding of mmap_sem means we can race against VMA
change/removal and page-table destruction. We use the SRCU VMA freeing
to keep the VMA around. We use the VMA seqcount to detect change
(including umapping / page-table deletion) and we use gup_fast() style
page-table walking to deal with page-table races.

Once we've obtained the page and are ready to update the PTE, we
validate if the state we started the fault with is still valid, if
not, we'll fail the fault with VM_FAULT_RETRY, otherwise we update the
PTE and we're done.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[Manage the newly introduced pte_spinlock() for speculative page
 fault to fail if the VMA is touched in our back]
[Rename vma_is_dead() to vma_has_changed() and declare it here]
[Call p4d_alloc() as it is safe since pgd is valid]
[Call pud_alloc() as it is safe since p4d is valid]
[Set fe.sequence in __handle_mm_fault()]
[Abort speculative path when handle_userfault() has to be called]
[Add additional VMA's flags checks in handle_speculative_fault()]
[Clear FAULT_FLAG_ALLOW_RETRY in handle_speculative_fault()]
[Don't set vmf->pte and vmf->ptl if pte_map_lock() failed]
[Remove warning comment about waiting for !seq&1 since we don't want
 to wait]
[Remove warning about no huge page support, mention it explictly]
[Don't call do_fault() in the speculative path as __do_fault() calls
 vma->vm_ops->fault() which may want to release mmap_sem]
[Only vm_fault pointer argument for vma_has_changed()]
[Fix check against huge page, calling pmd_trans_huge()]
[Introduce __HAVE_ARCH_CALL_SPF to declare the SPF handler only when
 architecture is supporting it]
[Use READ_ONCE() when reading VMA's fields in the speculative path]
[Explicitly check for __HAVE_ARCH_PTE_SPECIAL as we can't support for
 processing done in vm_normal_page()]
[Check that vma->anon_vma is already set when starting the speculative
 path]
[Check for memory policy as we can't support MPOL_INTERLEAVE case due to
 the processing done in mpol_misplaced()]
[Don't support VMA growing up or down]
[Move check on vm_sequence just before calling handle_pte_fault()]
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/linux/hugetlb_inline.h |   2 +-
 include/linux/mm.h             |   5 +
 include/linux/pagemap.h        |   4 +-
 mm/internal.h                  |  14 +++
 mm/memory.c                    | 249 ++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 266 insertions(+), 8 deletions(-)

diff --git a/include/linux/hugetlb_inline.h b/include/linux/hugetlb_inline.h
index a4e7ca0f3585..6cfdfca4cc2a 100644
--- a/include/linux/hugetlb_inline.h
+++ b/include/linux/hugetlb_inline.h
@@ -7,7 +7,7 @@
 
 static inline bool is_vm_hugetlb_page(struct vm_area_struct *vma)
 {
-	return !!(vma->vm_flags & VM_HUGETLB);
+	return !!(READ_ONCE(vma->vm_flags) & VM_HUGETLB);
 }
 
 #else
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a2857aaa03f1..966b69f10f57 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -320,6 +320,7 @@ struct vm_fault {
 	gfp_t gfp_mask;			/* gfp mask to be used for allocations */
 	pgoff_t pgoff;			/* Logical page offset based on vma */
 	unsigned long address;		/* Faulting virtual address */
+	unsigned int sequence;
 	pmd_t *pmd;			/* Pointer to pmd entry matching
 					 * the 'address' */
 	pud_t *pud;			/* Pointer to pud entry matching
@@ -1342,6 +1343,10 @@ int invalidate_inode_page(struct page *page);
 #ifdef CONFIG_MMU
 extern int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 		unsigned int flags);
+#ifdef __HAVE_ARCH_CALL_SPF
+extern int handle_speculative_fault(struct mm_struct *mm,
+				    unsigned long address, unsigned int flags);
+#endif /* __HAVE_ARCH_CALL_SPF */
 extern int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
 			    unsigned long address, unsigned int fault_flags,
 			    bool *unlocked);
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 5bbd6780f205..832aa3ec7d00 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -451,8 +451,8 @@ static inline pgoff_t linear_page_index(struct vm_area_struct *vma,
 	pgoff_t pgoff;
 	if (unlikely(is_vm_hugetlb_page(vma)))
 		return linear_hugepage_index(vma, address);
-	pgoff = (address - vma->vm_start) >> PAGE_SHIFT;
-	pgoff += vma->vm_pgoff;
+	pgoff = (address - READ_ONCE(vma->vm_start)) >> PAGE_SHIFT;
+	pgoff += READ_ONCE(vma->vm_pgoff);
 	return pgoff;
 }
 
diff --git a/mm/internal.h b/mm/internal.h
index 84360184eafd..4ddadc440c26 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -45,6 +45,20 @@ extern struct srcu_struct vma_srcu;
 extern struct vm_area_struct *find_vma_srcu(struct mm_struct *mm,
 					    unsigned long addr);
 
+static inline bool vma_has_changed(struct vm_fault *vmf)
+{
+	int ret = RB_EMPTY_NODE(&vmf->vma->vm_rb);
+	unsigned seq = ACCESS_ONCE(vmf->vma->vm_sequence.sequence);
+
+	/*
+	 * Matches both the wmb in write_seqlock_{begin,end}() and
+	 * the wmb in vma_rb_erase().
+	 */
+	smp_rmb();
+
+	return ret || seq != vmf->sequence;
+}
+
 void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
 		unsigned long floor, unsigned long ceiling);
 
diff --git a/mm/memory.c b/mm/memory.c
index 479b47a8ed7c..5e98259c7ac0 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -762,7 +762,8 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
 	if (page)
 		dump_page(page, "bad pte");
 	pr_alert("addr:%p vm_flags:%08lx anon_vma:%p mapping:%p index:%lx\n",
-		 (void *)addr, vma->vm_flags, vma->anon_vma, mapping, index);
+		 (void *)addr, READ_ONCE(vma->vm_flags), vma->anon_vma,
+		 mapping, index);
 	/*
 	 * Choose text because data symbols depend on CONFIG_KALLSYMS_ALL=y
 	 */
@@ -2417,15 +2418,69 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
 
 static bool pte_spinlock(struct vm_fault *vmf)
 {
+	bool ret = false;
+
+	/* Check if vma is still valid */
+	if (!(vmf->flags & FAULT_FLAG_SPECULATIVE)) {
+		vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
+		spin_lock(vmf->ptl);
+		return true;
+	}
+
+	local_irq_disable();
+	if (vma_has_changed(vmf))
+		goto out;
+
 	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
 	spin_lock(vmf->ptl);
-	return true;
+
+	if (vma_has_changed(vmf)) {
+		spin_unlock(vmf->ptl);
+		goto out;
+	}
+
+	ret = true;
+out:
+	local_irq_enable();
+	return ret;
 }
 
 static bool pte_map_lock(struct vm_fault *vmf)
 {
-	vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl);
-	return true;
+	bool ret = false;
+	pte_t *pte;
+	spinlock_t *ptl;
+
+	if (!(vmf->flags & FAULT_FLAG_SPECULATIVE)) {
+		vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd,
+					       vmf->address, &vmf->ptl);
+		return true;
+	}
+
+	/*
+	 * The first vma_has_changed() guarantees the page-tables are still
+	 * valid, having IRQs disabled ensures they stay around, hence the
+	 * second vma_has_changed() to make sure they are still valid once
+	 * we've got the lock. After that a concurrent zap_pte_range() will
+	 * block on the PTL and thus we're safe.
+	 */
+	local_irq_disable();
+	if (vma_has_changed(vmf))
+		goto out;
+
+	pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd,
+				  vmf->address, &ptl);
+	if (vma_has_changed(vmf)) {
+		pte_unmap_unlock(pte, ptl);
+		goto out;
+	}
+
+	vmf->pte = pte;
+	vmf->ptl = ptl;
+	ret = true;
+out:
+	local_irq_enable();
+	return ret;
 }
 
 /*
@@ -3094,6 +3149,14 @@ static int do_anonymous_page(struct vm_fault *vmf)
 		ret = check_stable_address_space(vma->vm_mm);
 		if (ret)
 			goto unlock;
+		/*
+		 * Don't call the userfaultfd during the speculative path.
+		 * We already checked for the VMA to not be managed through
+		 * userfaultfd, but it may be set in our back once we have lock
+		 * the pte. In such a case we can ignore it this time.
+		 */
+		if (vmf->flags & FAULT_FLAG_SPECULATIVE)
+			goto setpte;
 		/* Deliver the page fault to userland, check inside PT lock */
 		if (userfaultfd_missing(vma)) {
 			pte_unmap_unlock(vmf->pte, vmf->ptl);
@@ -3136,7 +3199,7 @@ static int do_anonymous_page(struct vm_fault *vmf)
 		goto release;
 
 	/* Deliver the page fault to userland, check inside PT lock */
-	if (userfaultfd_missing(vma)) {
+	if (!(vmf->flags & FAULT_FLAG_SPECULATIVE) && userfaultfd_missing(vma)) {
 		pte_unmap_unlock(vmf->pte, vmf->ptl);
 		mem_cgroup_cancel_charge(page, memcg, false);
 		put_page(page);
@@ -3915,6 +3978,8 @@ static int handle_pte_fault(struct vm_fault *vmf)
 	if (!vmf->pte) {
 		if (vma_is_anonymous(vmf->vma))
 			return do_anonymous_page(vmf);
+		else if (vmf->flags & FAULT_FLAG_SPECULATIVE)
+			return VM_FAULT_RETRY;
 		else
 			return do_fault(vmf);
 	}
@@ -4012,6 +4077,7 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 	vmf.pmd = pmd_alloc(mm, vmf.pud, address);
 	if (!vmf.pmd)
 		return VM_FAULT_OOM;
+	vmf.sequence = raw_read_seqcount(&vma->vm_sequence);
 	if (pmd_none(*vmf.pmd) && transparent_hugepage_enabled(vma)) {
 		ret = create_huge_pmd(&vmf);
 		if (!(ret & VM_FAULT_FALLBACK))
@@ -4045,6 +4111,179 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
 	return handle_pte_fault(&vmf);
 }
 
+#ifdef __HAVE_ARCH_CALL_SPF
+
+#ifndef __HAVE_ARCH_PTE_SPECIAL
+/* This is required by vm_normal_page() */
+#error "Speculative page fault handler requires __HAVE_ARCH_PTE_SPECIAL"
+#endif
+
+/*
+ * vm_normal_page() adds some processing which should be done while
+ * hodling the mmap_sem.
+ */
+int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
+			     unsigned int flags)
+{
+	struct vm_fault vmf = {
+		.address = address,
+	};
+	pgd_t *pgd;
+	p4d_t *p4d;
+	pud_t *pud;
+	pmd_t *pmd;
+	int dead, seq, idx, ret = VM_FAULT_RETRY;
+	struct vm_area_struct *vma;
+#ifdef CONFIG_NUMA
+	struct mempolicy *pol;
+#endif
+
+	/* Clear flags that may lead to release the mmap_sem to retry */
+	flags &= ~(FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_KILLABLE);
+	flags |= FAULT_FLAG_SPECULATIVE;
+
+	idx = srcu_read_lock(&vma_srcu);
+	vma = find_vma_srcu(mm, address);
+	if (!vma)
+		goto unlock;
+
+	/*
+	 * Validate the VMA found by the lockless lookup.
+	 */
+	dead = RB_EMPTY_NODE(&vma->vm_rb);
+	seq = raw_read_seqcount(&vma->vm_sequence); /* rmb <-> seqlock,vma_rb_erase() */
+	if ((seq & 1) || dead)
+		goto unlock;
+
+	/*
+	 * Can't call vm_ops service has we don't know what they would do
+	 * with the VMA.
+	 * This include huge page from hugetlbfs.
+	 */
+	if (vma->vm_ops)
+		goto unlock;
+
+	/*
+	 * __anon_vma_prepare() requires the mmap_sem to be held
+	 * because vm_next and vm_prev must be safe. This can't be guaranteed
+	 * in the speculative path.
+	 */
+	if (unlikely(!vma->anon_vma))
+		goto unlock;
+
+	vmf.vma_flags = READ_ONCE(vma->vm_flags);
+	vmf.vma_page_prot = READ_ONCE(vma->vm_page_prot);
+
+	/* Can't call userland page fault handler in the speculative path */
+	if (unlikely(vmf.vma_flags & VM_UFFD_MISSING))
+		goto unlock;
+
+#ifdef CONFIG_NUMA
+	/*
+	 * MPOL_INTERLEAVE implies additional check in mpol_misplaced() which
+	 * are not compatible with the speculative page fault processing.
+	 */
+	pol = __get_vma_policy(vma, address);
+	if (!pol)
+		pol = get_task_policy(current);
+	if (pol && pol->mode == MPOL_INTERLEAVE)
+		goto unlock;
+#endif
+
+	if (vmf.vma_flags & VM_GROWSDOWN || vmf.vma_flags & VM_GROWSUP)
+		/*
+		 * This could be detected by the check address against VMA's
+		 * boundaries but we want to trace it as not supported instead
+		 * of changed.
+		 */
+		goto unlock;
+
+	if (address < READ_ONCE(vma->vm_start)
+	    || READ_ONCE(vma->vm_end) <= address)
+		goto unlock;
+
+	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
+				       flags & FAULT_FLAG_INSTRUCTION,
+				       flags & FAULT_FLAG_REMOTE)) {
+		ret = VM_FAULT_SIGSEGV;
+		goto unlock;
+	}
+
+	/* This is one is required to check that the VMA has write access set */
+	if (flags & FAULT_FLAG_WRITE) {
+		if (unlikely(!(vmf.vma_flags & VM_WRITE))) {
+			ret = VM_FAULT_SIGSEGV;
+			goto unlock;
+		}
+	} else if (unlikely(!(vmf.vma_flags & (VM_READ|VM_EXEC|VM_WRITE)))) {
+		ret = VM_FAULT_SIGSEGV;
+		goto unlock;
+	}
+
+	/*
+	 * Do a speculative lookup of the PTE entry.
+	 */
+	local_irq_disable();
+	pgd = pgd_offset(mm, address);
+	if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
+		goto out_walk;
+
+	p4d = p4d_alloc(mm, pgd, address);
+	if (p4d_none(*p4d) || unlikely(p4d_bad(*p4d)))
+		goto out_walk;
+
+	pud = pud_alloc(mm, p4d, address);
+	if (pud_none(*pud) || unlikely(pud_bad(*pud)))
+		goto out_walk;
+
+	/* Transparent huge pages are not supported. */
+	if (unlikely(pud_trans_huge(*pud)))
+		goto out_walk;
+
+	pmd = pmd_offset(pud, address);
+	if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
+		goto out_walk;
+
+	/*
+	 * The above does not allocate/instantiate page-tables because doing so
+	 * would lead to the possibility of instantiating page-tables after
+	 * free_pgtables() -- and consequently leaking them.
+	 *
+	 * The result is that we take at least one !speculative fault per PMD
+	 * in order to instantiate it.
+	 */
+	/* Transparent huge pages are not supported. */
+	if (unlikely(pmd_trans_huge(*pmd)))
+		goto out_walk;
+
+	vmf.vma = vma;
+	vmf.pmd = pmd;
+	vmf.pgoff = linear_page_index(vma, address);
+	vmf.gfp_mask = __get_fault_gfp_mask(vma);
+	vmf.sequence = seq;
+	vmf.flags = flags;
+
+	local_irq_enable();
+
+	/*
+	 * We need to re-validate the VMA after checking the bounds, otherwise
+	 * we might have a false positive on the bounds.
+	 */
+	if (read_seqcount_retry(&vma->vm_sequence, seq))
+		goto unlock;
+
+	ret = handle_pte_fault(&vmf);
+
+unlock:
+	srcu_read_unlock(&vma_srcu, idx);
+	return ret;
+
+out_walk:
+	local_irq_enable();
+	goto unlock;
+}
+#endif /* __HAVE_ARCH_CALL_SPF */
+
 /*
  * By the time we get here, we already hold the mm semaphore
  *
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 15/20] mm: Try spin lock in speculative path
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (13 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 14/20] mm: Provide speculative fault infrastructure Laurent Dufour
@ 2017-09-08 18:06 ` Laurent Dufour
  2017-09-08 18:07 ` [PATCH v3 16/20] mm: Adding speculative page fault failure trace events Laurent Dufour
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:06 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

There is a deadlock when a CPU is doing a speculative page fault and
another one is calling do_unmap().

The deadlock occurred because the speculative path try to spinlock the
pte while the interrupt are disabled. When the other CPU in the
unmap's path has locked the pte then is waiting for all the CPU to
invalidate the TLB. As the CPU doing the speculative fault have the
interrupt disable it can't invalidate the TLB, and can't get the lock.

Since we are in a speculative path, we can race with other mm action.
So let assume that the lock may not get acquired and fail the
speculative page fault.

Here are the stacks captured during the deadlock:

	CPU 0
	native_flush_tlb_others+0x7c/0x260
	flush_tlb_mm_range+0x6a/0x220
	tlb_flush_mmu_tlbonly+0x63/0xc0
	unmap_page_range+0x897/0x9d0
	? unmap_single_vma+0x7d/0xe0
	? release_pages+0x2b3/0x360
	unmap_single_vma+0x7d/0xe0
	unmap_vmas+0x51/0xa0
	unmap_region+0xbd/0x130
	do_munmap+0x279/0x460
	SyS_munmap+0x53/0x70

	CPU 1
	do_raw_spin_lock+0x14e/0x160
	_raw_spin_lock+0x5d/0x80
	? pte_map_lock+0x169/0x1b0
	pte_map_lock+0x169/0x1b0
	handle_pte_fault+0xbf2/0xd80
	? trace_hardirqs_on+0xd/0x10
	handle_speculative_fault+0x272/0x280
	handle_speculative_fault+0x5/0x280
	__do_page_fault+0x187/0x580
	trace_do_page_fault+0x52/0x260
	do_async_page_fault+0x19/0x70
	async_page_fault+0x28/0x30

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 mm/memory.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 5e98259c7ac0..18b39f930ce1 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2432,7 +2432,8 @@ static bool pte_spinlock(struct vm_fault *vmf)
 		goto out;
 
 	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
-	spin_lock(vmf->ptl);
+	if (unlikely(!spin_trylock(vmf->ptl)))
+		goto out;
 
 	if (vma_has_changed(vmf)) {
 		spin_unlock(vmf->ptl);
@@ -2468,8 +2469,20 @@ static bool pte_map_lock(struct vm_fault *vmf)
 	if (vma_has_changed(vmf))
 		goto out;
 
-	pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd,
-				  vmf->address, &ptl);
+	/*
+	 * Same as pte_offset_map_lock() except that we call
+	 * spin_trylock() in place of spin_lock() to avoid race with
+	 * unmap path which may have the lock and wait for this CPU
+	 * to invalidate TLB but this CPU has irq disabled.
+	 * Since we are in a speculative patch, accept it could fail
+	 */
+	ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
+	pte = pte_offset_map(vmf->pmd, vmf->address);
+	if (unlikely(!spin_trylock(ptl))) {
+		pte_unmap(pte);
+		goto out;
+	}
+
 	if (vma_has_changed(vmf)) {
 		pte_unmap_unlock(pte, ptl);
 		goto out;
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 16/20] mm: Adding speculative page fault failure trace events
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (14 preceding siblings ...)
  2017-09-08 18:06 ` [PATCH v3 15/20] mm: Try spin lock in speculative path Laurent Dufour
@ 2017-09-08 18:07 ` Laurent Dufour
  2017-09-08 18:07 ` [PATCH v3 17/20] perf: Add a speculative page fault sw event Laurent Dufour
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:07 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

This patch a set of new trace events to collect the speculative page fault
event failures.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/trace/events/pagefault.h | 87 ++++++++++++++++++++++++++++++++++++++++
 mm/memory.c                      | 59 ++++++++++++++++++++++-----
 2 files changed, 135 insertions(+), 11 deletions(-)
 create mode 100644 include/trace/events/pagefault.h

diff --git a/include/trace/events/pagefault.h b/include/trace/events/pagefault.h
new file mode 100644
index 000000000000..d7d56f8102d1
--- /dev/null
+++ b/include/trace/events/pagefault.h
@@ -0,0 +1,87 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM pagefault
+
+#if !defined(_TRACE_PAGEFAULT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PAGEFAULT_H
+
+#include <linux/tracepoint.h>
+#include <linux/mm.h>
+
+DECLARE_EVENT_CLASS(spf,
+
+	TP_PROTO(unsigned long caller,
+		 struct vm_area_struct *vma, unsigned long address),
+
+	TP_ARGS(caller, vma, address),
+
+	TP_STRUCT__entry(
+		__field(unsigned long, caller)
+		__field(unsigned long, vm_start)
+		__field(unsigned long, vm_end)
+		__field(unsigned long, address)
+	),
+
+	TP_fast_assign(
+		__entry->caller		= caller;
+		__entry->vm_start	= vma->vm_start;
+		__entry->vm_end		= vma->vm_end;
+		__entry->address	= address;
+	),
+
+	TP_printk("ip:%lx vma:%lu-%lx address:%lx",
+		  __entry->caller, __entry->vm_start, __entry->vm_end,
+		  __entry->address)
+);
+
+DEFINE_EVENT(spf, spf_pte_lock,
+
+	TP_PROTO(unsigned long caller,
+		 struct vm_area_struct *vma, unsigned long address),
+
+	TP_ARGS(caller, vma, address)
+);
+
+DEFINE_EVENT(spf, spf_vma_changed,
+
+	TP_PROTO(unsigned long caller,
+		 struct vm_area_struct *vma, unsigned long address),
+
+	TP_ARGS(caller, vma, address)
+);
+
+DEFINE_EVENT(spf, spf_vma_dead,
+
+	TP_PROTO(unsigned long caller,
+		 struct vm_area_struct *vma, unsigned long address),
+
+	TP_ARGS(caller, vma, address)
+);
+
+DEFINE_EVENT(spf, spf_vma_noanon,
+
+	TP_PROTO(unsigned long caller,
+		 struct vm_area_struct *vma, unsigned long address),
+
+	TP_ARGS(caller, vma, address)
+);
+
+DEFINE_EVENT(spf, spf_vma_notsup,
+
+	TP_PROTO(unsigned long caller,
+		 struct vm_area_struct *vma, unsigned long address),
+
+	TP_ARGS(caller, vma, address)
+);
+
+DEFINE_EVENT(spf, spf_vma_access,
+
+	TP_PROTO(unsigned long caller,
+		 struct vm_area_struct *vma, unsigned long address),
+
+	TP_ARGS(caller, vma, address)
+);
+
+#endif /* _TRACE_PAGEFAULT_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/mm/memory.c b/mm/memory.c
index 18b39f930ce1..c2a52ddc850b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -81,6 +81,9 @@
 
 #include "internal.h"
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/pagefault.h>
+
 #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
 #warning Unfortunate NUMA and NUMA Balancing config, growing page-frame for last_cpupid.
 #endif
@@ -2428,15 +2431,20 @@ static bool pte_spinlock(struct vm_fault *vmf)
 	}
 
 	local_irq_disable();
-	if (vma_has_changed(vmf))
+	if (vma_has_changed(vmf)) {
+		trace_spf_vma_changed(_RET_IP_, vmf->vma, vmf->address);
 		goto out;
+	}
 
 	vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
-	if (unlikely(!spin_trylock(vmf->ptl)))
+	if (unlikely(!spin_trylock(vmf->ptl))) {
+		trace_spf_pte_lock(_RET_IP_, vmf->vma, vmf->address);
 		goto out;
+	}
 
 	if (vma_has_changed(vmf)) {
 		spin_unlock(vmf->ptl);
+		trace_spf_vma_changed(_RET_IP_, vmf->vma, vmf->address);
 		goto out;
 	}
 
@@ -2466,8 +2474,10 @@ static bool pte_map_lock(struct vm_fault *vmf)
 	 * block on the PTL and thus we're safe.
 	 */
 	local_irq_disable();
-	if (vma_has_changed(vmf))
+	if (vma_has_changed(vmf)) {
+		trace_spf_vma_changed(_RET_IP_, vmf->vma, vmf->address);
 		goto out;
+	}
 
 	/*
 	 * Same as pte_offset_map_lock() except that we call
@@ -2480,11 +2490,13 @@ static bool pte_map_lock(struct vm_fault *vmf)
 	pte = pte_offset_map(vmf->pmd, vmf->address);
 	if (unlikely(!spin_trylock(ptl))) {
 		pte_unmap(pte);
+		trace_spf_pte_lock(_RET_IP_, vmf->vma, vmf->address);
 		goto out;
 	}
 
 	if (vma_has_changed(vmf)) {
 		pte_unmap_unlock(pte, ptl);
+		trace_spf_vma_changed(_RET_IP_, vmf->vma, vmf->address);
 		goto out;
 	}
 
@@ -4164,32 +4176,45 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	 * Validate the VMA found by the lockless lookup.
 	 */
 	dead = RB_EMPTY_NODE(&vma->vm_rb);
+	if (dead) {
+		trace_spf_vma_dead(_RET_IP_, vma, address);
+		goto unlock;
+	}
+
 	seq = raw_read_seqcount(&vma->vm_sequence); /* rmb <-> seqlock,vma_rb_erase() */
-	if ((seq & 1) || dead)
+	if (seq & 1) {
+		trace_spf_vma_changed(_RET_IP_, vma, address);
 		goto unlock;
+	}
 
 	/*
 	 * Can't call vm_ops service has we don't know what they would do
 	 * with the VMA.
 	 * This include huge page from hugetlbfs.
 	 */
-	if (vma->vm_ops)
+	if (vma->vm_ops) {
+		trace_spf_vma_notsup(_RET_IP_, vma, address);
 		goto unlock;
+	}
 
 	/*
 	 * __anon_vma_prepare() requires the mmap_sem to be held
 	 * because vm_next and vm_prev must be safe. This can't be guaranteed
 	 * in the speculative path.
 	 */
-	if (unlikely(!vma->anon_vma))
+	if (unlikely(!vma->anon_vma)) {
+		trace_spf_vma_notsup(_RET_IP_, vma, address);
 		goto unlock;
+	}
 
 	vmf.vma_flags = READ_ONCE(vma->vm_flags);
 	vmf.vma_page_prot = READ_ONCE(vma->vm_page_prot);
 
 	/* Can't call userland page fault handler in the speculative path */
-	if (unlikely(vmf.vma_flags & VM_UFFD_MISSING))
+	if (unlikely(vmf.vma_flags & VM_UFFD_MISSING)) {
+		trace_spf_vma_notsup(_RET_IP_, vma, address);
 		goto unlock;
+	}
 
 #ifdef CONFIG_NUMA
 	/*
@@ -4199,25 +4224,32 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	pol = __get_vma_policy(vma, address);
 	if (!pol)
 		pol = get_task_policy(current);
-	if (pol && pol->mode == MPOL_INTERLEAVE)
+	if (pol && pol->mode == MPOL_INTERLEAVE) {
+		trace_spf_vma_notsup(_RET_IP_, vma, address);
 		goto unlock;
+	}
 #endif
 
-	if (vmf.vma_flags & VM_GROWSDOWN || vmf.vma_flags & VM_GROWSUP)
+	if (vmf.vma_flags & VM_GROWSDOWN || vmf.vma_flags & VM_GROWSUP) {
 		/*
 		 * This could be detected by the check address against VMA's
 		 * boundaries but we want to trace it as not supported instead
 		 * of changed.
 		 */
+		trace_spf_vma_notsup(_RET_IP_, vma, address);
 		goto unlock;
+	}
 
 	if (address < READ_ONCE(vma->vm_start)
-	    || READ_ONCE(vma->vm_end) <= address)
+	    || READ_ONCE(vma->vm_end) <= address) {
+		trace_spf_vma_changed(_RET_IP_, vma, address);
 		goto unlock;
+	}
 
 	if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
 				       flags & FAULT_FLAG_INSTRUCTION,
 				       flags & FAULT_FLAG_REMOTE)) {
+		trace_spf_vma_access(_RET_IP_, vma, address);
 		ret = VM_FAULT_SIGSEGV;
 		goto unlock;
 	}
@@ -4225,10 +4257,12 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	/* This is one is required to check that the VMA has write access set */
 	if (flags & FAULT_FLAG_WRITE) {
 		if (unlikely(!(vmf.vma_flags & VM_WRITE))) {
+			trace_spf_vma_access(_RET_IP_, vma, address);
 			ret = VM_FAULT_SIGSEGV;
 			goto unlock;
 		}
 	} else if (unlikely(!(vmf.vma_flags & (VM_READ|VM_EXEC|VM_WRITE)))) {
+		trace_spf_vma_access(_RET_IP_, vma, address);
 		ret = VM_FAULT_SIGSEGV;
 		goto unlock;
 	}
@@ -4282,8 +4316,10 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	 * We need to re-validate the VMA after checking the bounds, otherwise
 	 * we might have a false positive on the bounds.
 	 */
-	if (read_seqcount_retry(&vma->vm_sequence, seq))
+	if (read_seqcount_retry(&vma->vm_sequence, seq)) {
+		trace_spf_vma_changed(_RET_IP_, vma, address);
 		goto unlock;
+	}
 
 	ret = handle_pte_fault(&vmf);
 
@@ -4292,6 +4328,7 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
 	return ret;
 
 out_walk:
+	trace_spf_vma_notsup(_RET_IP_, vma, address);
 	local_irq_enable();
 	goto unlock;
 }
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 17/20] perf: Add a speculative page fault sw event
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (15 preceding siblings ...)
  2017-09-08 18:07 ` [PATCH v3 16/20] mm: Adding speculative page fault failure trace events Laurent Dufour
@ 2017-09-08 18:07 ` Laurent Dufour
  2017-09-08 18:07 ` [PATCH v3 18/20] perf tools: Add support for the SPF perf event Laurent Dufour
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:07 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

Add a new software event to count succeeded speculative page faults.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 include/uapi/linux/perf_event.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 140ae638cfd6..101e509ee39b 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -111,6 +111,7 @@ enum perf_sw_ids {
 	PERF_COUNT_SW_EMULATION_FAULTS		= 8,
 	PERF_COUNT_SW_DUMMY			= 9,
 	PERF_COUNT_SW_BPF_OUTPUT		= 10,
+	PERF_COUNT_SW_SPF			= 11,
 
 	PERF_COUNT_SW_MAX,			/* non-ABI */
 };
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 18/20] perf tools: Add support for the SPF perf event
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (16 preceding siblings ...)
  2017-09-08 18:07 ` [PATCH v3 17/20] perf: Add a speculative page fault sw event Laurent Dufour
@ 2017-09-08 18:07 ` Laurent Dufour
  2017-09-08 18:07 ` [PATCH v3 19/20] x86/mm: Add speculative pagefault handling Laurent Dufour
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:07 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

Add support for the new speculative faults event.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 tools/include/uapi/linux/perf_event.h | 1 +
 tools/perf/util/evsel.c               | 1 +
 tools/perf/util/parse-events.c        | 4 ++++
 tools/perf/util/parse-events.l        | 1 +
 tools/perf/util/python.c              | 1 +
 5 files changed, 8 insertions(+)

diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 2a37ae925d85..8ee9a661018d 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -111,6 +111,7 @@ enum perf_sw_ids {
 	PERF_COUNT_SW_EMULATION_FAULTS		= 8,
 	PERF_COUNT_SW_DUMMY			= 9,
 	PERF_COUNT_SW_BPF_OUTPUT		= 10,
+	PERF_COUNT_SW_SPF			= 11,
 
 	PERF_COUNT_SW_MAX,			/* non-ABI */
 };
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d9bd632ed7db..983e8104b523 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -432,6 +432,7 @@ const char *perf_evsel__sw_names[PERF_COUNT_SW_MAX] = {
 	"alignment-faults",
 	"emulation-faults",
 	"dummy",
+	"speculative-faults",
 };
 
 static const char *__perf_evsel__sw_name(u64 config)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index f44aeba51d1f..ac2ebf99e965 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -135,6 +135,10 @@ struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
 		.symbol = "bpf-output",
 		.alias  = "",
 	},
+	[PERF_COUNT_SW_SPF] = {
+		.symbol = "speculative-faults",
+		.alias	= "spf",
+	},
 };
 
 #define __PERF_EVENT_FIELD(config, name) \
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index c42edeac451f..af3e52fb9103 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -289,6 +289,7 @@ alignment-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_AL
 emulation-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
 dummy						{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
 bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUTPUT); }
+speculative-faults|spf				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_SPF); }
 
 	/*
 	 * We have to handle the kernel PMU event cycles-ct/cycles-t/mem-loads/mem-stores separately.
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index c129e99114ae..12209adb7cb5 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1141,6 +1141,7 @@ static struct {
 	PERF_CONST(COUNT_SW_ALIGNMENT_FAULTS),
 	PERF_CONST(COUNT_SW_EMULATION_FAULTS),
 	PERF_CONST(COUNT_SW_DUMMY),
+	PERF_CONST(COUNT_SW_SPF),
 
 	PERF_CONST(SAMPLE_IP),
 	PERF_CONST(SAMPLE_TID),
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 19/20] x86/mm: Add speculative pagefault handling
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (17 preceding siblings ...)
  2017-09-08 18:07 ` [PATCH v3 18/20] perf tools: Add support for the SPF perf event Laurent Dufour
@ 2017-09-08 18:07 ` Laurent Dufour
  2017-09-08 18:07 ` [PATCH v3 20/20] powerpc/mm: Add speculative page fault Laurent Dufour
  2017-09-18  7:15 ` [PATCH v3 00/20] Speculative page faults Laurent Dufour
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:07 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

From: Peter Zijlstra <peterz@infradead.org>

Try a speculative fault before acquiring mmap_sem, if it returns with
VM_FAULT_RETRY continue with the mmap_sem acquisition and do the
traditional fault.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[Clearing of FAULT_FLAG_ALLOW_RETRY is now done in
 handle_speculative_fault()]
[Retry with usual fault path in the case VM_ERROR is returned by
 handle_speculative_fault(). This allows signal to be delivered]
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 arch/x86/include/asm/pgtable_types.h |  7 +++++++
 arch/x86/mm/fault.c                  | 19 +++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index f1492473f10e..aadc8ecb91fb 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -257,6 +257,13 @@ enum page_cache_mode {
 #define PGD_IDENT_ATTR	 0x001		/* PRESENT (no other attributes) */
 #endif
 
+/*
+ * Advertise that we call the Speculative Page Fault handler.
+ */
+#ifdef CONFIG_X86_64
+#define __HAVE_ARCH_CALL_SPF
+#endif
+
 #ifdef CONFIG_X86_32
 # include <asm/pgtable_32_types.h>
 #else
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index b836a7274e12..652af5524f42 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1365,6 +1365,24 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
 	if (error_code & PF_INSTR)
 		flags |= FAULT_FLAG_INSTRUCTION;
 
+#ifdef __HAVE_ARCH_CALL_SPF
+	if (error_code & PF_USER) {
+		fault = handle_speculative_fault(mm, address, flags);
+
+		/*
+		 * We also check against VM_FAULT_ERROR because we have to
+		 * raise a signal by calling later mm_fault_error() which
+		 * requires the vma pointer to be set. So in that case,
+		 * we fall through the normal path.
+		 */
+		if (!(fault & VM_FAULT_RETRY || fault & VM_FAULT_ERROR)) {
+			perf_sw_event(PERF_COUNT_SW_SPF, 1,
+				      regs, address);
+			goto done;
+		}
+	}
+#endif /* __HAVE_ARCH_CALL_SPF */
+
 	/*
 	 * When running in the kernel we expect faults to occur only to
 	 * addresses in user space.  All other faults represent errors in
@@ -1474,6 +1492,7 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
 		return;
 	}
 
+done:
 	/*
 	 * Major/minor page fault accounting. If any of the events
 	 * returned VM_FAULT_MAJOR, we account it as a major fault.
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v3 20/20] powerpc/mm: Add speculative page fault
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (18 preceding siblings ...)
  2017-09-08 18:07 ` [PATCH v3 19/20] x86/mm: Add speculative pagefault handling Laurent Dufour
@ 2017-09-08 18:07 ` Laurent Dufour
  2017-09-18  7:15 ` [PATCH v3 00/20] Speculative page faults Laurent Dufour
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-08 18:07 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

This patch enable the speculative page fault on the PowerPC
architecture.

This will try a speculative page fault without holding the mmap_sem,
if it returns with VM_FAULT_RETRY, the mmap_sem is acquired and the
traditional page fault processing is done.

Support is only provide for BOOK3S_64 currently because:
- require CONFIG_PPC_STD_MMU because checks done in
  set_access_flags_filter()
- require BOOK3S because we can't support for book3e_hugetlb_preload()
  called by update_mmu_cache()

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h |  5 +++++
 arch/powerpc/mm/fault.c                      | 15 +++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index b9aff515b4de..470203daf97e 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -314,6 +314,11 @@ extern unsigned long pci_io_base;
 /* Advertise support for _PAGE_SPECIAL */
 #define __HAVE_ARCH_PTE_SPECIAL
 
+/* Advertise that we call the Speculative Page Fault handler */
+#if defined(CONFIG_PPC_BOOK3S_64)
+#define __HAVE_ARCH_CALL_SPF
+#endif
+
 #ifndef __ASSEMBLY__
 
 /*
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 4797d08581ce..97c7de242627 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -442,6 +442,20 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
 	if (is_exec)
 		flags |= FAULT_FLAG_INSTRUCTION;
 
+#if defined(__HAVE_ARCH_CALL_SPF)
+	if (is_user) {
+		/* let's try a speculative page fault without grabbing the
+		 * mmap_sem.
+		 */
+		fault = handle_speculative_fault(mm, address, flags);
+		if (!(fault & VM_FAULT_RETRY)) {
+			perf_sw_event(PERF_COUNT_SW_SPF, 1,
+				      regs, address);
+			goto done;
+		}
+	}
+#endif /* defined(__HAVE_ARCH_CALL_SPF) */
+
 	/* When running in the kernel we expect faults to occur only to
 	 * addresses in user space.  All other faults represent errors in the
 	 * kernel and should generate an OOPS.  Unfortunately, in the case of an
@@ -526,6 +540,7 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
 
 	up_read(&current->mm->mmap_sem);
 
+done:
 	if (unlikely(fault & VM_FAULT_ERROR))
 		return mm_fault_error(regs, address, fault);
 
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-08 18:06 ` [PATCH v3 04/20] mm: VMA sequence count Laurent Dufour
@ 2017-09-13 11:53   ` Sergey Senozhatsky
  2017-09-13 16:56     ` Laurent Dufour
  0 siblings, 1 reply; 33+ messages in thread
From: Sergey Senozhatsky @ 2017-09-13 11:53 UTC (permalink / raw)
  To: Laurent Dufour
  Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky, linux-kernel, linux-mm,
	haren, khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev,
	x86

Hi,

On (09/08/17 20:06), Laurent Dufour wrote:
[..]
> @@ -903,6 +910,7 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>  		mm->map_count--;
>  		mpol_put(vma_policy(next));
>  		kmem_cache_free(vm_area_cachep, next);
> +		write_seqcount_end(&next->vm_sequence);
>  		/*
>  		 * In mprotect's case 6 (see comments on vma_merge),
>  		 * we must remove another next too. It would clutter
> @@ -932,11 +940,14 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>  		if (remove_next == 2) {
>  			remove_next = 1;
>  			end = next->vm_end;
> +			write_seqcount_end(&vma->vm_sequence);
>  			goto again;
> -		}
> -		else if (next)
> +		} else if (next) {
> +			if (next != vma)
> +				write_seqcount_begin_nested(&next->vm_sequence,
> +							    SINGLE_DEPTH_NESTING);
>  			vma_gap_update(next);
> -		else {
> +		} else {
>  			/*
>  			 * If remove_next == 2 we obviously can't
>  			 * reach this path.
> @@ -962,6 +973,10 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>  	if (insert && file)
>  		uprobe_mmap(insert);
>  
> +	if (next && next != vma)
> +		write_seqcount_end(&next->vm_sequence);
> +	write_seqcount_end(&vma->vm_sequence);


ok, so what I got on my box is:

vm_munmap()  -> down_write_killable(&mm->mmap_sem)
 do_munmap()
  __split_vma()
   __vma_adjust()  -> write_seqcount_begin(&vma->vm_sequence)
                   -> write_seqcount_begin_nested(&next->vm_sequence, SINGLE_DEPTH_NESTING)

so this gives 3 dependencies  ->mmap_sem   ->   ->vm_seq
                              ->vm_seq     ->   ->vm_seq/1
                              ->mmap_sem   ->   ->vm_seq/1


SyS_mremap() -> down_write_killable(&current->mm->mmap_sem)
 move_vma()   -> write_seqcount_begin(&vma->vm_sequence)
              -> write_seqcount_begin_nested(&new_vma->vm_sequence, SINGLE_DEPTH_NESTING);
  move_page_tables()
   __pte_alloc()
    pte_alloc_one()
     __alloc_pages_nodemask()
      fs_reclaim_acquire()


I think here we have prepare_alloc_pages() call, that does

        -> fs_reclaim_acquire(gfp_mask)
        -> fs_reclaim_release(gfp_mask)

so that adds one more dependency  ->mmap_sem   ->   ->vm_seq    ->   fs_reclaim
                                  ->mmap_sem   ->   ->vm_seq/1  ->   fs_reclaim


now, under memory pressure we hit the slow path and perform direct
reclaim. direct reclaim is done under fs_reclaim lock, so we end up
with the following call chain

__alloc_pages_nodemask()
 __alloc_pages_slowpath()
  __perform_reclaim()       ->   fs_reclaim_acquire(gfp_mask);
   try_to_free_pages()
    shrink_node()
     shrink_active_list()
      rmap_walk_file()      ->   i_mmap_lock_read(mapping);


and this break the existing dependency. since we now take the leaf lock
(fs_reclaim) first and the the root lock (->mmap_sem).


well, seems to be the case.

	-ss

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-13 11:53   ` Sergey Senozhatsky
@ 2017-09-13 16:56     ` Laurent Dufour
  2017-09-14  0:31       ` Sergey Senozhatsky
  0 siblings, 1 reply; 33+ messages in thread
From: Laurent Dufour @ 2017-09-13 16:56 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky, linux-kernel, linux-mm,
	haren, khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev,
	x86

Hi Sergey,

On 13/09/2017 13:53, Sergey Senozhatsky wrote:
> Hi,
> 
> On (09/08/17 20:06), Laurent Dufour wrote:
> [..]
>> @@ -903,6 +910,7 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>>  		mm->map_count--;
>>  		mpol_put(vma_policy(next));
>>  		kmem_cache_free(vm_area_cachep, next);
>> +		write_seqcount_end(&next->vm_sequence);
>>  		/*
>>  		 * In mprotect's case 6 (see comments on vma_merge),
>>  		 * we must remove another next too. It would clutter
>> @@ -932,11 +940,14 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>>  		if (remove_next == 2) {
>>  			remove_next = 1;
>>  			end = next->vm_end;
>> +			write_seqcount_end(&vma->vm_sequence);
>>  			goto again;
>> -		}
>> -		else if (next)
>> +		} else if (next) {
>> +			if (next != vma)
>> +				write_seqcount_begin_nested(&next->vm_sequence,
>> +							    SINGLE_DEPTH_NESTING);
>>  			vma_gap_update(next);
>> -		else {
>> +		} else {
>>  			/*
>>  			 * If remove_next == 2 we obviously can't
>>  			 * reach this path.
>> @@ -962,6 +973,10 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>>  	if (insert && file)
>>  		uprobe_mmap(insert);
>>  
>> +	if (next && next != vma)
>> +		write_seqcount_end(&next->vm_sequence);
>> +	write_seqcount_end(&vma->vm_sequence);
> 
> 
> ok, so what I got on my box is:
> 
> vm_munmap()  -> down_write_killable(&mm->mmap_sem)
>  do_munmap()
>   __split_vma()
>    __vma_adjust()  -> write_seqcount_begin(&vma->vm_sequence)
>                    -> write_seqcount_begin_nested(&next->vm_sequence, SINGLE_DEPTH_NESTING)
> 
> so this gives 3 dependencies  ->mmap_sem   ->   ->vm_seq
>                               ->vm_seq     ->   ->vm_seq/1
>                               ->mmap_sem   ->   ->vm_seq/1
> 
> 
> SyS_mremap() -> down_write_killable(&current->mm->mmap_sem)
>  move_vma()   -> write_seqcount_begin(&vma->vm_sequence)
>               -> write_seqcount_begin_nested(&new_vma->vm_sequence, SINGLE_DEPTH_NESTING);
>   move_page_tables()
>    __pte_alloc()
>     pte_alloc_one()
>      __alloc_pages_nodemask()
>       fs_reclaim_acquire()
> 
> 
> I think here we have prepare_alloc_pages() call, that does
> 
>         -> fs_reclaim_acquire(gfp_mask)
>         -> fs_reclaim_release(gfp_mask)
> 
> so that adds one more dependency  ->mmap_sem   ->   ->vm_seq    ->   fs_reclaim
>                                   ->mmap_sem   ->   ->vm_seq/1  ->   fs_reclaim
> 
> 
> now, under memory pressure we hit the slow path and perform direct
> reclaim. direct reclaim is done under fs_reclaim lock, so we end up
> with the following call chain
> 
> __alloc_pages_nodemask()
>  __alloc_pages_slowpath()
>   __perform_reclaim()       ->   fs_reclaim_acquire(gfp_mask);
>    try_to_free_pages()
>     shrink_node()
>      shrink_active_list()
>       rmap_walk_file()      ->   i_mmap_lock_read(mapping);
> 
> 
> and this break the existing dependency. since we now take the leaf lock
> (fs_reclaim) first and the the root lock (->mmap_sem).

Thanks for looking at this.
I'm sorry, I should have miss something.

My understanding is that there are 2 chains of locks:
 1. from __vma_adjust() mmap_sem -> i_mmap_rwsem -> vm_seq
 2. from move_vmap() mmap_sem -> vm_seq -> fs_reclaim
 2. from __alloc_pages_nodemask() fs_reclaim -> i_mmap_rwsem

So the solution would be to have in __vma_adjust()
 mmap_sem -> vm_seq -> i_mmap_rwsem

But this will raised the following dependency from  unmap_mapping_range()
unmap_mapping_range() 		-> i_mmap_rwsem
 unmap_mapping_range_tree()
  unmap_mapping_range_vma()
   zap_page_range_single()
    unmap_single_vma()
     unmap_page_range()	 	-> vm_seq

And there is no way to get rid of it easily as in unmap_mapping_range()
there is no VMA identified yet.

That's being said I can't see any clear way to get lock dependency cleaned
here.
Furthermore, this is not clear to me how a deadlock could happen as vm_seq
is a sequence lock, and there is no way to get blocked here.

Cheers,
Laurent.

> 
> well, seems to be the case.
> 
> 	-ss
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-13 16:56     ` Laurent Dufour
@ 2017-09-14  0:31       ` Sergey Senozhatsky
  2017-09-14  7:55         ` Laurent Dufour
  0 siblings, 1 reply; 33+ messages in thread
From: Sergey Senozhatsky @ 2017-09-14  0:31 UTC (permalink / raw)
  To: Laurent Dufour
  Cc: Sergey Senozhatsky, paulmck, peterz, akpm, kirill, ak, mhocko,
	dave, jack, Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner,
	Ingo Molnar, hpa, Will Deacon, Sergey Senozhatsky, linux-kernel,
	linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
	linuxppc-dev, x86

Hi,

On (09/13/17 18:56), Laurent Dufour wrote:
> Hi Sergey,
> 
> On 13/09/2017 13:53, Sergey Senozhatsky wrote:
> > Hi,
> > 
> > On (09/08/17 20:06), Laurent Dufour wrote:
[..]
> > ok, so what I got on my box is:
> > 
> > vm_munmap()  -> down_write_killable(&mm->mmap_sem)
> >  do_munmap()
> >   __split_vma()
> >    __vma_adjust()  -> write_seqcount_begin(&vma->vm_sequence)
> >                    -> write_seqcount_begin_nested(&next->vm_sequence, SINGLE_DEPTH_NESTING)
> > 
> > so this gives 3 dependencies  ->mmap_sem   ->   ->vm_seq
> >                               ->vm_seq     ->   ->vm_seq/1
> >                               ->mmap_sem   ->   ->vm_seq/1
> > 
> > 
> > SyS_mremap() -> down_write_killable(&current->mm->mmap_sem)
> >  move_vma()   -> write_seqcount_begin(&vma->vm_sequence)
> >               -> write_seqcount_begin_nested(&new_vma->vm_sequence, SINGLE_DEPTH_NESTING);
> >   move_page_tables()
> >    __pte_alloc()
> >     pte_alloc_one()
> >      __alloc_pages_nodemask()
> >       fs_reclaim_acquire()
> > 
> > 
> > I think here we have prepare_alloc_pages() call, that does
> > 
> >         -> fs_reclaim_acquire(gfp_mask)
> >         -> fs_reclaim_release(gfp_mask)
> > 
> > so that adds one more dependency  ->mmap_sem   ->   ->vm_seq    ->   fs_reclaim
> >                                   ->mmap_sem   ->   ->vm_seq/1  ->   fs_reclaim
> > 
> > 
> > now, under memory pressure we hit the slow path and perform direct
> > reclaim. direct reclaim is done under fs_reclaim lock, so we end up
> > with the following call chain
> > 
> > __alloc_pages_nodemask()
> >  __alloc_pages_slowpath()
> >   __perform_reclaim()       ->   fs_reclaim_acquire(gfp_mask);
> >    try_to_free_pages()
> >     shrink_node()
> >      shrink_active_list()
> >       rmap_walk_file()      ->   i_mmap_lock_read(mapping);
> > 
> > 
> > and this break the existing dependency. since we now take the leaf lock
> > (fs_reclaim) first and the the root lock (->mmap_sem).
> 
> Thanks for looking at this.
> I'm sorry, I should have miss something.

no prob :)


> My understanding is that there are 2 chains of locks:
>  1. from __vma_adjust() mmap_sem -> i_mmap_rwsem -> vm_seq
>  2. from move_vmap() mmap_sem -> vm_seq -> fs_reclaim
>  2. from __alloc_pages_nodemask() fs_reclaim -> i_mmap_rwsem

yes, as far as lockdep warning suggests.

> So the solution would be to have in __vma_adjust()
>  mmap_sem -> vm_seq -> i_mmap_rwsem
> 
> But this will raised the following dependency from  unmap_mapping_range()
> unmap_mapping_range() 		-> i_mmap_rwsem
>  unmap_mapping_range_tree()
>   unmap_mapping_range_vma()
>    zap_page_range_single()
>     unmap_single_vma()
>      unmap_page_range()	 	-> vm_seq
> 
> And there is no way to get rid of it easily as in unmap_mapping_range()
> there is no VMA identified yet.
> 
> That's being said I can't see any clear way to get lock dependency cleaned
> here.
> Furthermore, this is not clear to me how a deadlock could happen as vm_seq
> is a sequence lock, and there is no way to get blocked here.

as far as I understand,
   seq locks can deadlock, technically. not on the write() side, but on
the read() side:

read_seqcount_begin()
 raw_read_seqcount_begin()
   __read_seqcount_begin()

and __read_seqcount_begin() spins for ever

   __read_seqcount_begin()
   {
    repeat:
     ret = READ_ONCE(s->sequence);
     if (unlikely(ret & 1)) {
         cpu_relax();
         goto repeat;
     }
     return ret;
   }


so if there are two CPUs, one doing write_seqcount() and the other one
doing read_seqcount() then what can happen is something like this

	CPU0					CPU1

						fs_reclaim_acquire()
	write_seqcount_begin()
	fs_reclaim_acquire()			read_seqcount_begin()
	write_seqcount_end()

CPU0 can't write_seqcount_end() because of fs_reclaim_acquire() from
CPU1, CPU1 can't read_seqcount_begin() because CPU0 did write_seqcount_begin()
and now waits for fs_reclaim_acquire(). makes sense?

	-ss

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-14  0:31       ` Sergey Senozhatsky
@ 2017-09-14  7:55         ` Laurent Dufour
  2017-09-14  8:13           ` Sergey Senozhatsky
  0 siblings, 1 reply; 33+ messages in thread
From: Laurent Dufour @ 2017-09-14  7:55 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky, linux-kernel, linux-mm,
	haren, khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev,
	x86

Hi,

On 14/09/2017 02:31, Sergey Senozhatsky wrote:
> Hi,
> 
> On (09/13/17 18:56), Laurent Dufour wrote:
>> Hi Sergey,
>>
>> On 13/09/2017 13:53, Sergey Senozhatsky wrote:
>>> Hi,
>>>
>>> On (09/08/17 20:06), Laurent Dufour wrote:
> [..]
>>> ok, so what I got on my box is:
>>>
>>> vm_munmap()  -> down_write_killable(&mm->mmap_sem)
>>>  do_munmap()
>>>   __split_vma()
>>>    __vma_adjust()  -> write_seqcount_begin(&vma->vm_sequence)
>>>                    -> write_seqcount_begin_nested(&next->vm_sequence, SINGLE_DEPTH_NESTING)
>>>
>>> so this gives 3 dependencies  ->mmap_sem   ->   ->vm_seq
>>>                               ->vm_seq     ->   ->vm_seq/1
>>>                               ->mmap_sem   ->   ->vm_seq/1
>>>
>>>
>>> SyS_mremap() -> down_write_killable(&current->mm->mmap_sem)
>>>  move_vma()   -> write_seqcount_begin(&vma->vm_sequence)
>>>               -> write_seqcount_begin_nested(&new_vma->vm_sequence, SINGLE_DEPTH_NESTING);
>>>   move_page_tables()
>>>    __pte_alloc()
>>>     pte_alloc_one()
>>>      __alloc_pages_nodemask()
>>>       fs_reclaim_acquire()
>>>
>>>
>>> I think here we have prepare_alloc_pages() call, that does
>>>
>>>         -> fs_reclaim_acquire(gfp_mask)
>>>         -> fs_reclaim_release(gfp_mask)
>>>
>>> so that adds one more dependency  ->mmap_sem   ->   ->vm_seq    ->   fs_reclaim
>>>                                   ->mmap_sem   ->   ->vm_seq/1  ->   fs_reclaim
>>>
>>>
>>> now, under memory pressure we hit the slow path and perform direct
>>> reclaim. direct reclaim is done under fs_reclaim lock, so we end up
>>> with the following call chain
>>>
>>> __alloc_pages_nodemask()
>>>  __alloc_pages_slowpath()
>>>   __perform_reclaim()       ->   fs_reclaim_acquire(gfp_mask);
>>>    try_to_free_pages()
>>>     shrink_node()
>>>      shrink_active_list()
>>>       rmap_walk_file()      ->   i_mmap_lock_read(mapping);
>>>
>>>
>>> and this break the existing dependency. since we now take the leaf lock
>>> (fs_reclaim) first and the the root lock (->mmap_sem).
>>
>> Thanks for looking at this.
>> I'm sorry, I should have miss something.
> 
> no prob :)
> 
> 
>> My understanding is that there are 2 chains of locks:
>>  1. from __vma_adjust() mmap_sem -> i_mmap_rwsem -> vm_seq
>>  2. from move_vmap() mmap_sem -> vm_seq -> fs_reclaim
>>  2. from __alloc_pages_nodemask() fs_reclaim -> i_mmap_rwsem
> 
> yes, as far as lockdep warning suggests.
> 
>> So the solution would be to have in __vma_adjust()
>>  mmap_sem -> vm_seq -> i_mmap_rwsem
>>
>> But this will raised the following dependency from  unmap_mapping_range()
>> unmap_mapping_range() 		-> i_mmap_rwsem
>>  unmap_mapping_range_tree()
>>   unmap_mapping_range_vma()
>>    zap_page_range_single()
>>     unmap_single_vma()
>>      unmap_page_range()	 	-> vm_seq
>>
>> And there is no way to get rid of it easily as in unmap_mapping_range()
>> there is no VMA identified yet.
>>
>> That's being said I can't see any clear way to get lock dependency cleaned
>> here.
>> Furthermore, this is not clear to me how a deadlock could happen as vm_seq
>> is a sequence lock, and there is no way to get blocked here.
> 
> as far as I understand,
>    seq locks can deadlock, technically. not on the write() side, but on
> the read() side:
> 
> read_seqcount_begin()
>  raw_read_seqcount_begin()
>    __read_seqcount_begin()
> 
> and __read_seqcount_begin() spins for ever
> 
>    __read_seqcount_begin()
>    {
>     repeat:
>      ret = READ_ONCE(s->sequence);
>      if (unlikely(ret & 1)) {
>          cpu_relax();
>          goto repeat;
>      }
>      return ret;
>    }
> 
> 
> so if there are two CPUs, one doing write_seqcount() and the other one
> doing read_seqcount() then what can happen is something like this
> 
> 	CPU0					CPU1
> 
> 						fs_reclaim_acquire()
> 	write_seqcount_begin()
> 	fs_reclaim_acquire()			read_seqcount_begin()
> 	write_seqcount_end()
> 
> CPU0 can't write_seqcount_end() because of fs_reclaim_acquire() from
> CPU1, CPU1 can't read_seqcount_begin() because CPU0 did write_seqcount_begin()
> and now waits for fs_reclaim_acquire(). makes sense?

Yes, this makes sense.

But in the case of this series, there is no call to
__read_seqcount_begin(), and the reader (the speculative page fault
handler), is just checking for (vm_seq & 1) and if this is true, simply
exit the speculative path without waiting.
So there is no deadlock possibility.

The bad case would be to have 2 concurrent threads calling
write_seqcount_begin() on the same VMA, leading a wrongly freed sequence
lock but this can't happen because of the mmap_sem holding for write in
such a case.

Cheers,
Laurent.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-14  7:55         ` Laurent Dufour
@ 2017-09-14  8:13           ` Sergey Senozhatsky
  2017-09-14  8:58             ` Laurent Dufour
  0 siblings, 1 reply; 33+ messages in thread
From: Sergey Senozhatsky @ 2017-09-14  8:13 UTC (permalink / raw)
  To: Laurent Dufour
  Cc: Sergey Senozhatsky, paulmck, peterz, akpm, kirill, ak, mhocko,
	dave, jack, Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner,
	Ingo Molnar, hpa, Will Deacon, Sergey Senozhatsky, linux-kernel,
	linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
	linuxppc-dev, x86

Hi,

On (09/14/17 09:55), Laurent Dufour wrote:
[..]
> > so if there are two CPUs, one doing write_seqcount() and the other one
> > doing read_seqcount() then what can happen is something like this
> > 
> > 	CPU0					CPU1
> > 
> > 						fs_reclaim_acquire()
> > 	write_seqcount_begin()
> > 	fs_reclaim_acquire()			read_seqcount_begin()
> > 	write_seqcount_end()
> > 
> > CPU0 can't write_seqcount_end() because of fs_reclaim_acquire() from
> > CPU1, CPU1 can't read_seqcount_begin() because CPU0 did write_seqcount_begin()
> > and now waits for fs_reclaim_acquire(). makes sense?
> 
> Yes, this makes sense.
> 
> But in the case of this series, there is no call to
> __read_seqcount_begin(), and the reader (the speculative page fault
> handler), is just checking for (vm_seq & 1) and if this is true, simply
> exit the speculative path without waiting.
> So there is no deadlock possibility.

probably lockdep just knows that those locks interleave at some
point.


by the way, I think there is one path that can spin

find_vma_srcu()
 read_seqbegin()
  read_seqcount_begin()
   raw_read_seqcount_begin()
    __read_seqcount_begin()

	-ss

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-14  8:13           ` Sergey Senozhatsky
@ 2017-09-14  8:58             ` Laurent Dufour
  2017-09-14  9:11               ` Sergey Senozhatsky
  0 siblings, 1 reply; 33+ messages in thread
From: Laurent Dufour @ 2017-09-14  8:58 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky, linux-kernel, linux-mm,
	haren, khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev,
	x86

On 14/09/2017 10:13, Sergey Senozhatsky wrote:
> Hi,
> 
> On (09/14/17 09:55), Laurent Dufour wrote:
> [..]
>>> so if there are two CPUs, one doing write_seqcount() and the other one
>>> doing read_seqcount() then what can happen is something like this
>>>
>>> 	CPU0					CPU1
>>>
>>> 						fs_reclaim_acquire()
>>> 	write_seqcount_begin()
>>> 	fs_reclaim_acquire()			read_seqcount_begin()
>>> 	write_seqcount_end()
>>>
>>> CPU0 can't write_seqcount_end() because of fs_reclaim_acquire() from
>>> CPU1, CPU1 can't read_seqcount_begin() because CPU0 did write_seqcount_begin()
>>> and now waits for fs_reclaim_acquire(). makes sense?
>>
>> Yes, this makes sense.
>>
>> But in the case of this series, there is no call to
>> __read_seqcount_begin(), and the reader (the speculative page fault
>> handler), is just checking for (vm_seq & 1) and if this is true, simply
>> exit the speculative path without waiting.
>> So there is no deadlock possibility.
> 
> probably lockdep just knows that those locks interleave at some
> point.
> 
> 
> by the way, I think there is one path that can spin
> 
> find_vma_srcu()
>  read_seqbegin()
>   read_seqcount_begin()
>    raw_read_seqcount_begin()
>     __read_seqcount_begin()


That's right, but here this is the  sequence counter mm->mm_seq, not the
vm_seq one.

This one is held to protect walking the VMA list "locklessly"...

Cheers,
Laurent.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-14  8:58             ` Laurent Dufour
@ 2017-09-14  9:11               ` Sergey Senozhatsky
  2017-09-14  9:15                 ` Laurent Dufour
  0 siblings, 1 reply; 33+ messages in thread
From: Sergey Senozhatsky @ 2017-09-14  9:11 UTC (permalink / raw)
  To: Laurent Dufour
  Cc: Sergey Senozhatsky, paulmck, peterz, akpm, kirill, ak, mhocko,
	dave, jack, Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner,
	Ingo Molnar, hpa, Will Deacon, Sergey Senozhatsky, linux-kernel,
	linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
	linuxppc-dev, x86

On (09/14/17 10:58), Laurent Dufour wrote:
[..]
> That's right, but here this is the  sequence counter mm->mm_seq, not the
> vm_seq one.

d'oh... you are right.

	-ss

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-14  9:11               ` Sergey Senozhatsky
@ 2017-09-14  9:15                 ` Laurent Dufour
  2017-09-14  9:40                   ` Sergey Senozhatsky
  0 siblings, 1 reply; 33+ messages in thread
From: Laurent Dufour @ 2017-09-14  9:15 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky, linux-kernel, linux-mm,
	haren, khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev,
	x86

On 14/09/2017 11:11, Sergey Senozhatsky wrote:
> On (09/14/17 10:58), Laurent Dufour wrote:
> [..]
>> That's right, but here this is the  sequence counter mm->mm_seq, not the
>> vm_seq one.
> 
> d'oh... you are right.

So I'm doubting about the probability of a deadlock here, but I don't like
to see lockdep complaining. Is there an easy way to make it happy ?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-14  9:15                 ` Laurent Dufour
@ 2017-09-14  9:40                   ` Sergey Senozhatsky
  2017-09-15 12:38                     ` Laurent Dufour
  0 siblings, 1 reply; 33+ messages in thread
From: Sergey Senozhatsky @ 2017-09-14  9:40 UTC (permalink / raw)
  To: Laurent Dufour
  Cc: Sergey Senozhatsky, paulmck, peterz, akpm, kirill, ak, mhocko,
	dave, jack, Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner,
	Ingo Molnar, hpa, Will Deacon, Sergey Senozhatsky, linux-kernel,
	linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
	linuxppc-dev, x86

On (09/14/17 11:15), Laurent Dufour wrote:
> On 14/09/2017 11:11, Sergey Senozhatsky wrote:
> > On (09/14/17 10:58), Laurent Dufour wrote:
> > [..]
> >> That's right, but here this is the  sequence counter mm->mm_seq, not the
> >> vm_seq one.
> > 
> > d'oh... you are right.
> 
> So I'm doubting about the probability of a deadlock here, but I don't like
> to see lockdep complaining. Is there an easy way to make it happy ?


 /*
  * well... answering your question - it seems raw versions of seqcount
  * functions don't call lockdep's lock_acquire/lock_release...
  *
  * but I have never told you that. never.
  */


lockdep, perhaps, can be wrong sometimes, and may be it's one of those
cases. may be not... I'm not a MM guy myself.

below is a lockdep splat I got yesterday. that's v3 of SPF patch set.


[ 2763.365898] ======================================================
[ 2763.365899] WARNING: possible circular locking dependency detected
[ 2763.365902] 4.13.0-next-20170913-dbg-00039-ge3c06ea4b028-dirty #1837 Not tainted
[ 2763.365903] ------------------------------------------------------
[ 2763.365905] khugepaged/42 is trying to acquire lock:
[ 2763.365906]  (&mapping->i_mmap_rwsem){++++}, at: [<ffffffff811181cc>] rmap_walk_file+0x5a/0x142
[ 2763.365913] 
               but task is already holding lock:
[ 2763.365915]  (fs_reclaim){+.+.}, at: [<ffffffff810e99dc>] fs_reclaim_acquire+0x12/0x35
[ 2763.365920] 
               which lock already depends on the new lock.

[ 2763.365922] 
               the existing dependency chain (in reverse order) is:
[ 2763.365924] 
               -> #3 (fs_reclaim){+.+.}:
[ 2763.365930]        lock_acquire+0x176/0x19e
[ 2763.365932]        fs_reclaim_acquire+0x32/0x35
[ 2763.365934]        __alloc_pages_nodemask+0x6d/0x1f9
[ 2763.365937]        pte_alloc_one+0x17/0x62
[ 2763.365940]        __pte_alloc+0x1f/0x83
[ 2763.365943]        move_page_tables+0x2c3/0x5a2
[ 2763.365944]        move_vma.isra.25+0xff/0x29f
[ 2763.365946]        SyS_mremap+0x41b/0x49e
[ 2763.365949]        entry_SYSCALL_64_fastpath+0x18/0xad
[ 2763.365951] 
               -> #2 (&vma->vm_sequence/1){+.+.}:
[ 2763.365955]        lock_acquire+0x176/0x19e
[ 2763.365958]        write_seqcount_begin_nested+0x1b/0x1d
[ 2763.365959]        __vma_adjust+0x1c4/0x5f1
[ 2763.365961]        __split_vma+0x12c/0x181
[ 2763.365963]        do_munmap+0x128/0x2af
[ 2763.365965]        vm_munmap+0x5a/0x73
[ 2763.365968]        elf_map+0xb1/0xce
[ 2763.365970]        load_elf_binary+0x91e/0x137a
[ 2763.365973]        search_binary_handler+0x70/0x1f3
[ 2763.365974]        do_execveat_common+0x45e/0x68e
[ 2763.365978]        call_usermodehelper_exec_async+0xf7/0x11f
[ 2763.365980]        ret_from_fork+0x27/0x40
[ 2763.365981] 
               -> #1 (&vma->vm_sequence){+.+.}:
[ 2763.365985]        lock_acquire+0x176/0x19e
[ 2763.365987]        write_seqcount_begin_nested+0x1b/0x1d
[ 2763.365989]        __vma_adjust+0x1a9/0x5f1
[ 2763.365991]        __split_vma+0x12c/0x181
[ 2763.365993]        do_munmap+0x128/0x2af
[ 2763.365994]        vm_munmap+0x5a/0x73
[ 2763.365996]        elf_map+0xb1/0xce
[ 2763.365998]        load_elf_binary+0x91e/0x137a
[ 2763.365999]        search_binary_handler+0x70/0x1f3
[ 2763.366001]        do_execveat_common+0x45e/0x68e
[ 2763.366003]        call_usermodehelper_exec_async+0xf7/0x11f
[ 2763.366005]        ret_from_fork+0x27/0x40
[ 2763.366006] 
               -> #0 (&mapping->i_mmap_rwsem){++++}:
[ 2763.366010]        __lock_acquire+0xa72/0xca0
[ 2763.366012]        lock_acquire+0x176/0x19e
[ 2763.366015]        down_read+0x3b/0x55
[ 2763.366017]        rmap_walk_file+0x5a/0x142
[ 2763.366018]        page_referenced+0xfc/0x134
[ 2763.366022]        shrink_active_list+0x1ac/0x37d
[ 2763.366024]        shrink_node_memcg.constprop.72+0x3ca/0x567
[ 2763.366026]        shrink_node+0x3f/0x14c
[ 2763.366028]        try_to_free_pages+0x288/0x47a
[ 2763.366030]        __alloc_pages_slowpath+0x3a7/0xa49
[ 2763.366032]        __alloc_pages_nodemask+0xf1/0x1f9
[ 2763.366035]        khugepaged+0xc8/0x167c
[ 2763.366037]        kthread+0x133/0x13b
[ 2763.366039]        ret_from_fork+0x27/0x40
[ 2763.366040] 
               other info that might help us debug this:

[ 2763.366042] Chain exists of:
                 &mapping->i_mmap_rwsem --> &vma->vm_sequence/1 --> fs_reclaim

[ 2763.366048]  Possible unsafe locking scenario:

[ 2763.366049]        CPU0                    CPU1
[ 2763.366050]        ----                    ----
[ 2763.366051]   lock(fs_reclaim);
[ 2763.366054]                                lock(&vma->vm_sequence/1);
[ 2763.366056]                                lock(fs_reclaim);
[ 2763.366058]   lock(&mapping->i_mmap_rwsem);
[ 2763.366061] 
                *** DEADLOCK ***

[ 2763.366063] 1 lock held by khugepaged/42:
[ 2763.366064]  #0:  (fs_reclaim){+.+.}, at: [<ffffffff810e99dc>] fs_reclaim_acquire+0x12/0x35
[ 2763.366068] 
               stack backtrace:
[ 2763.366071] CPU: 2 PID: 42 Comm: khugepaged Not tainted 4.13.0-next-20170913-dbg-00039-ge3c06ea4b028-dirty #1837
[ 2763.366073] Call Trace:
[ 2763.366077]  dump_stack+0x67/0x8e
[ 2763.366080]  print_circular_bug+0x2a1/0x2af
[ 2763.366083]  ? graph_unlock+0x69/0x69
[ 2763.366085]  check_prev_add+0x76/0x20d
[ 2763.366087]  ? graph_unlock+0x69/0x69
[ 2763.366090]  __lock_acquire+0xa72/0xca0
[ 2763.366093]  ? __save_stack_trace+0xa3/0xbf
[ 2763.366096]  lock_acquire+0x176/0x19e
[ 2763.366098]  ? rmap_walk_file+0x5a/0x142
[ 2763.366100]  down_read+0x3b/0x55
[ 2763.366102]  ? rmap_walk_file+0x5a/0x142
[ 2763.366103]  rmap_walk_file+0x5a/0x142
[ 2763.366106]  page_referenced+0xfc/0x134
[ 2763.366108]  ? page_vma_mapped_walk_done.isra.17+0xb/0xb
[ 2763.366109]  ? page_get_anon_vma+0x6d/0x6d
[ 2763.366112]  shrink_active_list+0x1ac/0x37d
[ 2763.366115]  shrink_node_memcg.constprop.72+0x3ca/0x567
[ 2763.366118]  ? ___might_sleep+0xd5/0x234
[ 2763.366121]  shrink_node+0x3f/0x14c
[ 2763.366123]  try_to_free_pages+0x288/0x47a
[ 2763.366126]  __alloc_pages_slowpath+0x3a7/0xa49
[ 2763.366128]  ? ___might_sleep+0xd5/0x234
[ 2763.366131]  __alloc_pages_nodemask+0xf1/0x1f9
[ 2763.366133]  khugepaged+0xc8/0x167c
[ 2763.366138]  ? remove_wait_queue+0x47/0x47
[ 2763.366140]  ? collapse_shmem.isra.45+0x828/0x828
[ 2763.366142]  kthread+0x133/0x13b
[ 2763.366145]  ? __list_del_entry+0x1d/0x1d
[ 2763.366147]  ret_from_fork+0x27/0x40

	-ss

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-14  9:40                   ` Sergey Senozhatsky
@ 2017-09-15 12:38                     ` Laurent Dufour
  2017-09-25 12:22                       ` Peter Zijlstra
  0 siblings, 1 reply; 33+ messages in thread
From: Laurent Dufour @ 2017-09-15 12:38 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky, linux-kernel, linux-mm,
	haren, khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev,
	x86

Hi,

On 14/09/2017 11:40, Sergey Senozhatsky wrote:
> On (09/14/17 11:15), Laurent Dufour wrote:
>> On 14/09/2017 11:11, Sergey Senozhatsky wrote:
>>> On (09/14/17 10:58), Laurent Dufour wrote:
>>> [..]
>>>> That's right, but here this is the  sequence counter mm->mm_seq, not the
>>>> vm_seq one.
>>>
>>> d'oh... you are right.
>>
>> So I'm doubting about the probability of a deadlock here, but I don't like
>> to see lockdep complaining. Is there an easy way to make it happy ?
> 
> 
>  /*
>   * well... answering your question - it seems raw versions of seqcount
>   * functions don't call lockdep's lock_acquire/lock_release...
>   *
>   * but I have never told you that. never.
>   */

Hum... I'm not sure that would be the best way since in other case lockdep
checks are valid, but if getting rid of locked's warning is required to get
this series upstream, I'd use raw versions... Please advice...

> 
> lockdep, perhaps, can be wrong sometimes, and may be it's one of those
> cases. may be not... I'm not a MM guy myself.

>From the code reading I can't see any valid reason for a circular lock
dependency.

> below is a lockdep splat I got yesterday. that's v3 of SPF patch set.

This is exactly the same you got previously, and I still can't see how the
chain "&mapping->i_mmap_rwsem --> &vma->vm_sequence/1 --> fs_reclaim" could
happen.

Cheers,
Laurent.

> 
> [ 2763.365898] ======================================================
> [ 2763.365899] WARNING: possible circular locking dependency detected
> [ 2763.365902] 4.13.0-next-20170913-dbg-00039-ge3c06ea4b028-dirty #1837 Not tainted
> [ 2763.365903] ------------------------------------------------------
> [ 2763.365905] khugepaged/42 is trying to acquire lock:
> [ 2763.365906]  (&mapping->i_mmap_rwsem){++++}, at: [<ffffffff811181cc>] rmap_walk_file+0x5a/0x142
> [ 2763.365913] 
>                but task is already holding lock:
> [ 2763.365915]  (fs_reclaim){+.+.}, at: [<ffffffff810e99dc>] fs_reclaim_acquire+0x12/0x35
> [ 2763.365920] 
>                which lock already depends on the new lock.
> 
> [ 2763.365922] 
>                the existing dependency chain (in reverse order) is:
> [ 2763.365924] 
>                -> #3 (fs_reclaim){+.+.}:
> [ 2763.365930]        lock_acquire+0x176/0x19e
> [ 2763.365932]        fs_reclaim_acquire+0x32/0x35
> [ 2763.365934]        __alloc_pages_nodemask+0x6d/0x1f9
> [ 2763.365937]        pte_alloc_one+0x17/0x62
> [ 2763.365940]        __pte_alloc+0x1f/0x83
> [ 2763.365943]        move_page_tables+0x2c3/0x5a2
> [ 2763.365944]        move_vma.isra.25+0xff/0x29f
> [ 2763.365946]        SyS_mremap+0x41b/0x49e
> [ 2763.365949]        entry_SYSCALL_64_fastpath+0x18/0xad
> [ 2763.365951] 
>                -> #2 (&vma->vm_sequence/1){+.+.}:
> [ 2763.365955]        lock_acquire+0x176/0x19e
> [ 2763.365958]        write_seqcount_begin_nested+0x1b/0x1d
> [ 2763.365959]        __vma_adjust+0x1c4/0x5f1
> [ 2763.365961]        __split_vma+0x12c/0x181
> [ 2763.365963]        do_munmap+0x128/0x2af
> [ 2763.365965]        vm_munmap+0x5a/0x73
> [ 2763.365968]        elf_map+0xb1/0xce
> [ 2763.365970]        load_elf_binary+0x91e/0x137a
> [ 2763.365973]        search_binary_handler+0x70/0x1f3
> [ 2763.365974]        do_execveat_common+0x45e/0x68e
> [ 2763.365978]        call_usermodehelper_exec_async+0xf7/0x11f
> [ 2763.365980]        ret_from_fork+0x27/0x40
> [ 2763.365981] 
>                -> #1 (&vma->vm_sequence){+.+.}:
> [ 2763.365985]        lock_acquire+0x176/0x19e
> [ 2763.365987]        write_seqcount_begin_nested+0x1b/0x1d
> [ 2763.365989]        __vma_adjust+0x1a9/0x5f1
> [ 2763.365991]        __split_vma+0x12c/0x181
> [ 2763.365993]        do_munmap+0x128/0x2af
> [ 2763.365994]        vm_munmap+0x5a/0x73
> [ 2763.365996]        elf_map+0xb1/0xce
> [ 2763.365998]        load_elf_binary+0x91e/0x137a
> [ 2763.365999]        search_binary_handler+0x70/0x1f3
> [ 2763.366001]        do_execveat_common+0x45e/0x68e
> [ 2763.366003]        call_usermodehelper_exec_async+0xf7/0x11f
> [ 2763.366005]        ret_from_fork+0x27/0x40
> [ 2763.366006] 
>                -> #0 (&mapping->i_mmap_rwsem){++++}:
> [ 2763.366010]        __lock_acquire+0xa72/0xca0
> [ 2763.366012]        lock_acquire+0x176/0x19e
> [ 2763.366015]        down_read+0x3b/0x55
> [ 2763.366017]        rmap_walk_file+0x5a/0x142
> [ 2763.366018]        page_referenced+0xfc/0x134
> [ 2763.366022]        shrink_active_list+0x1ac/0x37d
> [ 2763.366024]        shrink_node_memcg.constprop.72+0x3ca/0x567
> [ 2763.366026]        shrink_node+0x3f/0x14c
> [ 2763.366028]        try_to_free_pages+0x288/0x47a
> [ 2763.366030]        __alloc_pages_slowpath+0x3a7/0xa49
> [ 2763.366032]        __alloc_pages_nodemask+0xf1/0x1f9
> [ 2763.366035]        khugepaged+0xc8/0x167c
> [ 2763.366037]        kthread+0x133/0x13b
> [ 2763.366039]        ret_from_fork+0x27/0x40
> [ 2763.366040] 
>                other info that might help us debug this:
> 
> [ 2763.366042] Chain exists of:
>                  &mapping->i_mmap_rwsem --> &vma->vm_sequence/1 --> fs_reclaim
> 
> [ 2763.366048]  Possible unsafe locking scenario:
> 
> [ 2763.366049]        CPU0                    CPU1
> [ 2763.366050]        ----                    ----
> [ 2763.366051]   lock(fs_reclaim);
> [ 2763.366054]                                lock(&vma->vm_sequence/1);
> [ 2763.366056]                                lock(fs_reclaim);
> [ 2763.366058]   lock(&mapping->i_mmap_rwsem);
> [ 2763.366061] 
>                 *** DEADLOCK ***
> 
> [ 2763.366063] 1 lock held by khugepaged/42:
> [ 2763.366064]  #0:  (fs_reclaim){+.+.}, at: [<ffffffff810e99dc>] fs_reclaim_acquire+0x12/0x35
> [ 2763.366068] 
>                stack backtrace:
> [ 2763.366071] CPU: 2 PID: 42 Comm: khugepaged Not tainted 4.13.0-next-20170913-dbg-00039-ge3c06ea4b028-dirty #1837
> [ 2763.366073] Call Trace:
> [ 2763.366077]  dump_stack+0x67/0x8e
> [ 2763.366080]  print_circular_bug+0x2a1/0x2af
> [ 2763.366083]  ? graph_unlock+0x69/0x69
> [ 2763.366085]  check_prev_add+0x76/0x20d
> [ 2763.366087]  ? graph_unlock+0x69/0x69
> [ 2763.366090]  __lock_acquire+0xa72/0xca0
> [ 2763.366093]  ? __save_stack_trace+0xa3/0xbf
> [ 2763.366096]  lock_acquire+0x176/0x19e
> [ 2763.366098]  ? rmap_walk_file+0x5a/0x142
> [ 2763.366100]  down_read+0x3b/0x55
> [ 2763.366102]  ? rmap_walk_file+0x5a/0x142
> [ 2763.366103]  rmap_walk_file+0x5a/0x142
> [ 2763.366106]  page_referenced+0xfc/0x134
> [ 2763.366108]  ? page_vma_mapped_walk_done.isra.17+0xb/0xb
> [ 2763.366109]  ? page_get_anon_vma+0x6d/0x6d
> [ 2763.366112]  shrink_active_list+0x1ac/0x37d
> [ 2763.366115]  shrink_node_memcg.constprop.72+0x3ca/0x567
> [ 2763.366118]  ? ___might_sleep+0xd5/0x234
> [ 2763.366121]  shrink_node+0x3f/0x14c
> [ 2763.366123]  try_to_free_pages+0x288/0x47a
> [ 2763.366126]  __alloc_pages_slowpath+0x3a7/0xa49
> [ 2763.366128]  ? ___might_sleep+0xd5/0x234
> [ 2763.366131]  __alloc_pages_nodemask+0xf1/0x1f9
> [ 2763.366133]  khugepaged+0xc8/0x167c
> [ 2763.366138]  ? remove_wait_queue+0x47/0x47
> [ 2763.366140]  ? collapse_shmem.isra.45+0x828/0x828
> [ 2763.366142]  kthread+0x133/0x13b
> [ 2763.366145]  ? __list_del_entry+0x1d/0x1d
> [ 2763.366147]  ret_from_fork+0x27/0x40
> 
> 	-ss
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 00/20] Speculative page faults
  2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
                   ` (19 preceding siblings ...)
  2017-09-08 18:07 ` [PATCH v3 20/20] powerpc/mm: Add speculative page fault Laurent Dufour
@ 2017-09-18  7:15 ` Laurent Dufour
  20 siblings, 0 replies; 33+ messages in thread
From: Laurent Dufour @ 2017-09-18  7:15 UTC (permalink / raw)
  To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
	Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
	hpa, Will Deacon, Sergey Senozhatsky
  Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
	Tim Chen, linuxppc-dev, x86

Despite the unprovable lockdep warning raised by Sergey, I didn't get any
feedback on this series.

Is there a chance to get it moved upstream ?

Thanks,
Laurent.

On 08/09/2017 20:06, Laurent Dufour wrote:
> This is a port on kernel 4.13 of the work done by Peter Zijlstra to
> handle page fault without holding the mm semaphore [1].
> 
> The idea is to try to handle user space page faults without holding the
> mmap_sem. This should allow better concurrency for massively threaded
> process since the page fault handler will not wait for other threads memory
> layout change to be done, assuming that this change is done in another part
> of the process's memory space. This type page fault is named speculative
> page fault. If the speculative page fault fails because of a concurrency is
> detected or because underlying PMD or PTE tables are not yet allocating, it
> is failing its processing and a classic page fault is then tried.
> 
> The speculative page fault (SPF) has to look for the VMA matching the fault
> address without holding the mmap_sem, so the VMA list is now managed using
> SRCU allowing lockless walking. The only impact would be the deferred file
> derefencing in the case of a file mapping, since the file pointer is
> released once the SRCU cleaning is done.  This patch relies on the change
> done recently by Paul McKenney in SRCU which now runs a callback per CPU
> instead of per SRCU structure [1].
> 
> The VMA's attributes checked during the speculative page fault processing
> have to be protected against parallel changes. This is done by using a per
> VMA sequence lock. This sequence lock allows the speculative page fault
> handler to fast check for parallel changes in progress and to abort the
> speculative page fault in that case.
> 
> Once the VMA is found, the speculative page fault handler would check for
> the VMA's attributes to verify that the page fault has to be handled
> correctly or not. Thus the VMA is protected through a sequence lock which
> allows fast detection of concurrent VMA changes. If such a change is
> detected, the speculative page fault is aborted and a *classic* page fault
> is tried.  VMA sequence locks are added when VMA attributes which are
> checked during the page fault are modified.
> 
> When the PTE is fetched, the VMA is checked to see if it has been changed,
> so once the page table is locked, the VMA is valid, so any other changes
> leading to touching this PTE will need to lock the page table, so no
> parallel change is possible at this time.
> 
> Compared to the Peter's initial work, this series introduces a spin_trylock
> when dealing with speculative page fault. This is required to avoid dead
> lock when handling a page fault while a TLB invalidate is requested by an
> other CPU holding the PTE. Another change due to a lock dependency issue
> with mapping->i_mmap_rwsem.
> 
> In addition some VMA field values which are used once the PTE is unlocked
> at the end the page fault path are saved into the vm_fault structure to
> used the values matching the VMA at the time the PTE was locked.
> 
> This series only support VMA with no vm_ops define, so huge page and mapped
> file are not managed with the speculative path. In addition transparent
> huge page are not supported. Once this series will be accepted upstream
> I'll extend the support to mapped files, and transparent huge pages.
> 
> This series builds on top of v4.13.9-mm1 and is functional on x86 and
> PowerPC.
> 
> Tests have been made using a large commercial in-memory database on a
> PowerPC system with 752 CPU using RFC v5 using a previous version of this
> series. The results are very encouraging since the loading of the 2TB
> database was faster by 14% with the speculative page fault.
> 
> Using ebizzy test [3], which spreads a lot of threads, the result are good
> when running on both a large or a small system. When using kernbench, the
> result are quite similar which expected as not so much multithreaded
> processes are involved. But there is no performance degradation neither
> which is good.
> 
> ------------------
> Benchmarks results
> 
> Note these test have been made on top of 4.13.0-mm1.
> 
> Ebizzy:
> -------
> The test is counting the number of records per second it can manage, the
> higher is the best. I run it like this 'ebizzy -mTRp'. To get consistent
> result I repeated the test 100 times and measure the average result, mean
> deviation, max and min.
> 
> - 16 CPUs x86 VM
> Records/s	4.13.0-mm1	4.13.0-mm1-spf	delta
> Average		13217.90 	65765.94	+397.55%
> Mean deviation	690.37		2609.36		+277.97%
> Max		16726		77675		+364.40%
> Min		12194		616340		+405.45%
> 		
> - 80 CPUs Power 8 node:
> Records/s	4.13.0-mm1	4.13.0-mm1-spf	delta
> Average		38175.40	67635.55	77.17% 
> Mean deviation	600.09	 	2349.66		291.55%
> Max		39563		74292		87.78% 
> Min		35846		62657		74.79% 
> 
> The number of record per second is far better with the speculative page
> fault. 
> The mean deviation is higher with the speculative page fault, may be
> because sometime the fault are not handled in a speculative way leading to
> more variation.
> The numbers for the x86 guest are really insane for the SPF case, but I
> did the test several times and this leads each time this delta. I did again
> the test using the previous version of the patch and I got similar
> numbers. It happens that the host running the VM is far less loaded now
> leading to better results as more threads are eligible to run.
> Test on Power are done on a badly balanced node where the memory is only
> attached to one core.
> 
> Kernbench:
> ----------
> This test is building a 4.12 kernel using platform default config. The
> build has been run 5 times each time.
> 
> - 16 CPUs x86 VM
> Average Half load -j 8 Run (std deviation)
>  		 4.13.0-mm1		4.13.0-mm1-spf		delta %
> Elapsed Time     145.968 (0.402206)	145.654 (0.533601)	-0.22
> User Time        1006.58 (2.74729)	1003.7 (4.11294)	-0.29
> System Time      108.464 (0.177567)	111.034 (0.718213)	+2.37
> Percent CPU 	 763.4 (1.34164)	764.8 (1.30384)		+0.18
> Context Switches 46599.6 (412.013)	63771 (1049.95)		+36.85
> Sleeps           85313.2 (514.456)	85532.2 (681.199)	-0.26
> 
> Average Optimal load -j 16 Run (std deviation)
>  		 4.13.0-mm1		4.13.0-mm1-spf		delta %
> Elapsed Time     74.292 (0.75998)	74.484 (0.723035)	+0.26
> User Time        959.949 (49.2036)	956.057 (50.2993)	-0.41
> System Time      100.203 (8.7119)	101.984 (9.56099)	+1.78
> Percent CPU 	 1058 (310.661)		1054.3 (305.263)	-0.35
> Context Switches 65713.8 (20161.7)	86619.4 (24095.4)	+31.81
> Sleeps           90344.9 (5364.74)	90877.4 (5655.87)	-0.59
> 
> The elapsed time are similar, but the impact less important since there are
> less multithreaded processes involved here. 
> 
> - 80 CPUs Power 8 node:
> Average Half load -j 40 Run (std deviation)
> 		 4.13.0-mm1		4.13.0-mm1-spf		delta %
> Elapsed Time 	 115.342 (0.321668)	115.786 (0.427118)	+0.38
> User Time 	 4355.08 (10.1778)	4371.77 (14.9715)	+0.38
> System Time 	 127.612 (0.882083)	130.048 (1.06258)	+1.91
> Percent CPU 	 3885.8 (11.606)	3887.4 (8.04984)	+0.04
> Context Switches 80907.8 (657.481)	81936.4 (729.538)	+1.27
> Sleeps		 162109 (793.331)	162057 (1414.08)	+0.03
> 
> Average Optimal load -j 80 Run (std deviation)
>  		 4.13.0-mm1		4.13.0-mm1-spf
> Elapsed Time 	 110.308 (0.725445)	109.78 (0.826862)	-0.48
> User Time 	 5893.12 (1621.33)	5923.19 (1635.48)	+0.51
> System Time 	 162.168 (36.4347)	166.533 (38.4695)	+2.69
> Percent CPU 	 5400.2 (1596.89)	5440.4 (1637.71)	+0.74
> Context Switches 129372 (51088.2)	144529 (65985.5)	+11.72
> Sleeps		 157312 (5113.57)	158696 (4301.48)	-0.87
> 
> Here the elapsed time are similar the SPF release, but we remain in the error
> margin. It has to be noted that this system is not correctly balanced on
> the NUMA point of view as all the available memory is attached to one core.
> 
> ------------------------
> Changes since v2:
>  - Perf event is renamed in PERF_COUNT_SW_SPF
>  - On Power handle do_page_fault()'s cleaning
>  - On Power if the VM_FAULT_ERROR is returned by
>  handle_speculative_fault(), do not retry but jump to the error path
>  - If VMA's flags are not matching the fault, directly returns
>  VM_FAULT_SIGSEGV and not VM_FAULT_RETRY
>  - Check for pud_trans_huge() to avoid speculative path
>  - Handles _vm_normal_page()'s introduced by 6f16211df3bf
>  ("mm/device-public-memory: device memory cache coherent with CPU")
>  - add and review few comments in the code
> Changes since v1:
>  - Remove PERF_COUNT_SW_SPF_FAILED perf event.
>  - Add tracing events to details speculative page fault failures.
>  - Cache VMA fields values which are used once the PTE is unlocked at the
>  end of the page fault events.
>  - Ensure that fields read during the speculative path are written and read
>  using WRITE_ONCE and READ_ONCE.
>  - Add checks at the beginning of the speculative path to abort it if the
>  VMA is known to not be supported.
> Changes since RFC V5 [5]
>  - Port to 4.13 kernel
>  - Merging patch fixing lock dependency into the original patch
>  - Replace the 2 parameters of vma_has_changed() with the vmf pointer
>  - In patch 7, don't call __do_fault() in the speculative path as it may
>  want to unlock the mmap_sem.
>  - In patch 11-12, don't check for vma boundaries when
>  page_add_new_anon_rmap() is called during the spf path and protect against
>  anon_vma pointer's update.
>  - In patch 13-16, add performance events to report number of successful
>  and failed speculative events. 
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__linux-2Dkernel.2935.n7.nabble.com_RFC-2DPATCH-2D0-2D6-2DAnother-2Dgo-2Dat-2Dspeculative-2Dpage-2Dfaults-2Dtt965642.html-23none&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZOg&r=WE1-GjEMX6XRg4v6rPpC0RVdhh4z63Csy-Wmu5dgUp0&m=449ThuJ31DP_64d96xAqLlSqq4qgY5LlJvzwiULSaos&s=9wDEbeKddqKRa0zfN13yjrErkFIQJo9Ohe07I7IuBSk&e= 
> [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_commit_-3Fid-3Dda915ad5cf25b5f5d358dd3670c3378d8ae8c03e&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZOg&r=WE1-GjEMX6XRg4v6rPpC0RVdhh4z63Csy-Wmu5dgUp0&m=449ThuJ31DP_64d96xAqLlSqq4qgY5LlJvzwiULSaos&s=OUT_ItjCInfCHdZQS5cjmxUQd3Ws8VkT54MZgJm2dAE&e= 
> [3] https://urldefense.proofpoint.com/v2/url?u=http-3A__ebizzy.sourceforge.net_&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZOg&r=WE1-GjEMX6XRg4v6rPpC0RVdhh4z63Csy-Wmu5dgUp0&m=449ThuJ31DP_64d96xAqLlSqq4qgY5LlJvzwiULSaos&s=cMZB09rj1TqCKM2B3DPrtrB1LpZan637kvHrM6ShaDk&e= 
> [4] https://urldefense.proofpoint.com/v2/url?u=http-3A__ck.kolivas.org_apps_kernbench_kernbench-2D0.50_&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZOg&r=WE1-GjEMX6XRg4v6rPpC0RVdhh4z63Csy-Wmu5dgUp0&m=449ThuJ31DP_64d96xAqLlSqq4qgY5LlJvzwiULSaos&s=2D_JH8n0pGF5lE0jSXnb2RY5etKV7C7UfO7-8hknJDE&e= 
> [5] https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_725607_&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZOg&r=WE1-GjEMX6XRg4v6rPpC0RVdhh4z63Csy-Wmu5dgUp0&m=449ThuJ31DP_64d96xAqLlSqq4qgY5LlJvzwiULSaos&s=CEgoZjaMNHIZFX-XAzuzr8EswsKhQAArNwmc_8bnduA&e= 
> 
> Laurent Dufour (14):
>   mm: Introduce pte_spinlock for FAULT_FLAG_SPECULATIVE
>   mm: Protect VMA modifications using VMA sequence count
>   mm: Cache some VMA fields in the vm_fault structure
>   mm: Protect SPF handler against anon_vma changes
>   mm/migrate: Pass vm_fault pointer to migrate_misplaced_page()
>   mm: Introduce __lru_cache_add_active_or_unevictable
>   mm: Introduce __maybe_mkwrite()
>   mm: Introduce __vm_normal_page()
>   mm: Introduce __page_add_new_anon_rmap()
>   mm: Try spin lock in speculative path
>   mm: Adding speculative page fault failure trace events
>   perf: Add a speculative page fault sw event
>   perf tools: Add support for the SPF perf event
>   powerpc/mm: Add speculative page fault
> 
> Peter Zijlstra (6):
>   mm: Dont assume page-table invariance during faults
>   mm: Prepare for FAULT_FLAG_SPECULATIVE
>   mm: VMA sequence count
>   mm: RCU free VMAs
>   mm: Provide speculative fault infrastructure
>   x86/mm: Add speculative pagefault handling
> 
>  arch/powerpc/include/asm/book3s/64/pgtable.h |   5 +
>  arch/powerpc/mm/fault.c                      |  15 +
>  arch/x86/include/asm/pgtable_types.h         |   7 +
>  arch/x86/mm/fault.c                          |  19 ++
>  fs/proc/task_mmu.c                           |   5 +-
>  fs/userfaultfd.c                             |  17 +-
>  include/linux/hugetlb_inline.h               |   2 +-
>  include/linux/migrate.h                      |   4 +-
>  include/linux/mm.h                           |  28 +-
>  include/linux/mm_types.h                     |   3 +
>  include/linux/pagemap.h                      |   4 +-
>  include/linux/rmap.h                         |  12 +-
>  include/linux/swap.h                         |  11 +-
>  include/trace/events/pagefault.h             |  87 +++++
>  include/uapi/linux/perf_event.h              |   1 +
>  kernel/fork.c                                |   1 +
>  mm/hugetlb.c                                 |   2 +
>  mm/init-mm.c                                 |   1 +
>  mm/internal.h                                |  19 ++
>  mm/khugepaged.c                              |   5 +
>  mm/madvise.c                                 |   6 +-
>  mm/memory.c                                  | 478 ++++++++++++++++++++++-----
>  mm/mempolicy.c                               |  51 ++-
>  mm/migrate.c                                 |   4 +-
>  mm/mlock.c                                   |  13 +-
>  mm/mmap.c                                    | 138 ++++++--
>  mm/mprotect.c                                |   4 +-
>  mm/mremap.c                                  |   7 +
>  mm/rmap.c                                    |   5 +-
>  mm/swap.c                                    |  12 +-
>  tools/include/uapi/linux/perf_event.h        |   1 +
>  tools/perf/util/evsel.c                      |   1 +
>  tools/perf/util/parse-events.c               |   4 +
>  tools/perf/util/parse-events.l               |   1 +
>  tools/perf/util/python.c                     |   1 +
>  35 files changed, 796 insertions(+), 178 deletions(-)
>  create mode 100644 include/trace/events/pagefault.h
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 04/20] mm: VMA sequence count
  2017-09-15 12:38                     ` Laurent Dufour
@ 2017-09-25 12:22                       ` Peter Zijlstra
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Zijlstra @ 2017-09-25 12:22 UTC (permalink / raw)
  To: Laurent Dufour
  Cc: Sergey Senozhatsky, paulmck, akpm, kirill, ak, mhocko, dave,
	jack, Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner,
	Ingo Molnar, hpa, Will Deacon, Sergey Senozhatsky, linux-kernel,
	linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
	linuxppc-dev, x86

On Fri, Sep 15, 2017 at 02:38:51PM +0200, Laurent Dufour wrote:
> >  /*
> >   * well... answering your question - it seems raw versions of seqcount
> >   * functions don't call lockdep's lock_acquire/lock_release...
> >   *
> >   * but I have never told you that. never.
> >   */
> 
> Hum... I'm not sure that would be the best way since in other case lockdep
> checks are valid, but if getting rid of locked's warning is required to get
> this series upstream, I'd use raw versions... Please advice...

No sensible other way to shut it up come to mind though. Might be best
to use the raw primitives with a comment explaining why.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2017-09-25 12:22 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-08 18:06 [PATCH v3 00/20] Speculative page faults Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 01/20] mm: Dont assume page-table invariance during faults Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 02/20] mm: Prepare for FAULT_FLAG_SPECULATIVE Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 03/20] mm: Introduce pte_spinlock " Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 04/20] mm: VMA sequence count Laurent Dufour
2017-09-13 11:53   ` Sergey Senozhatsky
2017-09-13 16:56     ` Laurent Dufour
2017-09-14  0:31       ` Sergey Senozhatsky
2017-09-14  7:55         ` Laurent Dufour
2017-09-14  8:13           ` Sergey Senozhatsky
2017-09-14  8:58             ` Laurent Dufour
2017-09-14  9:11               ` Sergey Senozhatsky
2017-09-14  9:15                 ` Laurent Dufour
2017-09-14  9:40                   ` Sergey Senozhatsky
2017-09-15 12:38                     ` Laurent Dufour
2017-09-25 12:22                       ` Peter Zijlstra
2017-09-08 18:06 ` [PATCH v3 05/20] mm: Protect VMA modifications using " Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 06/20] mm: RCU free VMAs Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 07/20] mm: Cache some VMA fields in the vm_fault structure Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 08/20] mm: Protect SPF handler against anon_vma changes Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 09/20] mm/migrate: Pass vm_fault pointer to migrate_misplaced_page() Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 10/20] mm: Introduce __lru_cache_add_active_or_unevictable Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 11/20] mm: Introduce __maybe_mkwrite() Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 12/20] mm: Introduce __vm_normal_page() Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 13/20] mm: Introduce __page_add_new_anon_rmap() Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 14/20] mm: Provide speculative fault infrastructure Laurent Dufour
2017-09-08 18:06 ` [PATCH v3 15/20] mm: Try spin lock in speculative path Laurent Dufour
2017-09-08 18:07 ` [PATCH v3 16/20] mm: Adding speculative page fault failure trace events Laurent Dufour
2017-09-08 18:07 ` [PATCH v3 17/20] perf: Add a speculative page fault sw event Laurent Dufour
2017-09-08 18:07 ` [PATCH v3 18/20] perf tools: Add support for the SPF perf event Laurent Dufour
2017-09-08 18:07 ` [PATCH v3 19/20] x86/mm: Add speculative pagefault handling Laurent Dufour
2017-09-08 18:07 ` [PATCH v3 20/20] powerpc/mm: Add speculative page fault Laurent Dufour
2017-09-18  7:15 ` [PATCH v3 00/20] Speculative page faults Laurent Dufour

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).