This is a port on kernel 4.15 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, this is done by introducing a rwlock
which protects the access to the mm_rb tree. Previously this was done using
SRCU but it was introducing a lot of scheduling to process the VMA's
freeing operation which was hitting the performance by 20% as reported by
Kemi Wang [2].Using a rwlock to protect access to the mm_rb tree is
limiting the locking contention to these operations which are expected to
be in a O(log n) order. In addition to ensure that the VMA is not freed in
our back a reference count is added and 2 services (get_vma() and
put_vma()) are introduced to handle the reference count. When a VMA is
fetch from the RB tree using get_vma() is must be later freeed using
put_vma(). Furthermore, to allow the VMA to be used again by the classic
page fault handler a service is introduced can_reuse_spf_vma(). This
service is expected to be called with the mmap_sem hold. It checked that
the VMA is still matching the specified address and is releasing its
reference count as the mmap_sem is hold it is ensure that it will not be
freed in our back. In general, the VMA's reference count could be
decremented when holding the mmap_sem but it should not be increased as
holding the mmap_sem is ensuring that the VMA is stable. I can't see
anymore the overhead I got while will-it-scale benchmark anymore.
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 lockings 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.
The locking of the PTE is done with interrupts disabled, this allows to
check for the PMD to ensure that there is not an ongoing collapsing
operation. Since khugepaged is firstly set the PMD to pmd_none and then is
waiting for the other CPU to have catch the IPI interrupt, if the pmd is
valid at the time the PTE is locked, we have the guarantee that the
collapsing opertion will have to wait on the PTE lock to move foward. This
allows the SPF handler to map the PTE safely. If the PMD value is different
than the one recorded at the beginning of the SPF operation, the classic
page fault handler will be called to handle the operation while holding the
mmap_sem. As the PTE lock is done with the interrupts disabled, the lock is
done using spin_trylock() to avoid dead lock when handling a page fault
while a TLB invalidate is requested by an other CPU holding the PTE.
Support for THP is not done because when checking for the PMD, we can be
confused by an in progress collapsing operation done by khugepaged. The
issue is that pmd_none() could be true either if the PMD is not already
populate or if the underlying PTE are in the way to be collapsed. So we
cannot safely allocate a PMD if pmd_none() is true.
This series builds on top of v4.14-rc5 and is functional on x86 and
PowerPC.
------------------
Benchmarks results
Base kernel is 4.15-rc6-mmotm-2018-01-04-16-19
SPF is BASE + this series
Kernbench:
----------
Here are the results on a 16 CPUs X86 guest using kernbench on a 4.13-rc4
kernel (kernel is build 5 times):
Average Optimal load -j 8
Base SPF
Run (std deviation)
Elapsed Time 148.04 (0.62446) 150.31 (0.940585) 1.53%
User Time 1017.27 (1.23567) 1029.14 (4.43995) 1.17%
System Time 112.53 (0.888285) 118.62 (0.712215) 5.41%
Percent CPU 762.6 (2.30217) 763 (2.64575) 0.05%
Context Switches 46394.2 (792.6) 46880.2 (812.688) 1.05%
Sleeps 85207.4 (659.075) 85525.2 (663.924) 0.37%
Average Maximal load -j 16
Base SPF
Run (std deviation)
Elapsed Time 71.102 (0.424347) 72.438 (0.650054) 1.88%
User Time 945.085 (76.0995) 957.76 (75.2979) 1.34%
System Time 98.828 (14.4599) 104.596 (14.7918) 5.84%
Percent CPU 1054.8 (308.054) 1055.7 (308.63) 0.09%
Context Switches 65134.6 (19763.6) 65487 (19626.2) 0.54%
Sleeps 90760.7 (5896.06) 90821.1 (5611.64) 0.07%
The elapsed time is in the same order, a bit larger in the case of the spf
release, but that seems to be in the error margin. Context switches are now
in the same order.
Here are the kerbench results on a 80 CPUs Power8 system :
Average Maximal load -j 40
Base SPF
Run (std deviation)
Elapsed Time 108.37 (0.544885) 109.2 (0.454037) 0.77%
User Time 4111.38 (13.7343) 4129.09 (13.6466) 0.43%
System Time 99.098 (0.364925) 99.856 (0.386109) 0.76%
Percent CPU 3884.6 (9.31665) 3871.8 (12.0706) -0.33%
Context Switches 71962.2 (206.129) 72009.8 (185.303) 0.07%
Sleeps 156369 (850.193) 155912 (1099.63) -0.29%
Average Maximal load -j 80
Base SPF
Run (std deviation)
Elapsed Time 98.886 (0.629031) 99.726 (0.871854) 0.85%
User Time 5408.5 (1367.35) 5416.23 (1356.89) 0.14%
System Time 115.364 (17.161) 115.923 (16.939) 0.48%
Percent CPU 5399.2 (1596.95) 5362.9 (1572.71) -0.67%
Context Switches 138892 (70610.1) 138321 (69920.2) -0.41%
Sleeps 159509 (6171.69) 158354 (3595.75) -0.72%
We can't see any impact, neither performance improvement, but this is
mostly mono threaded processes and the SPF handler is not activated.
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. The
number is the record processes per second, the higher is the best.
- 16 CPUs x86 VM
BASE SPF delta
16 CPUs x86 VM 11485.14 60902.26 430.27%
80 CPUs P8 node 37648.73 83078.01 120.67%
Here the delta is big, as this test is triggering the SPF handler
massively.
Will-It-Scale [6]
-----------------
I got a lot of deviation in the results returned by this benchmark,
especially when running a high number of CPUs. However while the series v5
was showing a performance degradation, despite the deviations, I can't see
it anymore with this series. This being said, it's hard to produce numbers
of such a benchmark as deviation is really too high.
------------------
Changes since V5:
- use rwlock agains the mm RB tree in place of SRCU
- add a VMA's reference count to protect VMA while using it without
holding the mmap_sem.
- check PMD value to detect collapsing operation
- don't try speculative page fault for mono threaded processes
- try to reuse the fetched VMA if VM_RETRY is returned
- go directly to the error path if an error is detected during the SPF
path
- fix race window when moving VMA in move_vma()
Changes since v4:
- As requested by Andrew Morton, use CONFIG_SPF and define it earlier in
the series to ease bisection.
Changes since v3:
- Don't build when CONFIG_SMP is not set
- Fixed a lock dependency warning in __vma_adjust()
- Use READ_ONCE to access p*d values in handle_speculative_fault()
- Call memcp_oom() service in handle_speculative_fault()
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://patchwork.kernel.org/patch/9999687/
[3] http://ebizzy.sourceforge.net/
[4] http://ck.kolivas.org/apps/kernbench/kernbench-0.50/
[5] https://lwn.net/Articles/725607/
[6] https://github.com/antonblanchard/will-it-scale.git
Laurent Dufour (19):
x86/mm: Define CONFIG_SPF
powerpc/mm: Define CONFIG_SPF
mm: Introduce pte_spinlock for FAULT_FLAG_SPECULATIVE
mm: Protect VMA modifications using VMA sequence count
mm: protect mremap() against SPF hanlder
mm: Protect SPF handler against anon_vma changes
mm: Cache some VMA fields in the vm_fault structure
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: Protect mm_rb tree with a rwlock
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
mm: Speculative page fault handler return VMA
powerpc/mm: Add speculative page fault
Peter Zijlstra (5):
mm: Dont assume page-table invariance during faults
mm: Prepare for FAULT_FLAG_SPECULATIVE
mm: VMA sequence count
mm: Provide speculative fault infrastructure
x86/mm: Add speculative pagefault handling
arch/powerpc/Kconfig | 4 +
arch/powerpc/mm/fault.c | 31 +-
arch/x86/Kconfig | 4 +
arch/x86/mm/fault.c | 38 ++-
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 | 91 +++++-
include/linux/mm_types.h | 7 +
include/linux/pagemap.h | 4 +-
include/linux/rmap.h | 12 +-
include/linux/swap.h | 10 +-
include/trace/events/pagefault.h | 87 ++++++
include/uapi/linux/perf_event.h | 1 +
kernel/fork.c | 3 +
mm/hugetlb.c | 2 +
mm/init-mm.c | 3 +
mm/internal.h | 20 ++
mm/khugepaged.c | 5 +
mm/madvise.c | 6 +-
mm/memory.c | 563 ++++++++++++++++++++++++++++++----
mm/mempolicy.c | 51 ++-
mm/migrate.c | 4 +-
mm/mlock.c | 13 +-
mm/mmap.c | 209 ++++++++++---
mm/mprotect.c | 4 +-
mm/mremap.c | 13 +
mm/rmap.c | 5 +-
mm/swap.c | 6 +-
mm/swap_state.c | 8 +-
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 +
36 files changed, 1076 insertions(+), 164 deletions(-)
create mode 100644 include/trace/events/pagefault.h
--
2.7.4
Introduce CONFIG_SPF which turns on the Speculative Page Fault handler when
building for 64bits with SMP.
Signed-off-by: Laurent Dufour <redacted>
---
arch/x86/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
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>
[Remove only if !CONFIG_SPF]
Signed-off-by: Laurent Dufour <redacted>
---
mm/memory.c | 4 ++++
1 file changed, 4 insertions(+)
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 <redacted>
---
include/linux/mm.h | 1 +
mm/memory.c | 56 ++++++++++++++++++++++++++++++++++++++----------------
2 files changed, 41 insertions(+), 16 deletions(-)
@@ -302,6 +302,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"},\
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 <redacted>
---
mm/memory.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
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()
- 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 <redacted>
---
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/swap_state.c | 8 ++++++--
9 files changed, 86 insertions(+), 38 deletions(-)
@@ -550,6 +550,10 @@ static unsigned long swapin_nr_pages(unsigned long offset)*thereadahead.**Callermustholddown_readonthevma->vm_mmifvmaisnotNULL.+*ThisisneededtoensuretheVMAwillnotbefreedinourback.Inthecase+*ofthespeculativepagefaulthandler,thiscannothappen,evenifwedon't+*holdthemmap_sem.CalleesareassumedtotakecareofreadingVMA'sfields+*usingREAD_ONCE()toreadconsistentvalues.*/structpage*swapin_readahead(swp_entry_tentry,gfp_tgfp_mask,structvm_area_struct*vma,unsignedlongaddr)
If a thread is remapping an area while another one is faulting on the
destination area, the SPF handler may fetch the vma from the RB tree before
the pte has been moved by the other thread. This means that the moved ptes
will overwrite those create by the page fault handler leading to page
leaked.
CPU 1 CPU2
enter mremap()
unmap the dest area
copy_vma() Enter speculative page fault handler
>> at this time the dest area is present in the RB tree
fetch the vma matching dest area
create a pte as the VMA matched
Exit the SPF handler
<data written in the new page>
move_ptes()
> it is assumed that the dest area is empty,
> the move ptes overwrite the page mapped by the CPU2.
To prevent that, when the VMA matching the dest area is extended or created
by copy_vma(), it should be marked as non available to the SPF handler.
The usual way to so is to rely on vm_write_begin()/end().
This is already in __vma_adjust() called by copy_vma() (through
vma_merge()). But __vma_adjust() is calling vm_write_end() before returning
which create a window for another thread.
This patch adds a new parameter to vma_merge() which is passed down to
vma_adjust().
The assumption is that copy_vma() is returning a vma which should be
released by calling vm_raw_write_end() by the callee once the ptes have
been moved.
Signed-off-by: Laurent Dufour <redacted>
---
include/linux/mm.h | 16 ++++++++++++----
mm/mmap.c | 47 ++++++++++++++++++++++++++++++++++++-----------
mm/mremap.c | 13 +++++++++++++
3 files changed, 61 insertions(+), 15 deletions(-)
@@ -3163,9 +3168,20 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,if(find_vma_links(mm,addr,addr+len,&prev,&rb_link,&rb_parent))returnNULL;/* should never get here */-new_vma=vma_merge(mm,prev,addr,addr+len,vma->vm_flags,-vma->anon_vma,vma->vm_file,pgoff,vma_policy(vma),-vma->vm_userfaultfd_ctx);++/* There is 3 cases to manage here in+*AAAAAAAAAAAAAAAA+*PPPP....PPPP......NNNNPPPP....NNNNPP........NN+*PPPPPPPP(A)PPPP..NNNNNNNN(B)PPPPPPPPPPPP(1)NULL+*PPPPPPPPNNNN(2)+*PPPPNNNNNNNN(3)+*+*new_vma==previncaseA,1,2+*new_vma==nextincaseB,3+*/+new_vma=__vma_merge(mm,prev,addr,addr+len,vma->vm_flags,+vma->anon_vma,vma->vm_file,pgoff,+vma_policy(vma),vma->vm_userfaultfd_ctx,true);if(new_vma){/**Sourcevmamayhavebeenmergedintonew_vma
@@ -302,6 +302,14 @@ static unsigned long move_vma(struct vm_area_struct *vma,if(!new_vma)return-ENOMEM;+/* new_vma is returned protected by copy_vma, to prevent speculative+*pagefaulttobedoneinthedestinationareabeforewemovethepte.+*Now,wemustalsoprotectthesourceVMAsincewedon'twantpages+*tobemappedinourbackwhilewearecopyingthePTEs.+*/+if(vma!=new_vma)+vm_raw_write_begin(vma);+moved_len=move_page_tables(vma,old_addr,new_vma,new_addr,old_len,need_rmap_locks);if(moved_len<old_len){
@@ -318,6 +326,8 @@ static unsigned long move_vma(struct vm_area_struct *vma,*/move_page_tables(new_vma,new_addr,vma,old_addr,moved_len,true);+if(vma!=new_vma)+vm_raw_write_end(vma);vma=new_vma;old_len=new_len;old_addr=new_addr;
@@ -326,7 +336,10 @@ 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);+if(vma!=new_vma)+vm_raw_write_end(vma);}+vm_raw_write_end(new_vma);/* Conceal VM_ACCOUNT so old reservation is not undone */if(vm_flags&VM_ACCOUNT){
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 <redacted>
---
mm/memory.c | 4 ++++
1 file changed, 4 insertions(+)
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 <redacted>
---
include/linux/swap.h | 10 ++++++++--
mm/memory.c | 8 ++++----
mm/swap.c | 6 +++---
3 files changed, 15 insertions(+), 9 deletions(-)
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 <redacted>
---
include/linux/rmap.h | 12 ++++++++++--
mm/memory.c | 8 ++++----
mm/rmap.c | 5 ++---
3 files changed, 16 insertions(+), 9 deletions(-)
@@ -2539,7 +2539,7 @@ static int wp_page_copy(struct vm_fault *vmf)*threaddoingCOW.*/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);/*
@@ -3082,7 +3082,7 @@ int do_swap_page(struct vm_fault *vmf)/* ksm created a completely new copy */if(unlikely(page!=swapcache&&swapcache)){-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{
@@ -3232,7 +3232,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:
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 <redacted>
---
include/linux/mm.h | 7 +++++--
mm/memory.c | 18 ++++++++++--------
2 files changed, 15 insertions(+), 10 deletions(-)
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 <redacted>
---
mm/memory.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
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]
[Don't build SPF call if !CONFIG_SPF]
[Try speculative fault path only for multi threaded processes]
[Try to the VMA fetch during the speculative path in case of retry]
Signed-off-by: Laurent Dufour <redacted>
---
arch/x86/mm/fault.c | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
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.
The speculative path is only tried for multithreaded process as there is no
risk of contention on the mmap_sem otherwise.
Build on if CONFIG_SPF is defined (currently for BOOK3S_64 && SMP).
Signed-off-by: Laurent Dufour <redacted>
---
arch/powerpc/mm/fault.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
@@ -384,6 +384,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,unsignedlongerror_code){structvm_area_struct*vma;+#ifdef CONFIG_SPF+structvm_area_struct*spf_vma=NULL;+#endifstructmm_struct*mm=current->mm;unsignedintflags=FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_KILLABLE;intis_exec=TRAP(regs)==0x400;
@@ -447,6 +450,20 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,if(is_exec)flags|=FAULT_FLAG_INSTRUCTION;+#ifdef CONFIG_SPF+if(is_user&&(atomic_read(&mm->mm_users)>1)){+/* let's try a speculative page fault without grabbing the+*mmap_sem.+*/+fault=handle_speculative_fault(mm,address,flags,&spf_vma);+if(!(fault&VM_FAULT_RETRY)){+perf_sw_event(PERF_COUNT_SW_SPF,1,+regs,address);+gotodone;+}+}+#endif /* CONFIG_SPF */+/* When running in the kernel we expect faults to occur only to*addressesinuserspace.Allotherfaultsrepresenterrorsinthe*kernelandshouldgenerateanOOPS.Unfortunately,inthecaseofan
@@ -477,7 +494,16 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,might_sleep();}-vma=find_vma(mm,address);+#ifdef CONFIG_SPF+if(spf_vma){+if(can_reuse_spf_vma(spf_vma,address))+vma=spf_vma;+else+vma=find_vma(mm,address);+spf_vma=NULL;+}else+#endif+vma=find_vma(mm,address);if(unlikely(!vma))returnbad_area(regs,address);if(likely(vma->vm_start<=address))
@@ -531,6 +557,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,up_read(¤t->mm->mmap_sem);+#ifdef CONFIG_SPF+done:+#endifif(unlikely(fault&VM_FAULT_ERROR))returnmm_fault_error(regs,address,fault);
When the speculative page fault handler is returning VM_RETRY, there is a
chance that VMA fetched without grabbing the mmap_sem can be reused by the
legacy page fault handler. By reusing it, we avoid calling find_vma()
again. To achieve, that we must ensure that the VMA structure will not be
freed in our back. This is done by getting the reference on it (get_vma())
and by assuming that the caller will call the new service
can_reuse_spf_vma() once it has grabbed the mmap_sem.
can_reuse_spf_vma() is first checking that the VMA is still in the RB tree
, and then that the VMA's boundaries matched the passed address and release
the reference on the VMA so that it can be freed if needed.
In the case the VMA is freed, can_reuse_spf_vma() will have returned false
as the VMA is no more in the RB tree.
Signed-off-by: Laurent Dufour <redacted>
---
include/linux/mm.h | 5 +-
mm/memory.c | 136 +++++++++++++++++++++++++++++++++--------------------
2 files changed, 88 insertions(+), 53 deletions(-)
@@ -4284,13 +4284,22 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,/* This is required by vm_normal_page() */#error "Speculative page fault handler requires __HAVE_ARCH_PTE_SPECIAL"#endif-/**vm_normal_page()addssomeprocessingwhichshouldbedonewhile*hodlingthemmap_sem.*/++/*+*Triestohandlethepagefaultinaspeculativeway,withoutgrabbingthe+*mmap_sem.+*WhenVM_FAULT_RETRYisreturned,thevmapointerisvalidandthisvmamust+*becheckedlaterwhenthemmap_semhasbeengrabbedbycalling+*can_reuse_spf_vma().+*Thisisneededasthereturnedvmaiskeptinmemoryuntilthecallto+*can_reuse_spf_vma()ismade.+*/inthandle_speculative_fault(structmm_struct*mm,unsignedlongaddress,-unsignedintflags)+unsignedintflags,structvm_area_struct**vma){structvm_faultvmf={.address=address,
@@ -4299,7 +4308,6 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,p4d_t*p4d,p4dval;pud_tpudval;intseq,ret=VM_FAULT_RETRY;-structvm_area_struct*vma;#ifdef CONFIG_NUMAstructmempolicy*pol;#endif
@@ -4308,14 +4316,16 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,flags&=~(FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_KILLABLE);flags|=FAULT_FLAG_SPECULATIVE;-vma=get_vma(mm,address);-if(!vma)+*vma=get_vma(mm,address);+if(!*vma)returnret;+vmf.vma=*vma;-seq=raw_read_seqcount(&vma->vm_sequence);/* rmb <-> seqlock,vma_rb_erase() */+/* rmb <-> seqlock,vma_rb_erase() */+seq=raw_read_seqcount(&vmf.vma->vm_sequence);if(seq&1){-trace_spf_vma_changed(_RET_IP_,vma,address);-gotoout_put;+trace_spf_vma_changed(_RET_IP_,vmf.vma,address);+returnret;}/*
@@ -4323,9 +4333,9 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,*withtheVMA.*Thisincludehugepagefromhugetlbfs.*/-if(vma->vm_ops){-trace_spf_vma_notsup(_RET_IP_,vma,address);-gotoout_put;+if(vmf.vma->vm_ops){+trace_spf_vma_notsup(_RET_IP_,vmf.vma,address);+returnret;}/*
@@ -4333,18 +4343,18 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,*becausevm_nextandvm_prevmustbesafe.Thiscan'tbeguaranteed*inthespeculativepath.*/-if(unlikely(!vma->anon_vma)){-trace_spf_vma_notsup(_RET_IP_,vma,address);-gotoout_put;+if(unlikely(!vmf.vma->anon_vma)){+trace_spf_vma_notsup(_RET_IP_,vmf.vma,address);+returnret;}-vmf.vma_flags=READ_ONCE(vma->vm_flags);-vmf.vma_page_prot=READ_ONCE(vma->vm_page_prot);+vmf.vma_flags=READ_ONCE(vmf.vma->vm_flags);+vmf.vma_page_prot=READ_ONCE(vmf.vma->vm_page_prot);/* Can't call userland page fault handler in the speculative path */if(unlikely(vmf.vma_flags&VM_UFFD_MISSING)){-trace_spf_vma_notsup(_RET_IP_,vma,address);-gotoout_put;+trace_spf_vma_notsup(_RET_IP_,vmf.vma,address);+returnret;}if(vmf.vma_flags&VM_GROWSDOWN||vmf.vma_flags&VM_GROWSUP){
@@ -4353,48 +4363,39 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,*boundariesbutwewanttotraceitasnotsupportedinstead*ofchanged.*/-trace_spf_vma_notsup(_RET_IP_,vma,address);-gotoout_put;+trace_spf_vma_notsup(_RET_IP_,vmf.vma,address);+returnret;}-if(address<READ_ONCE(vma->vm_start)-||READ_ONCE(vma->vm_end)<=address){-trace_spf_vma_changed(_RET_IP_,vma,address);-gotoout_put;+if(address<READ_ONCE(vmf.vma->vm_start)+||READ_ONCE(vmf.vma->vm_end)<=address){+trace_spf_vma_changed(_RET_IP_,vmf.vma,address);+returnret;}-if(!arch_vma_access_permitted(vma,flags&FAULT_FLAG_WRITE,+if(!arch_vma_access_permitted(vmf.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;-gotoout_put;-}+flags&FAULT_FLAG_REMOTE))+gotoout_segv;/* 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;-gotoout_put;-}-}elseif(unlikely(!(vmf.vma_flags&(VM_READ|VM_EXEC|VM_WRITE)))){-trace_spf_vma_access(_RET_IP_,vma,address);-ret=VM_FAULT_SIGSEGV;-gotoout_put;-}+if(unlikely(!(vmf.vma_flags&VM_WRITE)))+gotoout_segv;+}elseif(unlikely(!(vmf.vma_flags&(VM_READ|VM_EXEC|VM_WRITE))))+gotoout_segv;#ifdef CONFIG_NUMA/**MPOL_INTERLEAVEimpliesadditionalcheckinmpol_misplaced()which*arenotcompatiblewiththespeculativepagefaultprocessing.*/-pol=__get_vma_policy(vma,address);+pol=__get_vma_policy(vmf.vma,address);if(!pol)pol=get_task_policy(current);if(pol&&pol->mode==MPOL_INTERLEAVE){-trace_spf_vma_notsup(_RET_IP_,vma,address);-gotoout_put;+trace_spf_vma_notsup(_RET_IP_,vmf.vma,address);+returnret;}#endif
@@ -4456,9 +4457,8 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,vmf.pte=NULL;}-vmf.vma=vma;-vmf.pgoff=linear_page_index(vma,address);-vmf.gfp_mask=__get_fault_gfp_mask(vma);+vmf.pgoff=linear_page_index(vmf.vma,address);+vmf.gfp_mask=__get_fault_gfp_mask(vmf.vma);vmf.sequence=seq;vmf.flags=flags;
@@ -4468,16 +4468,22 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,*Weneedtore-validatetheVMAaftercheckingthebounds,otherwise*wemighthaveafalsepositiveonthebounds.*/-if(read_seqcount_retry(&vma->vm_sequence,seq)){-trace_spf_vma_changed(_RET_IP_,vma,address);-gotoout_put;+if(read_seqcount_retry(&vmf.vma->vm_sequence,seq)){+trace_spf_vma_changed(_RET_IP_,vmf.vma,address);+returnret;}mem_cgroup_oom_enable();ret=handle_pte_fault(&vmf);mem_cgroup_oom_disable();-put_vma(vma);+/*+*Ifthereisnoneedtoretry,don'treturnthevmatothecaller.+*/+if(!(ret&VM_FAULT_RETRY)){+put_vma(vmf.vma);+*vma=NULL;+}/**ThetaskmayhaveenteredamemcgOOMsituationbut
@@ -4490,9 +4496,35 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,returnret;out_walk:-trace_spf_vma_notsup(_RET_IP_,vma,address);+trace_spf_vma_notsup(_RET_IP_,vmf.vma,address);local_irq_enable();-out_put:+returnret;++out_segv:+trace_spf_vma_access(_RET_IP_,vmf.vma,address);+/*+*Wedon'treturnVM_FAULT_RETRYsothecallerisnotexpectedto+*retrievethefetchedVMA.+*/+put_vma(vmf.vma);+*vma=NULL;+returnVM_FAULT_SIGSEGV;+}++/*+*Thisisusedtoknowifthevmafetchinthespeculativepagefaulthandler+*isstillvalidwhentryingtheregularfaultpathwhileholdingthe+*mmap_sem.+*Thecalltoput_vma(vma)mustbemadeaftercheckingthevma'sfields,as+*thevmamaybefreedbyput_vma().Insuchacaseitisexpectedthatfalse+*isreturned.+*/+boolcan_reuse_spf_vma(structvm_area_struct*vma,unsignedlongaddress)+{+boolret;++ret=!RB_EMPTY_NODE(&vma->vm_rb)&&+vma->vm_start<=address&&address<vma->vm_end;put_vma(vma);returnret;}
@@ -80,6 +80,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
@@ -4297,47 +4313,60 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,returnret;seq=raw_read_seqcount(&vma->vm_sequence);/* rmb <-> seqlock,vma_rb_erase() */-if(seq&1)+if(seq&1){+trace_spf_vma_changed(_RET_IP_,vma,address);gotoout_put;+}/**Can'tcallvm_opsservicehaswedon'tknowwhattheywoulddo*withtheVMA.*Thisincludehugepagefromhugetlbfs.*/-if(vma->vm_ops)+if(vma->vm_ops){+trace_spf_vma_notsup(_RET_IP_,vma,address);gotoout_put;+}/**__anon_vma_prepare()requiresthemmap_semtobeheld*becausevm_nextandvm_prevmustbesafe.Thiscan'tbeguaranteed*inthespeculativepath.*/-if(unlikely(!vma->anon_vma))+if(unlikely(!vma->anon_vma)){+trace_spf_vma_notsup(_RET_IP_,vma,address);gotoout_put;+}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);gotoout_put;+}-if(vmf.vma_flags&VM_GROWSDOWN||vmf.vma_flags&VM_GROWSUP)+if(vmf.vma_flags&VM_GROWSDOWN||vmf.vma_flags&VM_GROWSUP){/**ThiscouldbedetectedbythecheckaddressagainstVMA's*boundariesbutwewanttotraceitasnotsupportedinstead*ofchanged.*/+trace_spf_vma_notsup(_RET_IP_,vma,address);gotoout_put;+}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);gotoout_put;+}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;gotoout_put;}
@@ -4345,10 +4374,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;gotoout_put;}}elseif(unlikely(!(vmf.vma_flags&(VM_READ|VM_EXEC|VM_WRITE)))){+trace_spf_vma_access(_RET_IP_,vma,address);ret=VM_FAULT_SIGSEGV;gotoout_put;}
@@ -4361,8 +4392,10 @@ 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);gotoout_put;+}#endif/*
@@ -4435,8 +4468,10 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,*Weneedtore-validatetheVMAaftercheckingthebounds,otherwise*wemighthaveafalsepositiveonthebounds.*/-if(read_seqcount_retry(&vma->vm_sequence,seq))+if(read_seqcount_retry(&vma->vm_sequence,seq)){+trace_spf_vma_changed(_RET_IP_,vma,address);gotoout_put;+}mem_cgroup_oom_enable();ret=handle_pte_fault(&vmf);
@@ -4455,6 +4490,7 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,returnret;out_walk:+trace_spf_vma_notsup(_RET_IP_,vma,address);local_irq_enable();out_put:put_vma(vma);
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]
[Fetch p4d and pud]
[Set vmd.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()]
[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()]
[Don't build SPF services if !CONFIG_SPF]
[Add mem cgroup oom check]
[Use READ_ONCE to access p*d entries]
[Replace deprecated ACCESS_ONCE() by READ_ONCE() in vma_has_changed()]
[Don't fetch pte again in handle_pte_fault() when running the speculative
path]
[Check PMD against concurrent collapsing operation]
Signed-off-by: Laurent Dufour <redacted>
---
include/linux/hugetlb_inline.h | 2 +-
include/linux/mm.h | 8 +
include/linux/pagemap.h | 4 +-
mm/internal.h | 16 +-
mm/memory.c | 321 ++++++++++++++++++++++++++++++++++++++++-
5 files changed, 344 insertions(+), 7 deletions(-)
@@ -331,6 +331,10 @@ struct vm_fault {gfp_tgfp_mask;/* gfp mask to be used for allocations */pgoff_tpgoff;/* Logical page offset based on vma */unsignedlongaddress;/* Faulting virtual address */+#ifdef CONFIG_SPF+unsignedintsequence;+pmd_torig_pmd;/* value of PMD at the time of fault */+#endifpmd_t*pmd;/* Pointer to pmd entry matching*the'address'*/pud_t*pud;/* Pointer to pud entry matching
@@ -2445,19 +2446,108 @@ static inline void wp_page_reuse(struct vm_fault *vmf)pte_unmap_unlock(vmf->pte,vmf->ptl);}+#ifdef CONFIG_SPFstaticboolpte_spinlock(structvm_fault*vmf){+boolret=false;+pmd_tpmdval;++/* 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);+returntrue;+}++local_irq_disable();+if(vma_has_changed(vmf))+gotoout;++/*+*Wecheckifthepmdvalueisstillthesametoensurethatthere+*isahugecollapseoperationinprogressinourback.+*/+pmdval=READ_ONCE(*vmf->pmd);+if(!pmd_same(pmdval,vmf->orig_pmd))+gotoout;++vmf->ptl=pte_lockptr(vmf->vma->vm_mm,vmf->pmd);+spin_lock(vmf->ptl);++if(vma_has_changed(vmf)){+spin_unlock(vmf->ptl);+gotoout;+}++ret=true;+out:+local_irq_enable();+returnret;+}+#else+staticinlineboolpte_spinlock(structvm_fault*vmf)+{vmf->ptl=pte_lockptr(vmf->vma->vm_mm,vmf->pmd);spin_lock(vmf->ptl);returntrue;}+#endif /* CONFIG_SPF */+#ifdef CONFIG_SPFstaticboolpte_map_lock(structvm_fault*vmf){+boolret=false;+pte_t*pte;+pmd_tpmdval;+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);+returntrue;+}++/*+*Thefirstvma_has_changed()guaranteesthepage-tablesarestill+*valid,havingIRQsdisabledensurestheystayaround,hencethe+*secondvma_has_changed()tomakesuretheyarestillvalidonce+*we'vegotthelock.Afterthataconcurrentzap_pte_range()will+*blockonthePTLandthuswe'resafe.+*/+local_irq_disable();+if(vma_has_changed(vmf))+gotoout;++/*+*Wecheckifthepmdvalueisstillthesametoensurethatthere+*isahugecollapseoperationinprogressinourback.+*/+pmdval=READ_ONCE(*vmf->pmd);+if(!pmd_same(pmdval,vmf->orig_pmd))+gotoout;++pte=pte_offset_map_lock(vmf->vma->vm_mm,vmf->pmd,+vmf->address,&ptl);+if(vma_has_changed(vmf)){+pte_unmap_unlock(pte,ptl);+gotoout;+}++vmf->pte=pte;+vmf->ptl=ptl;+ret=true;+out:+local_irq_enable();+returnret;+}+#else+staticinlineboolpte_map_lock(structvm_fault*vmf)+{vmf->pte=pte_offset_map_lock(vmf->vma->vm_mm,vmf->pmd,vmf->address,&vmf->ptl);returntrue;}+#endif /* CONFIG_SPF *//**Handlethecaseofapagewhichweactuallyneedtocopytoanewpage.
@@ -3182,6 +3272,14 @@ static int do_anonymous_page(struct vm_fault *vmf)ret=check_stable_address_space(vma->vm_mm);if(ret)gotounlock;+/*+*Don'tcalltheuserfaultfdduringthespeculativepath.+*WealreadycheckedfortheVMAtonotbemanagedthrough+*userfaultfd,butitmaybesetinourbackoncewehavelock+*thepte.Insuchacasewecanignoreitthistime.+*/+if(vmf->flags&FAULT_FLAG_SPECULATIVE)+gotosetpte;/* Deliver the page fault to userland, check inside PT lock */if(userfaultfd_missing(vma)){pte_unmap_unlock(vmf->pte,vmf->ptl);
@@ -3224,7 +3322,7 @@ static int do_anonymous_page(struct vm_fault *vmf)gotorelease;/* 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);
@@ -3967,13 +4065,22 @@ static int handle_pte_fault(struct vm_fault *vmf)if(unlikely(pmd_none(*vmf->pmd))){/*+*Inthecaseofthespeculativepagefaulthandlerweabort+*thespeculativepathimmediatelyasthepmdisprobably+*inthewaytobeconvertedinahugeone.Wewilltry+*againholdingthemmap_sem(whichimpliesthatthecollapse+*operationisdone).+*/+if(vmf->flags&FAULT_FLAG_SPECULATIVE)+returnVM_FAULT_RETRY;+/**Leave__pte_alloc()untillater:becausevm_ops->faultmay*wanttoallocatehugepage,andifweexposepagetable*foraninstant,itwillbedifficulttoretractfrom*concurrentfaultsandfromrmaplookups.*/vmf->pte=NULL;-}else{+}elseif(!(vmf->flags&FAULT_FLAG_SPECULATIVE)){/* See comment in pte_alloc_one_map() */if(pmd_devmap_trans_unstable(vmf->pmd))return0;
@@ -3982,6 +4089,9 @@ static int handle_pte_fault(struct vm_fault *vmf)*pmdfromunderusanymoreatthispointbecauseweholdthe*mmap_semreadmodeandkhugepagedtakesitinwritemode.*Sonowit'ssafetorunpte_offset_map().+*Thisisnotapplicabletothespeculativepagefaulthandler+*butinthatcase,thepteisfetchedearlierin+*handle_speculative_fault().*/vmf->pte=pte_offset_map(vmf->pmd,vmf->address);vmf->orig_pte=*vmf->pte;
@@ -4004,6 +4114,8 @@ static int handle_pte_fault(struct vm_fault *vmf)if(!vmf->pte){if(vma_is_anonymous(vmf->vma))returndo_anonymous_page(vmf);+elseif(vmf->flags&FAULT_FLAG_SPECULATIVE)+returnVM_FAULT_RETRY;elsereturndo_fault(vmf);}
@@ -4101,6 +4213,9 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,vmf.pmd=pmd_alloc(mm,vmf.pud,address);if(!vmf.pmd)returnVM_FAULT_OOM;+#ifdef CONFIG_SPF+vmf.sequence=raw_read_seqcount(&vma->vm_sequence);+#endifif(pmd_none(*vmf.pmd)&&transparent_hugepage_enabled(vma)){ret=create_huge_pmd(&vmf);if(!(ret&VM_FAULT_FALLBACK))
@@ -4134,6 +4249,206 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,returnhandle_pte_fault(&vmf);}+#ifdef CONFIG_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()addssomeprocessingwhichshouldbedonewhile+*hodlingthemmap_sem.+*/+inthandle_speculative_fault(structmm_struct*mm,unsignedlongaddress,+unsignedintflags)+{+structvm_faultvmf={+.address=address,+};+pgd_t*pgd,pgdval;+p4d_t*p4d,p4dval;+pud_tpudval;+intseq,ret=VM_FAULT_RETRY;+structvm_area_struct*vma;+#ifdef CONFIG_NUMA+structmempolicy*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;++vma=get_vma(mm,address);+if(!vma)+returnret;++seq=raw_read_seqcount(&vma->vm_sequence);/* rmb <-> seqlock,vma_rb_erase() */+if(seq&1)+gotoout_put;++/*+*Can'tcallvm_opsservicehaswedon'tknowwhattheywoulddo+*withtheVMA.+*Thisincludehugepagefromhugetlbfs.+*/+if(vma->vm_ops)+gotoout_put;++/*+*__anon_vma_prepare()requiresthemmap_semtobeheld+*becausevm_nextandvm_prevmustbesafe.Thiscan'tbeguaranteed+*inthespeculativepath.+*/+if(unlikely(!vma->anon_vma))+gotoout_put;++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))+gotoout_put;++if(vmf.vma_flags&VM_GROWSDOWN||vmf.vma_flags&VM_GROWSUP)+/*+*ThiscouldbedetectedbythecheckaddressagainstVMA's+*boundariesbutwewanttotraceitasnotsupportedinstead+*ofchanged.+*/+gotoout_put;++if(address<READ_ONCE(vma->vm_start)+||READ_ONCE(vma->vm_end)<=address)+gotoout_put;++if(!arch_vma_access_permitted(vma,flags&FAULT_FLAG_WRITE,+flags&FAULT_FLAG_INSTRUCTION,+flags&FAULT_FLAG_REMOTE)){+ret=VM_FAULT_SIGSEGV;+gotoout_put;+}++/* 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;+gotoout_put;+}+}elseif(unlikely(!(vmf.vma_flags&(VM_READ|VM_EXEC|VM_WRITE)))){+ret=VM_FAULT_SIGSEGV;+gotoout_put;+}++#ifdef CONFIG_NUMA+/*+*MPOL_INTERLEAVEimpliesadditionalcheckinmpol_misplaced()which+*arenotcompatiblewiththespeculativepagefaultprocessing.+*/+pol=__get_vma_policy(vma,address);+if(!pol)+pol=get_task_policy(current);+if(pol&&pol->mode==MPOL_INTERLEAVE)+gotoout_put;+#endif++/*+*DoaspeculativelookupofthePTEentry.+*/+local_irq_disable();+pgd=pgd_offset(mm,address);+pgdval=READ_ONCE(*pgd);+if(pgd_none(pgdval)||unlikely(pgd_bad(pgdval)))+gotoout_walk;++p4d=p4d_offset(pgd,address);+p4dval=READ_ONCE(*p4d);+if(p4d_none(p4dval)||unlikely(p4d_bad(p4dval)))+gotoout_walk;++vmf.pud=pud_offset(p4d,address);+pudval=READ_ONCE(*vmf.pud);+if(pud_none(pudval)||unlikely(pud_bad(pudval)))+gotoout_walk;++/* Huge pages at PUD level are not supported. */+if(unlikely(pud_trans_huge(pudval)))+gotoout_walk;++vmf.pmd=pmd_offset(vmf.pud,address);+vmf.orig_pmd=READ_ONCE(*vmf.pmd);+/*+*pmd_nonecouldmeanthatahugepagecollapseisinprogress+*inourbackascollapse_huge_page()markitbefore+*invalidatingthepte(whichisdoneoncetheIPIiscatched+*byallCPUandwehaveinterruptdisabled).+*ForthisreasonwecannothandleTHPinaspeculativewaysincewe+*can'tsafelyindentifyaninprogresscollapseoperationdoneinour+*backonthatPMD.+*Regardingtheorderofthefollowingchecks,seecommentin+*pmd_devmap_trans_unstable()+*/+if(unlikely(pmd_devmap(vmf.orig_pmd)||+pmd_none(vmf.orig_pmd)||pmd_trans_huge(vmf.orig_pmd)||+is_swap_pmd(vmf.orig_pmd)))+gotoout_walk;++/*+*Theabovedoesnotallocate/instantiatepage-tablesbecausedoingso+*wouldleadtothepossibilityofinstantiatingpage-tablesafter+*free_pgtables()--andconsequentlyleakingthem.+*+*Theresultisthatwetakeatleastone!speculativefaultperPMD+*inordertoinstantiateit.+*/++vmf.pte=pte_offset_map(vmf.pmd,address);+vmf.orig_pte=READ_ONCE(*vmf.pte);+barrier();/* See comment in handle_pte_fault() */+if(pte_none(vmf.orig_pte)){+pte_unmap(vmf.pte);+vmf.pte=NULL;+}++vmf.vma=vma;+vmf.pgoff=linear_page_index(vma,address);+vmf.gfp_mask=__get_fault_gfp_mask(vma);+vmf.sequence=seq;+vmf.flags=flags;++local_irq_enable();++/*+*Weneedtore-validatetheVMAaftercheckingthebounds,otherwise+*wemighthaveafalsepositiveonthebounds.+*/+if(read_seqcount_retry(&vma->vm_sequence,seq))+gotoout_put;++mem_cgroup_oom_enable();+ret=handle_pte_fault(&vmf);+mem_cgroup_oom_disable();++put_vma(vma);++/*+*ThetaskmayhaveenteredamemcgOOMsituationbut+*iftheallocationerrorwashandledgracefully(no+*VM_FAULT_OOM),thereisnoneedtokillanything.+*JustcleanuptheOOMstatepeacefully.+*/+if(task_in_memcg_oom(current)&&!(ret&VM_FAULT_OOM))+mem_cgroup_oom_synchronize(false);+returnret;++out_walk:+local_irq_enable();+out_put:+put_vma(vma);+returnret;+}+#endif /* CONFIG_SPF */+/**Bythetimewegethere,wealreadyholdthemmsemaphore*
This change is inspired by the Peter's proposal patch [1] which was
protecting the VMA using SRCU. Unfortunately, SRCU is not scaling well in
that particular case, and it is introducing major performance degradation
due to excessive scheduling operations.
To allow access to the mm_rb tree without grabbing the mmap_sem, this patch
is protecting it access using a rwlock. As the mm_rb tree is a O(log n)
search it is safe to protect it using such a lock. The VMA cache is not
protected by the new rwlock and it should not be used without holding the
mmap_sem.
To allow the picked VMA structure to be used once the rwlock is released, a
use count is added to the VMA structure. When the VMA is allocated it is
set to 1. Each time the VMA is picked with the rwlock held its use count
is incremented. Each time the VMA is released it is decremented. When the
use count hits zero, this means that the VMA is no more used and should be
freed.
This patch is preparing for 2 kind of VMA access :
- as usual, under the control of the mmap_sem,
- without holding the mmap_sem for the speculative page fault handler.
Access done under the control the mmap_sem doesn't require to grab the
rwlock to protect read access to the mm_rb tree, but access in write must
be done under the protection of the rwlock too. This affects inserting and
removing of elements in the RB tree.
The patch is introducing 2 new functions:
- vma_get() to find a VMA based on an address by holding the new rwlock.
- vma_put() to release the VMA when its no more used.
These services are designed to be used when access are made to the RB tree
without holding the mmap_sem.
When a VMA is removed from the RB tree, its vma->vm_rb field is cleared and
we rely on the WMB done when releasing the rwlock to serialize the write
with the RMB done in a later patch to check for the VMA's validity.
When free_vma is called, the file associated with the VMA is closed
immediately, but the policy and the file structure remained in used until
the VMA's use count reach 0, which may happens later when exiting an
in progress speculative page fault.
[1] https://patchwork.kernel.org/patch/5108281/
Signed-off-by: Laurent Dufour <redacted>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/mm_types.h | 4 ++
kernel/fork.c | 3 ++
mm/init-mm.c | 3 ++
mm/internal.h | 6 +++
mm/mmap.c | 120 ++++++++++++++++++++++++++++++++++-------------
5 files changed, 104 insertions(+), 32 deletions(-)
@@ -923,16 +959,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));vm_raw_write_end(next);-kmem_cache_free(vm_area_cachep,next);+put_vma(next);/**Inmprotect'scase6(seecommentsonvma_merge),*wemustremoveanothernexttoo.Itwouldclutter
@@ -2182,15 +2215,11 @@ 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. */-structvm_area_struct*find_vma(structmm_struct*mm,unsignedlongaddr)+staticstructvm_area_struct*__find_vma(structmm_struct*mm,+unsignedlongaddr){structrb_node*rb_node;-structvm_area_struct*vma;--/* Check the cache first. */-vma=vmacache_find(mm,addr);-if(likely(vma))-returnvma;+structvm_area_struct*vma=NULL;rb_node=mm->mm_rb.rb_node;
@@ -2208,13 +2237,40 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)rb_node=rb_node->rb_right;}+returnvma;+}++structvm_area_struct*find_vma(structmm_struct*mm,unsignedlongaddr)+{+structvm_area_struct*vma;++/* Check the cache first. */+vma=vmacache_find(mm,addr);+if(likely(vma))+returnvma;++vma=__find_vma(mm,addr);if(vma)vmacache_update(addr,vma);returnvma;}-EXPORT_SYMBOL(find_vma);+#ifdef CONFIG_SPF+structvm_area_struct*get_vma(structmm_struct*mm,unsignedlongaddr)+{+structvm_area_struct*vma=NULL;++read_lock(&mm->mm_rb_lock);+vma=__find_vma(mm,addr);+if(vma)+atomic_inc(&vma->vm_ref_count);+read_unlock(&mm->mm_rb_lock);++returnvma;+}+#endif+/**Sameasfind_vma,butalsoreturnapointertothepreviousVMAin*pprev.*/
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 <redacted>
---
include/linux/mm.h | 9 +++++++--
mm/memory.c | 6 +++---
2 files changed, 10 insertions(+), 5 deletions(-)
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 <redacted>
---
include/linux/migrate.h | 4 ++--
mm/memory.c | 2 +-
mm/migrate.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
@@ -3878,7 +3878,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;
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 <redacted>
---
include/linux/mm.h | 6 ++++++
mm/hugetlb.c | 2 ++
mm/khugepaged.c | 2 ++
mm/memory.c | 38 ++++++++++++++++++++------------------
4 files changed, 30 insertions(+), 18 deletions(-)
@@ -882,6 +882,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 */
@@ -3519,7 +3519,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;elsepage=vmf->page;
@@ -3773,7 +3773,7 @@ static int do_fault(struct vm_fault *vmf)ret=VM_FAULT_SIGBUS;elseif(!(vmf->flags&FAULT_FLAG_WRITE))ret=do_read_fault(vmf);-elseif(!(vma->vm_flags&VM_SHARED))+elseif(!(vmf->vma_flags&VM_SHARED))ret=do_cow_fault(vmf);elseret=do_shared_fault(vmf);
@@ -3830,7 +3830,7 @@ static int do_numa_page(struct vm_fault *vmf)*accessibleptes,somecanallowaccessbykernelmode.*/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);
@@ -3864,7 +3864,7 @@ static int do_numa_page(struct vm_fault *vmf)*Flagifthepageissharedbetweenmultipleaddressspaces.This*islaterusedwhendeterminingwhethertogrouptaskstogether*/-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);
@@ -3909,7 +3909,7 @@ static inline int wp_huge_pmd(struct vm_fault *vmf, pmd_t orig_pmd)returnvmf->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);returnVM_FAULT_FALLBACK;
@@ -4056,6 +4056,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,};unsignedintdirty=flags&FAULT_FLAG_WRITE;structmm_struct*mm=vma->vm_mm;
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]
[Build depends on CONFIG_SPF]
[Introduce vm_write_* inline function depending on CONFIG_SPF]
[Fix lock dependency between mapping->i_mmap_rwsem and vma->vm_sequence by
using vm_raw_write* functions]
Signed-off-by: Laurent Dufour <redacted>
---
include/linux/mm.h | 41 +++++++++++++++++++++++++++++++++++++++++
include/linux/mm_types.h | 3 +++
mm/memory.c | 2 ++
mm/mmap.c | 35 +++++++++++++++++++++++++++++++++++
4 files changed, 81 insertions(+)
@@ -333,6 +333,9 @@ struct vm_area_struct {structmempolicy*vm_policy;/* NUMA policy for the VMA */#endifstructvm_userfaultfd_ctxvm_userfaultfd_ctx;+#ifdef CONFIG_SPF+seqcount_tvm_sequence;+#endif}__randomize_layout;structcore_thread{
Define CONFIG_SPF for BOOK3S_64 and SMP. This enables the Speculative Page
Fault handler.
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 <redacted>
---
arch/powerpc/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
From: Matthew Wilcox <willy@infradead.org> Date: 2018-01-12 18:19:07
On Fri, Jan 12, 2018 at 06:26:02PM +0100, Laurent Dufour wrote:
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.
It seems like you introduced this bug in the previous patch, and now
you're fixing it in this patch? Why not merge the two?
From: Matthew Wilcox <willy@infradead.org> Date: 2018-01-12 18:48:41
On Fri, Jan 12, 2018 at 06:26:00PM +0100, Laurent Dufour wrote:
-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.
*/
+#ifdef CONFIG_SPF
+ write_lock(&mm->mm_rb_lock);
+#endif
rb_erase_augmented(&vma->vm_rb, root, &vma_gap_callbacks);
+#ifdef CONFIG_SPF
+ write_unlock(&mm->mm_rb_lock); /* wmb */
+#endif
I can't say I love this. Have you considered:
#ifdef CONFIG_SPF
#define vma_rb_write_lock(mm) write_lock(&mm->mm_rb_lock)
#define vma_rb_write_unlock(mm) write_unlock(&mm->mm_rb_lock)
#else
#define vma_rb_write_lock(mm) do { } while (0)
#define vma_rb_write_unlock(mm) do { } while (0)
#endif
Also, SPF is kind of uninformative. CONFIG_MM_SPF might be better?
Or perhaps even CONFIG_SPECULATIVE_PAGE_FAULT, just to make it really
painful to do these one-liner ifdefs that make the code so hard to read.
From: Thomas Gleixner <hidden> Date: 2018-01-12 18:58:21
On Fri, 12 Jan 2018, Laurent Dufour wrote:
quoted hunk
Introduce CONFIG_SPF which turns on the Speculative Page Fault handler when
building for 64bits with SMP.
Signed-off-by: Laurent Dufour <redacted>
---
arch/x86/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
Can you please put that into a generic place as
config SPF
bool
and let the architectures select it.
Also SPF could be bit more elaborate and self explaining for the causual
reader. 3 letter acronyms are reserved for non existing agencies.
Thanks,
tglx
From: Matthew Wilcox <willy@infradead.org> Date: 2018-01-12 19:03:11
On Fri, Jan 12, 2018 at 06:26:06PM +0100, Laurent Dufour wrote:
quoted hunk
@@ -1354,7 +1354,10 @@ extern int handle_mm_fault(struct vm_area_struct *vma, unsigned long address, unsigned int flags); #ifdef CONFIG_SPF extern int handle_speculative_fault(struct mm_struct *mm,+ unsigned long address, unsigned int flags,+ struct vm_area_struct **vma);
I think this shows that we need to create 'struct vm_fault' on the stack
in the arch code and then pass it to handle_speculative_fault(), followed
by handle_mm_fault(). That should be quite a nice cleanup actually.
I know that's only 30+ architectures to change ;-)
From: Matthew Wilcox <willy@infradead.org> Date: 2018-01-13 04:24:28
On Fri, Jan 12, 2018 at 11:02:51AM -0800, Matthew Wilcox wrote:
On Fri, Jan 12, 2018 at 06:26:06PM +0100, Laurent Dufour wrote:
quoted
@@ -1354,7 +1354,10 @@ extern int handle_mm_fault(struct vm_area_struct *vma, unsigned long address, unsigned int flags); #ifdef CONFIG_SPF extern int handle_speculative_fault(struct mm_struct *mm,+ unsigned long address, unsigned int flags,+ struct vm_area_struct **vma);
I think this shows that we need to create 'struct vm_fault' on the stack
in the arch code and then pass it to handle_speculative_fault(), followed
by handle_mm_fault(). That should be quite a nice cleanup actually.
I know that's only 30+ architectures to change ;-)
Of course, we don't need to change them all. Try this:
Subject: [PATCH] Add vm_handle_fault
For the speculative fault handler, we want to create the struct vm_fault
on the stack in the arch code and pass it into the generic mm code.
To avoid changing 30+ architectures, leave handle_mm_fault with its
current function signature and move its guts into the new vm_handle_fault
function. Even this saves a nice 172 bytes on the random x86-64 .config
I happen to have around.
Signed-off-by: Matthew Wilcox <redacted>
@@ -3977,36 +3977,28 @@ static int handle_pte_fault(struct vm_fault *vmf)*Themmap_semmayhavebeenreleaseddependingonflagsandour*returnvalue.Seefilemap_fault()and__lock_page_or_retry().*/-staticint__handle_mm_fault(structvm_area_struct*vma,unsignedlongaddress,-unsignedintflags)+staticint__handle_mm_fault(structvm_fault*vmf){-structvm_faultvmf={-.vma=vma,-.address=address&PAGE_MASK,-.flags=flags,-.pgoff=linear_page_index(vma,address),-.gfp_mask=__get_fault_gfp_mask(vma),-};-unsignedintdirty=flags&FAULT_FLAG_WRITE;-structmm_struct*mm=vma->vm_mm;+unsignedintdirty=vmf->flags&FAULT_FLAG_WRITE;+structmm_struct*mm=vmf->vma->vm_mm;pgd_t*pgd;p4d_t*p4d;intret;-pgd=pgd_offset(mm,address);-p4d=p4d_alloc(mm,pgd,address);+pgd=pgd_offset(mm,vmf->address);+p4d=p4d_alloc(mm,pgd,vmf->address);if(!p4d)returnVM_FAULT_OOM;-vmf.pud=pud_alloc(mm,p4d,address);-if(!vmf.pud)+vmf->pud=pud_alloc(mm,p4d,vmf->address);+if(!vmf->pud)returnVM_FAULT_OOM;-if(pud_none(*vmf.pud)&&transparent_hugepage_enabled(vma)){-ret=create_huge_pud(&vmf);+if(pud_none(*vmf->pud)&&transparent_hugepage_enabled(vmf->vma)){+ret=create_huge_pud(vmf);if(!(ret&VM_FAULT_FALLBACK))returnret;}else{-pud_torig_pud=*vmf.pud;+pud_torig_pud=*vmf->pud;barrier();if(pud_trans_huge(orig_pud)||pud_devmap(orig_pud)){
@@ -4014,50 +4006,51 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,/* NUMA case for anonymous PUDs would go here */if(dirty&&!pud_access_permitted(orig_pud,WRITE)){-ret=wp_huge_pud(&vmf,orig_pud);+ret=wp_huge_pud(vmf,orig_pud);if(!(ret&VM_FAULT_FALLBACK))returnret;}else{-huge_pud_set_accessed(&vmf,orig_pud);+huge_pud_set_accessed(vmf,orig_pud);return0;}}}-vmf.pmd=pmd_alloc(mm,vmf.pud,address);-if(!vmf.pmd)+vmf->pmd=pmd_alloc(mm,vmf->pud,vmf->address);+if(!vmf->pmd)returnVM_FAULT_OOM;-if(pmd_none(*vmf.pmd)&&transparent_hugepage_enabled(vma)){-ret=create_huge_pmd(&vmf);+if(pmd_none(*vmf->pmd)&&transparent_hugepage_enabled(vmf->vma)){+ret=create_huge_pmd(vmf);if(!(ret&VM_FAULT_FALLBACK))returnret;}else{-pmd_torig_pmd=*vmf.pmd;+pmd_torig_pmd=*vmf->pmd;barrier();if(unlikely(is_swap_pmd(orig_pmd))){VM_BUG_ON(thp_migration_supported()&&!is_pmd_migration_entry(orig_pmd));if(is_pmd_migration_entry(orig_pmd))-pmd_migration_entry_wait(mm,vmf.pmd);+pmd_migration_entry_wait(mm,vmf->pmd);return0;}if(pmd_trans_huge(orig_pmd)||pmd_devmap(orig_pmd)){-if(pmd_protnone(orig_pmd)&&vma_is_accessible(vma))-returndo_huge_pmd_numa_page(&vmf,orig_pmd);+if(pmd_protnone(orig_pmd)&&+vma_is_accessible(vmf->vma))+returndo_huge_pmd_numa_page(vmf,orig_pmd);if(dirty&&!pmd_access_permitted(orig_pmd,WRITE)){-ret=wp_huge_pmd(&vmf,orig_pmd);+ret=wp_huge_pmd(vmf,orig_pmd);if(!(ret&VM_FAULT_FALLBACK))returnret;}else{-huge_pmd_set_accessed(&vmf,orig_pmd);+huge_pmd_set_accessed(vmf,orig_pmd);return0;}}}-returnhandle_pte_fault(&vmf);+returnhandle_pte_fault(vmf);}/*
@@ -4066,9 +4059,10 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,*Themmap_semmayhavebeenreleaseddependingonflagsandour*returnvalue.Seefilemap_fault()and__lock_page_or_retry().*/-inthandle_mm_fault(structvm_area_struct*vma,unsignedlongaddress,-unsignedintflags)+intvm_handle_fault(structvm_fault*vmf){+unsignedintflags=vmf->flags;+structvm_area_struct*vma=vmf->vma;intret;__set_current_state(TASK_RUNNING);
@@ -4092,9 +4086,9 @@ int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,mem_cgroup_oom_enable();if(unlikely(is_vm_hugetlb_page(vma)))-ret=hugetlb_fault(vma->vm_mm,vma,address,flags);+ret=hugetlb_fault(vma->vm_mm,vma,vmf->address,flags);else-ret=__handle_mm_fault(vma,address,flags);+ret=__handle_mm_fault(vmf);if(flags&FAULT_FLAG_USER){mem_cgroup_oom_disable();
@@ -4110,6 +4104,26 @@ int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,returnret;}++/*+*Bythetimewegethere,wealreadyholdthemmsemaphore+*+*Themmap_semmayhavebeenreleaseddependingonflagsandour+*returnvalue.Seefilemap_fault()and__lock_page_or_retry().+*/+inthandle_mm_fault(structvm_area_struct*vma,unsignedlongaddress,+unsignedintflags)+{+structvm_faultvmf={+.vma=vma,+.address=address&PAGE_MASK,+.flags=flags,+.pgoff=linear_page_index(vma,address),+.gfp_mask=__get_fault_gfp_mask(vma),+};++returnvm_handle_fault(&vmf);+}EXPORT_SYMBOL_GPL(handle_mm_fault);#ifndef __PAGETABLE_P4D_FOLDED
Hi Thomas,
Thanks for reviewing this series.
On 12/01/2018 19:57, Thomas Gleixner wrote:
On Fri, 12 Jan 2018, Laurent Dufour wrote:
quoted
Introduce CONFIG_SPF which turns on the Speculative Page Fault handler when
building for 64bits with SMP.
Signed-off-by: Laurent Dufour <redacted>
---
arch/x86/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
Can you please put that into a generic place as
config SPF
bool
and let the architectures select it.
I'll change that to let the architectures (x86 and ppc64 currently)
selecting it, but the definition will remain in the arch/xxx/Kconfig file
since it depends on the architecture support in the page fault handler.
Also SPF could be bit more elaborate and self explaining for the causual
reader. 3 letter acronyms are reserved for non existing agencies.
That's true 3 letter acronyms are already reserved...
I'll change it to CONFIG_SPECULATIVE_PAGE_FAULT as suggested by Matthew Wilcox.
Thanks,
Laurent.
Hi Matthew,
Thanks for reviewing this series.
On 12/01/2018 19:48, Matthew Wilcox wrote:
On Fri, Jan 12, 2018 at 06:26:00PM +0100, Laurent Dufour wrote:
quoted
-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.
*/
+#ifdef CONFIG_SPF
+ write_lock(&mm->mm_rb_lock);
+#endif
rb_erase_augmented(&vma->vm_rb, root, &vma_gap_callbacks);
+#ifdef CONFIG_SPF
+ write_unlock(&mm->mm_rb_lock); /* wmb */
+#endif
I can't say I love this. Have you considered:
#ifdef CONFIG_SPF
#define vma_rb_write_lock(mm) write_lock(&mm->mm_rb_lock)
#define vma_rb_write_unlock(mm) write_unlock(&mm->mm_rb_lock)
#else
#define vma_rb_write_lock(mm) do { } while (0)
#define vma_rb_write_unlock(mm) do { } while (0)
#endif
I haven't consider this, but this sounds to be smarter. I'll do that.
Also, SPF is kind of uninformative. CONFIG_MM_SPF might be better?
Or perhaps even CONFIG_SPECULATIVE_PAGE_FAULT, just to make it really
painful to do these one-liner ifdefs that make the code so hard to read.
Thomas also complained about that, and I agree, SPF is quite cryptic. This
being said, I don't think that CONFIG_MM_SPF will be far better, so I'll
change this define to CONFIG_SPECULATIVE_PAGE_FAULT, even if it's longer,
it should not be too much present in the code.
Thanks,
Laurent.
From: Thomas Gleixner <hidden> Date: 2018-01-15 17:50:58
On Mon, 15 Jan 2018, Laurent Dufour wrote:
On 12/01/2018 19:57, Thomas Gleixner wrote:
quoted
On Fri, 12 Jan 2018, Laurent Dufour wrote:
quoted
Introduce CONFIG_SPF which turns on the Speculative Page Fault handler when
building for 64bits with SMP.
Signed-off-by: Laurent Dufour <redacted>
---
arch/x86/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
Can you please put that into a generic place as
config SPF
bool
and let the architectures select it.
I'll change that to let the architectures (x86 and ppc64 currently)
selecting it, but the definition will remain in the arch/xxx/Kconfig file
since it depends on the architecture support in the page fault handler.
Errm. No.
config SPECULATIVE_PAGE_FAULT
bool
goes into a generic config file, e.g. mm/Kconfig
Each architecture which implements support does:
select SPECULATIVE_PAGE_FAULT
in arch/xxx/Kconfig
Thanks,
tglx
Introduce CONFIG_SPF which turns on the Speculative Page Fault handler when
building for 64bits with SMP.
Signed-off-by: Laurent Dufour <redacted>
---
arch/x86/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
Can you please put that into a generic place as
config SPF
bool
and let the architectures select it.
I'll change that to let the architectures (x86 and ppc64 currently)
selecting it, but the definition will remain in the arch/xxx/Kconfig file
since it depends on the architecture support in the page fault handler.
Errm. No.
config SPECULATIVE_PAGE_FAULT
bool
goes into a generic config file, e.g. mm/Kconfig
Each architecture which implements support does:
select SPECULATIVE_PAGE_FAULT
in arch/xxx/Kconfig
Oh ok, I think I got it this time ;)
Will do this way, this will be smarter.
Thanks a lot,
Laurent.
On Fri, Jan 12, 2018 at 06:26:02PM +0100, Laurent Dufour wrote:
quoted
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.
It seems like you introduced this bug in the previous patch, and now
you're fixing it in this patch? Why not merge the two?
You're right this is a fix from the previous patch. Initially my idea was
to keep the original Peter's patch as is, but sounds that this is not a
good idea.
I'll merge it in the previous one.
Thanks,
Laurent.
On Fri, Jan 12, 2018 at 11:02:51AM -0800, Matthew Wilcox wrote:
quoted
On Fri, Jan 12, 2018 at 06:26:06PM +0100, Laurent Dufour wrote:
quoted
@@ -1354,7 +1354,10 @@ extern int handle_mm_fault(struct vm_area_struct *vma, unsigned long address, unsigned int flags); #ifdef CONFIG_SPF extern int handle_speculative_fault(struct mm_struct *mm,+ unsigned long address, unsigned int flags,+ struct vm_area_struct **vma);
I think this shows that we need to create 'struct vm_fault' on the stack
in the arch code and then pass it to handle_speculative_fault(), followed
by handle_mm_fault(). That should be quite a nice cleanup actually.
I know that's only 30+ architectures to change ;-)
Of course, we don't need to change them all. Try this:
That would be good candidate for a clean up but I'm not sure this should be
part of this already too long series.
If you don't mind, unless a global agreement is stated on that, I'd prefer
to postpone such a change once the initial series is accepted.
Cheers,
Laurent.
quoted hunk
Subject: [PATCH] Add vm_handle_fault
For the speculative fault handler, we want to create the struct vm_fault
on the stack in the arch code and pass it into the generic mm code.
To avoid changing 30+ architectures, leave handle_mm_fault with its
current function signature and move its guts into the new vm_handle_fault
function. Even this saves a nice 172 bytes on the random x86-64 .config
I happen to have around.
Signed-off-by: Matthew Wilcox <redacted>
@@ -3977,36 +3977,28 @@ static int handle_pte_fault(struct vm_fault *vmf)*Themmap_semmayhavebeenreleaseddependingonflagsandour*returnvalue.Seefilemap_fault()and__lock_page_or_retry().*/-staticint__handle_mm_fault(structvm_area_struct*vma,unsignedlongaddress,-unsignedintflags)+staticint__handle_mm_fault(structvm_fault*vmf){-structvm_faultvmf={-.vma=vma,-.address=address&PAGE_MASK,-.flags=flags,-.pgoff=linear_page_index(vma,address),-.gfp_mask=__get_fault_gfp_mask(vma),-};-unsignedintdirty=flags&FAULT_FLAG_WRITE;-structmm_struct*mm=vma->vm_mm;+unsignedintdirty=vmf->flags&FAULT_FLAG_WRITE;+structmm_struct*mm=vmf->vma->vm_mm;pgd_t*pgd;p4d_t*p4d;intret;-pgd=pgd_offset(mm,address);-p4d=p4d_alloc(mm,pgd,address);+pgd=pgd_offset(mm,vmf->address);+p4d=p4d_alloc(mm,pgd,vmf->address);if(!p4d)returnVM_FAULT_OOM;-vmf.pud=pud_alloc(mm,p4d,address);-if(!vmf.pud)+vmf->pud=pud_alloc(mm,p4d,vmf->address);+if(!vmf->pud)returnVM_FAULT_OOM;-if(pud_none(*vmf.pud)&&transparent_hugepage_enabled(vma)){-ret=create_huge_pud(&vmf);+if(pud_none(*vmf->pud)&&transparent_hugepage_enabled(vmf->vma)){+ret=create_huge_pud(vmf);if(!(ret&VM_FAULT_FALLBACK))returnret;}else{-pud_torig_pud=*vmf.pud;+pud_torig_pud=*vmf->pud;barrier();if(pud_trans_huge(orig_pud)||pud_devmap(orig_pud)){
@@ -4014,50 +4006,51 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,/* NUMA case for anonymous PUDs would go here */if(dirty&&!pud_access_permitted(orig_pud,WRITE)){-ret=wp_huge_pud(&vmf,orig_pud);+ret=wp_huge_pud(vmf,orig_pud);if(!(ret&VM_FAULT_FALLBACK))returnret;}else{-huge_pud_set_accessed(&vmf,orig_pud);+huge_pud_set_accessed(vmf,orig_pud);return0;}}}-vmf.pmd=pmd_alloc(mm,vmf.pud,address);-if(!vmf.pmd)+vmf->pmd=pmd_alloc(mm,vmf->pud,vmf->address);+if(!vmf->pmd)returnVM_FAULT_OOM;-if(pmd_none(*vmf.pmd)&&transparent_hugepage_enabled(vma)){-ret=create_huge_pmd(&vmf);+if(pmd_none(*vmf->pmd)&&transparent_hugepage_enabled(vmf->vma)){+ret=create_huge_pmd(vmf);if(!(ret&VM_FAULT_FALLBACK))returnret;}else{-pmd_torig_pmd=*vmf.pmd;+pmd_torig_pmd=*vmf->pmd;barrier();if(unlikely(is_swap_pmd(orig_pmd))){VM_BUG_ON(thp_migration_supported()&&!is_pmd_migration_entry(orig_pmd));if(is_pmd_migration_entry(orig_pmd))-pmd_migration_entry_wait(mm,vmf.pmd);+pmd_migration_entry_wait(mm,vmf->pmd);return0;}if(pmd_trans_huge(orig_pmd)||pmd_devmap(orig_pmd)){-if(pmd_protnone(orig_pmd)&&vma_is_accessible(vma))-returndo_huge_pmd_numa_page(&vmf,orig_pmd);+if(pmd_protnone(orig_pmd)&&+vma_is_accessible(vmf->vma))+returndo_huge_pmd_numa_page(vmf,orig_pmd);if(dirty&&!pmd_access_permitted(orig_pmd,WRITE)){-ret=wp_huge_pmd(&vmf,orig_pmd);+ret=wp_huge_pmd(vmf,orig_pmd);if(!(ret&VM_FAULT_FALLBACK))returnret;}else{-huge_pmd_set_accessed(&vmf,orig_pmd);+huge_pmd_set_accessed(vmf,orig_pmd);return0;}}}-returnhandle_pte_fault(&vmf);+returnhandle_pte_fault(vmf);}/*
@@ -4066,9 +4059,10 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,*Themmap_semmayhavebeenreleaseddependingonflagsandour*returnvalue.Seefilemap_fault()and__lock_page_or_retry().*/-inthandle_mm_fault(structvm_area_struct*vma,unsignedlongaddress,-unsignedintflags)+intvm_handle_fault(structvm_fault*vmf){+unsignedintflags=vmf->flags;+structvm_area_struct*vma=vmf->vma;intret;__set_current_state(TASK_RUNNING);
@@ -4092,9 +4086,9 @@ int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,mem_cgroup_oom_enable();if(unlikely(is_vm_hugetlb_page(vma)))-ret=hugetlb_fault(vma->vm_mm,vma,address,flags);+ret=hugetlb_fault(vma->vm_mm,vma,vmf->address,flags);else-ret=__handle_mm_fault(vma,address,flags);+ret=__handle_mm_fault(vmf);if(flags&FAULT_FLAG_USER){mem_cgroup_oom_disable();
@@ -4110,6 +4104,26 @@ int handle_mm_fault(struct vm_area_struct *vma, unsigned long address,returnret;}++/*+*Bythetimewegethere,wealreadyholdthemmsemaphore+*+*Themmap_semmayhavebeenreleaseddependingonflagsandour+*returnvalue.Seefilemap_fault()and__lock_page_or_retry().+*/+inthandle_mm_fault(structvm_area_struct*vma,unsignedlongaddress,+unsignedintflags)+{+structvm_faultvmf={+.vma=vma,+.address=address&PAGE_MASK,+.flags=flags,+.pgoff=linear_page_index(vma,address),+.gfp_mask=__get_fault_gfp_mask(vma),+};++returnvm_handle_fault(&vmf);+}EXPORT_SYMBOL_GPL(handle_mm_fault);#ifndef __PAGETABLE_P4D_FOLDED
From: Matthew Wilcox <willy@infradead.org> Date: 2018-01-16 14:59:48
On Tue, Jan 16, 2018 at 03:47:51PM +0100, Laurent Dufour wrote:
On 13/01/2018 05:23, Matthew Wilcox wrote:
quoted
Of course, we don't need to change them all. Try this:
That would be good candidate for a clean up but I'm not sure this should be
part of this already too long series.
If you don't mind, unless a global agreement is stated on that, I'd prefer
to postpone such a change once the initial series is accepted.
Actually, I think this can go in first, independently of the speculative
fault series. It's a win in memory savings, and probably shaves a
cycle or two off the fault handler due to less argument marshalling in
the call-stack.
From: Kirill A. Shutemov <hidden> Date: 2018-01-16 15:11:49
On Fri, Jan 12, 2018 at 06:25:44PM +0100, Laurent Dufour wrote:
------------------
Benchmarks results
Base kernel is 4.15-rc6-mmotm-2018-01-04-16-19
SPF is BASE + this series
Do you have THP=always here? Lack of THP support worries me.
What is performance in the worst case scenario? Like when we go far enough into
speculative code path on every page fault and then fallback to normal page
fault?
--
Kirill A. Shutemov
Hi Kirill,
Thanks for reviewing this series.
On 16/01/2018 16:11, Kirill A. Shutemov wrote:
On Fri, Jan 12, 2018 at 06:25:44PM +0100, Laurent Dufour wrote:
quoted
------------------
Benchmarks results
Base kernel is 4.15-rc6-mmotm-2018-01-04-16-19
SPF is BASE + this series
Do you have THP=always here? Lack of THP support worries me.
Yes my kernel is built with THP=always.
For the record, I wrote all the code to support THP, but when I was about
to plug it into the speculative page fault handler, I was wondering about
the pmd_none() check and this raises the issue with khugepaged and the way
it is invalidating the pmd before collapsing the underlying pages.
Currently, there is no easy way to detect when such a collapsing operation
is occurring.
What is performance in the worst case scenario? Like when we go far enough into
speculative code path on every page fault and then fallback to normal page
fault?
I did further tests focusing on the THP with a patched ebizzy (to use
posix_memalign() and MADV_HUGEPAGE) to force the use of the transparent
huge pages. I double checked that use through /proc/#/smaps.
Here is the result I got on a 16 CPUs x86 VM (higher the best):
BASE SPF
mean 276.83 276.93 record/s
max 280 280 record/s
The run was done 100 times using a large enough size records (128 MB).
Here is also the event I recorded when running ebizzy during 60s:
275 records/s
Performance counter stats for './ebizzy -HT -s 134217728':
182,470 faults
5,085 spf
176,634 pagefault:spf_vma_notsup
10.518504612 seconds time elapsed
Most of the speculative page fault events were aborted because the VMA was
not supported, which is matching the huge pages (pagefault:spf_vma_notsup).
Only 5,000 were managed fully without holding the mmap_sem, I guess for
other part of the memory's process.
Running the same command on the Base kernel gave:
293 records/s
Performance counter stats for './ebizzy -HT -s 134217728':
183,170 faults
10.660787623 seconds time elapsed
So I'd say that aborting the speculative page fault handler when a THP is
detected, has no visible impact.
Cheers,
Laurent.