* [PATCH v6 12/24] mm: Introduce __lru_cache_add_active_or_unevictable
From: Laurent Dufour @ 2018-01-12 17:25 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 | 10 ++++++++--
mm/memory.c | 8 ++++----
mm/swap.c | 6 +++---
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index a1a3f4ed94ce..99377b66ea93 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -337,8 +337,14 @@ extern void deactivate_file_page(struct page *page);
extern void mark_page_lazyfree(struct page *page);
extern void swap_setup(void);
-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 e4c0f08b78e8..cbd7e5c3a42f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2540,7 +2540,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
@@ -3082,7 +3082,7 @@ int do_swap_page(struct vm_fault *vmf)
if (unlikely(page != swapcache && swapcache)) {
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 {
do_page_add_anon_rmap(page, vma, vmf->address, exclusive);
mem_cgroup_commit_charge(page, memcg, true, false);
@@ -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);
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);
@@ -3484,7 +3484,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 566cfb9fdaf3..7e25a74397b9 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -455,12 +455,12 @@ void lru_cache_add(struct page *page)
* 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);
else if (!TestSetPageMlocked(page)) {
/*
--
2.7.4
^ permalink raw reply related
* [PATCH v6 13/24] mm: Introduce __maybe_mkwrite()
From: Laurent Dufour @ 2018-01-12 17:25 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 f14a73a8d420..d77a11067b94 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -685,13 +685,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 cbd7e5c3a42f..c24891d5676f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2438,7 +2438,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);
@@ -2529,8 +2529,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
^ permalink raw reply related
* [PATCH v6 14/24] mm: Introduce __vm_normal_page()
From: Laurent Dufour @ 2018-01-12 17:25 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 d77a11067b94..0d1b8d2d1e4f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1260,8 +1260,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 c24891d5676f..a7cb109bf25a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -826,8 +826,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);
@@ -836,7 +837,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;
@@ -868,8 +869,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;
@@ -878,7 +879,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;
}
}
@@ -2722,7 +2723,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
@@ -3837,7 +3839,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
^ permalink raw reply related
* [PATCH v6 15/24] mm: Introduce __page_add_new_anon_rmap()
From: Laurent Dufour @ 2018-01-12 17:25 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 988d176472df..a5d282573093 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -174,8 +174,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 a7cb109bf25a..0ce303260ad1 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2539,7 +2539,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);
/*
@@ -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:
@@ -3484,7 +3484,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 47db27f8049e..6ec168ba5f73 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1136,7 +1136,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
@@ -1146,12 +1146,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
^ permalink raw reply related
* [PATCH v6 16/24] mm: Protect mm_rb tree with a rwlock
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 <ldufour@linux.vnet.ibm.com>
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(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index e0e3df3b9641..2684df7e7294 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -335,6 +335,7 @@ struct vm_area_struct {
struct vm_userfaultfd_ctx vm_userfaultfd_ctx;
#ifdef CONFIG_SPF
seqcount_t vm_sequence;
+ atomic_t vm_ref_count; /* see vma_get(), vma_put() */
#endif
} __randomize_layout;
@@ -353,6 +354,9 @@ struct kioctx_table;
struct mm_struct {
struct vm_area_struct *mmap; /* list of VMAs */
struct rb_root mm_rb;
+#ifdef CONFIG_SPF
+ rwlock_t mm_rb_lock;
+#endif
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 0914307d4f3b..d99606e1e9ba 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -898,6 +898,9 @@ 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;
+#ifdef CONFIG_SPF
+ rwlock_init(&mm->mm_rb_lock);
+#endif
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 f94d5d15ebc0..aaa5d7851d87 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -17,6 +17,9 @@
struct mm_struct init_mm = {
.mm_rb = RB_ROOT,
+#ifdef CONFIG_SPF
+ .mm_rb_lock = __RW_LOCK_UNLOCKED(init_mm.mm_rb_lock),
+#endif
.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 62d8c34e63d5..4b9c3357bd6c 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -40,6 +40,12 @@ void page_writeback_init(void);
int do_swap_page(struct vm_fault *vmf);
+#ifdef CONFIG_SPF
+extern struct vm_area_struct *get_vma(struct mm_struct *mm,
+ unsigned long addr);
+extern void put_vma(struct vm_area_struct *vma);
+#endif
+
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 960e2f16ffcf..972ddee0b151 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -160,6 +160,27 @@ void unlink_file_vma(struct vm_area_struct *vma)
}
}
+static void __free_vma(struct vm_area_struct *vma)
+{
+ if (vma->vm_file)
+ fput(vma->vm_file);
+ mpol_put(vma_policy(vma));
+ kmem_cache_free(vm_area_cachep, vma);
+}
+
+#ifdef CONFIG_SPF
+void put_vma(struct vm_area_struct *vma)
+{
+ if (atomic_dec_and_test(&vma->vm_ref_count))
+ __free_vma(vma);
+}
+#else
+static inline void put_vma(struct vm_area_struct *vma)
+{
+ return __free_vma(vma);
+}
+#endif
+
/*
* Close a vm structure and free it, returning the next.
*/
@@ -170,10 +191,7 @@ 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);
+ put_vma(vma);
return next;
}
@@ -411,26 +429,41 @@ 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.
*/
+#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
+
+ /*
+ * 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 +471,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);
}
/*
@@ -558,10 +591,6 @@ void __vma_link_rb(struct mm_struct *mm, struct vm_area_struct *vma,
else
mm->highest_vm_end = vm_end_gap(vma);
-#ifdef CONFIG_SPF
- seqcount_init(&vma->vm_sequence);
-#endif
-
/*
* 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
@@ -571,10 +600,17 @@ 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.
*/
+#ifdef CONFIG_SPF
+ atomic_set(&vma->vm_ref_count, 1);
+ write_lock(&mm->mm_rb_lock);
+#endif
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);
+#ifdef CONFIG_SPF
+ write_unlock(&mm->mm_rb_lock);
+#endif
}
static void __vma_link_file(struct vm_area_struct *vma)
@@ -650,7 +686,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;
@@ -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);
/*
* In mprotect's case 6 (see comments on vma_merge),
* we must remove another next too. It would clutter
@@ -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. */
-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;
@@ -2208,13 +2237,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);
+#ifdef CONFIG_SPF
+struct vm_area_struct *get_vma(struct mm_struct *mm, unsigned long addr)
+{
+ struct vm_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);
+
+ return vma;
+}
+#endif
+
/*
* Same as find_vma, but also return a pointer to the previous VMA in *pprev.
*/
@@ -2582,7 +2638,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
^ permalink raw reply related
* [PATCH v6 17/24] mm: Provide speculative fault infrastructure
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 <ldufour@linux.vnet.ibm.com>
---
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(-)
diff --git a/include/linux/hugetlb_inline.h b/include/linux/hugetlb_inline.h
index 0660a03d37d9..9e25283d6fc9 100644
--- a/include/linux/hugetlb_inline.h
+++ b/include/linux/hugetlb_inline.h
@@ -8,7 +8,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 0d1b8d2d1e4f..4d8a7621da8a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -331,6 +331,10 @@ 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 */
+#ifdef CONFIG_SPF
+ unsigned int sequence;
+ pmd_t orig_pmd; /* value of PMD at the time of fault */
+#endif
pmd_t *pmd; /* Pointer to pmd entry matching
* the 'address' */
pud_t *pud; /* Pointer to pud entry matching
@@ -1348,6 +1352,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 CONFIG_SPF
+extern int handle_speculative_fault(struct mm_struct *mm,
+ unsigned long address, unsigned int flags);
+#endif /* CONFIG_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 34ce3ebf97d5..70e4d2688e7b 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -456,8 +456,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 4b9c3357bd6c..f35998b72183 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -44,7 +44,21 @@ int do_swap_page(struct vm_fault *vmf);
extern struct vm_area_struct *get_vma(struct mm_struct *mm,
unsigned long addr);
extern void put_vma(struct vm_area_struct *vma);
-#endif
+
+static inline bool vma_has_changed(struct vm_fault *vmf)
+{
+ int ret = RB_EMPTY_NODE(&vmf->vma->vm_rb);
+ unsigned int seq = READ_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;
+}
+#endif /* CONFIG_SPF */
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 0ce303260ad1..96720cc7ca74 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -769,7 +769,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);
pr_alert("file:%pD fault:%pf mmap:%pf readpage:%pf\n",
vma->vm_file,
vma->vm_ops ? vma->vm_ops->fault : NULL,
@@ -2445,19 +2446,108 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
pte_unmap_unlock(vmf->pte, vmf->ptl);
}
+#ifdef CONFIG_SPF
static bool pte_spinlock(struct vm_fault *vmf)
{
+ bool ret = false;
+ pmd_t pmdval;
+
+ /* 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;
+
+ /*
+ * We check if the pmd value is still the same to ensure that there
+ * is a huge collapse operation in progress in our back.
+ */
+ pmdval = READ_ONCE(*vmf->pmd);
+ if (!pmd_same(pmdval, vmf->orig_pmd))
+ goto out;
+
+ vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
+ spin_lock(vmf->ptl);
+
+ if (vma_has_changed(vmf)) {
+ spin_unlock(vmf->ptl);
+ goto out;
+ }
+
+ ret = true;
+out:
+ local_irq_enable();
+ return ret;
+}
+#else
+static inline bool pte_spinlock(struct vm_fault *vmf)
+{
vmf->ptl = pte_lockptr(vmf->vma->vm_mm, vmf->pmd);
spin_lock(vmf->ptl);
return true;
}
+#endif /* CONFIG_SPF */
+#ifdef CONFIG_SPF
static bool pte_map_lock(struct vm_fault *vmf)
{
+ bool ret = false;
+ pte_t *pte;
+ pmd_t pmdval;
+ 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;
+
+ /*
+ * We check if the pmd value is still the same to ensure that there
+ * is a huge collapse operation in progress in our back.
+ */
+ pmdval = READ_ONCE(*vmf->pmd);
+ if (!pmd_same(pmdval, vmf->orig_pmd))
+ 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;
+}
+#else
+static inline 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;
}
+#endif /* CONFIG_SPF */
/*
* Handle the case of a page which we actually need to copy to a new page.
@@ -3182,6 +3272,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);
@@ -3224,7 +3322,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);
@@ -3967,13 +4065,22 @@ static int handle_pte_fault(struct vm_fault *vmf)
if (unlikely(pmd_none(*vmf->pmd))) {
/*
+ * In the case of the speculative page fault handler we abort
+ * the speculative path immediately as the pmd is probably
+ * in the way to be converted in a huge one. We will try
+ * again holding the mmap_sem (which implies that the collapse
+ * operation is done).
+ */
+ if (vmf->flags & FAULT_FLAG_SPECULATIVE)
+ return VM_FAULT_RETRY;
+ /*
* Leave __pte_alloc() until later: because vm_ops->fault may
* want to allocate huge page, and if we expose page table
* for an instant, it will be difficult to retract from
* concurrent faults and from rmap lookups.
*/
vmf->pte = NULL;
- } else {
+ } else if (!(vmf->flags & FAULT_FLAG_SPECULATIVE)) {
/* See comment in pte_alloc_one_map() */
if (pmd_devmap_trans_unstable(vmf->pmd))
return 0;
@@ -3982,6 +4089,9 @@ static int handle_pte_fault(struct vm_fault *vmf)
* pmd from under us anymore at this point because we hold the
* mmap_sem read mode and khugepaged takes it in write mode.
* So now it's safe to run pte_offset_map().
+ * This is not applicable to the speculative page fault handler
+ * but in that case, the pte is fetched earlier in
+ * 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))
return do_anonymous_page(vmf);
+ else if (vmf->flags & FAULT_FLAG_SPECULATIVE)
+ return VM_FAULT_RETRY;
else
return do_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)
return VM_FAULT_OOM;
+#ifdef CONFIG_SPF
+ vmf.sequence = raw_read_seqcount(&vma->vm_sequence);
+#endif
if (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,
return handle_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() 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, pgdval;
+ p4d_t *p4d, p4dval;
+ pud_t pudval;
+ int seq, 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;
+
+ vma = get_vma(mm, address);
+ if (!vma)
+ return ret;
+
+ seq = raw_read_seqcount(&vma->vm_sequence); /* rmb <-> seqlock,vma_rb_erase() */
+ if (seq & 1)
+ goto out_put;
+
+ /*
+ * 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 out_put;
+
+ /*
+ * __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 out_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))
+ goto out_put;
+
+ 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 out_put;
+
+ if (address < READ_ONCE(vma->vm_start)
+ || READ_ONCE(vma->vm_end) <= address)
+ goto out_put;
+
+ if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
+ flags & FAULT_FLAG_INSTRUCTION,
+ flags & FAULT_FLAG_REMOTE)) {
+ ret = VM_FAULT_SIGSEGV;
+ goto out_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;
+ goto out_put;
+ }
+ } else if (unlikely(!(vmf.vma_flags & (VM_READ|VM_EXEC|VM_WRITE)))) {
+ ret = VM_FAULT_SIGSEGV;
+ goto out_put;
+ }
+
+#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 out_put;
+#endif
+
+ /*
+ * Do a speculative lookup of the PTE entry.
+ */
+ local_irq_disable();
+ pgd = pgd_offset(mm, address);
+ pgdval = READ_ONCE(*pgd);
+ if (pgd_none(pgdval) || unlikely(pgd_bad(pgdval)))
+ goto out_walk;
+
+ p4d = p4d_offset(pgd, address);
+ p4dval = READ_ONCE(*p4d);
+ if (p4d_none(p4dval) || unlikely(p4d_bad(p4dval)))
+ goto out_walk;
+
+ vmf.pud = pud_offset(p4d, address);
+ pudval = READ_ONCE(*vmf.pud);
+ if (pud_none(pudval) || unlikely(pud_bad(pudval)))
+ goto out_walk;
+
+ /* Huge pages at PUD level are not supported. */
+ if (unlikely(pud_trans_huge(pudval)))
+ goto out_walk;
+
+ vmf.pmd = pmd_offset(vmf.pud, address);
+ vmf.orig_pmd = READ_ONCE(*vmf.pmd);
+ /*
+ * pmd_none could mean that a hugepage collapse is in progress
+ * in our back as collapse_huge_page() mark it before
+ * invalidating the pte (which is done once the IPI is catched
+ * by all CPU and we have interrupt disabled).
+ * For this reason we cannot handle THP in a speculative way since we
+ * can't safely indentify an in progress collapse operation done in our
+ * back on that PMD.
+ * Regarding the order of the following checks, see comment in
+ * 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)))
+ 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.
+ */
+
+ 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();
+
+ /*
+ * 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 out_put;
+
+ mem_cgroup_oom_enable();
+ ret = handle_pte_fault(&vmf);
+ mem_cgroup_oom_disable();
+
+ put_vma(vma);
+
+ /*
+ * The task may have entered a memcg OOM situation but
+ * if the allocation error was handled gracefully (no
+ * VM_FAULT_OOM), there is no need to kill anything.
+ * Just clean up the OOM state peacefully.
+ */
+ if (task_in_memcg_oom(current) && !(ret & VM_FAULT_OOM))
+ mem_cgroup_oom_synchronize(false);
+ return ret;
+
+out_walk:
+ local_irq_enable();
+out_put:
+ put_vma(vma);
+ return ret;
+}
+#endif /* CONFIG_SPF */
+
/*
* By the time we get here, we already hold the mm semaphore
*
--
2.7.4
^ permalink raw reply related
* [PATCH v6 18/24] mm: Try spin lock in speculative path
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 96720cc7ca74..83640079d407 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2472,7 +2472,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);
@@ -2526,8 +2527,20 @@ static bool pte_map_lock(struct vm_fault *vmf)
if (!pmd_same(pmdval, vmf->orig_pmd))
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
^ permalink raw reply related
* [PATCH v6 19/24] mm: Adding speculative page fault failure trace events
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 | 62 ++++++++++++++++++++++------
2 files changed, 136 insertions(+), 13 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..1d793f8c739b
--- /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:%lx-%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_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)
+);
+
+DEFINE_EVENT(spf, spf_pmd_changed,
+
+ 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 83640079d407..6ccb1f45473a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -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
@@ -2460,23 +2463,30 @@ 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;
+ }
/*
* We check if the pmd value is still the same to ensure that there
* is a huge collapse operation in progress in our back.
*/
pmdval = READ_ONCE(*vmf->pmd);
- if (!pmd_same(pmdval, vmf->orig_pmd))
+ if (!pmd_same(pmdval, vmf->orig_pmd)) {
+ trace_spf_pmd_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;
}
@@ -2516,16 +2526,20 @@ 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;
+ }
/*
* We check if the pmd value is still the same to ensure that there
* is a huge collapse operation in progress in our back.
*/
pmdval = READ_ONCE(*vmf->pmd);
- if (!pmd_same(pmdval, vmf->orig_pmd))
+ if (!pmd_same(pmdval, vmf->orig_pmd)) {
+ trace_spf_pmd_changed(_RET_IP_, vmf->vma, vmf->address);
goto out;
+ }
/*
* Same as pte_offset_map_lock() except that we call
@@ -2538,11 +2552,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;
}
@@ -4297,47 +4313,60 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
return ret;
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);
goto out_put;
+ }
/*
* 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 out_put;
+ }
/*
* __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 out_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);
goto out_put;
+ }
- 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 out_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);
goto out_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;
goto out_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;
goto out_put;
}
} 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 out_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);
goto out_put;
+ }
#endif
/*
@@ -4435,8 +4468,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 out_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,
return ret;
out_walk:
+ trace_spf_vma_notsup(_RET_IP_, vma, address);
local_irq_enable();
out_put:
put_vma(vma);
--
2.7.4
^ permalink raw reply related
* [PATCH v6 20/24] perf: Add a speculative page fault sw event
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 769533696483..06c7fdb14f89 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -112,6 +112,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
^ permalink raw reply related
* [PATCH v6 21/24] perf tools: Add support for the SPF perf event
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 769533696483..06c7fdb14f89 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -112,6 +112,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 a4d256ea0dc4..9493d7b0f9b7 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -438,6 +438,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 170316795a18..e75de3c3ffbb 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -137,6 +137,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 655ecff636a8..5d6782426b30 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -308,6 +308,7 @@ emulation-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EM
dummy { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
duration_time { 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 b1e999bd21ef..100507d632fa 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1142,6 +1142,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
^ permalink raw reply related
* [PATCH v6 22/24] mm: Speculative page fault handler return VMA
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 <ldufour@linux.vnet.ibm.com>
---
include/linux/mm.h | 5 +-
mm/memory.c | 136 +++++++++++++++++++++++++++++++++--------------------
2 files changed, 88 insertions(+), 53 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 4d8a7621da8a..02da17792f0d 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -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);
+ unsigned long address, unsigned int flags,
+ struct vm_area_struct **vma);
+extern bool can_reuse_spf_vma(struct vm_area_struct *vma,
+ unsigned long address);
#endif /* CONFIG_SPF */
extern int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
unsigned long address, unsigned int fault_flags,
diff --git a/mm/memory.c b/mm/memory.c
index 6ccb1f45473a..e1f172ac2c90 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -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() adds some processing which should be done while
* hodling the mmap_sem.
*/
+
+/*
+ * Tries to handle the page fault in a speculative way, without grabbing the
+ * mmap_sem.
+ * When VM_FAULT_RETRY is returned, the vma pointer is valid and this vma must
+ * be checked later when the mmap_sem has been grabbed by calling
+ * can_reuse_spf_vma().
+ * This is needed as the returned vma is kept in memory until the call to
+ * can_reuse_spf_vma() is made.
+ */
int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
- unsigned int flags)
+ unsigned int flags, struct vm_area_struct **vma)
{
struct vm_fault vmf = {
.address = address,
@@ -4299,7 +4308,6 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
p4d_t *p4d, p4dval;
pud_t pudval;
int seq, ret = VM_FAULT_RETRY;
- struct vm_area_struct *vma;
#ifdef CONFIG_NUMA
struct mempolicy *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)
return ret;
+ 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);
- goto out_put;
+ trace_spf_vma_changed(_RET_IP_, vmf.vma, address);
+ return ret;
}
/*
@@ -4323,9 +4333,9 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
* with the VMA.
* This include huge page from hugetlbfs.
*/
- if (vma->vm_ops) {
- trace_spf_vma_notsup(_RET_IP_, vma, address);
- goto out_put;
+ if (vmf.vma->vm_ops) {
+ trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+ return ret;
}
/*
@@ -4333,18 +4343,18 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
* because vm_next and vm_prev must be safe. This can't be guaranteed
* in the speculative path.
*/
- if (unlikely(!vma->anon_vma)) {
- trace_spf_vma_notsup(_RET_IP_, vma, address);
- goto out_put;
+ if (unlikely(!vmf.vma->anon_vma)) {
+ trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+ return ret;
}
- 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);
- goto out_put;
+ trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+ return ret;
}
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,
* boundaries but we want to trace it as not supported instead
* of changed.
*/
- trace_spf_vma_notsup(_RET_IP_, vma, address);
- goto out_put;
+ trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+ return ret;
}
- if (address < READ_ONCE(vma->vm_start)
- || READ_ONCE(vma->vm_end) <= address) {
- trace_spf_vma_changed(_RET_IP_, vma, address);
- goto out_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);
+ return ret;
}
- 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;
- goto out_put;
- }
+ flags & FAULT_FLAG_REMOTE))
+ goto out_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;
- goto out_put;
- }
- } 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 out_put;
- }
+ if (unlikely(!(vmf.vma_flags & VM_WRITE)))
+ goto out_segv;
+ } else if (unlikely(!(vmf.vma_flags & (VM_READ|VM_EXEC|VM_WRITE))))
+ goto out_segv;
#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);
+ 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);
- goto out_put;
+ trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
+ return ret;
}
#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,
* 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)) {
- trace_spf_vma_changed(_RET_IP_, vma, address);
- goto out_put;
+ if (read_seqcount_retry(&vmf.vma->vm_sequence, seq)) {
+ trace_spf_vma_changed(_RET_IP_, vmf.vma, address);
+ return ret;
}
mem_cgroup_oom_enable();
ret = handle_pte_fault(&vmf);
mem_cgroup_oom_disable();
- put_vma(vma);
+ /*
+ * If there is no need to retry, don't return the vma to the caller.
+ */
+ if (!(ret & VM_FAULT_RETRY)) {
+ put_vma(vmf.vma);
+ *vma = NULL;
+ }
/*
* The task may have entered a memcg OOM situation but
@@ -4490,9 +4496,35 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
return ret;
out_walk:
- trace_spf_vma_notsup(_RET_IP_, vma, address);
+ trace_spf_vma_notsup(_RET_IP_, vmf.vma, address);
local_irq_enable();
-out_put:
+ return ret;
+
+out_segv:
+ trace_spf_vma_access(_RET_IP_, vmf.vma, address);
+ /*
+ * We don't return VM_FAULT_RETRY so the caller is not expected to
+ * retrieve the fetched VMA.
+ */
+ put_vma(vmf.vma);
+ *vma = NULL;
+ return VM_FAULT_SIGSEGV;
+}
+
+/*
+ * This is used to know if the vma fetch in the speculative page fault handler
+ * is still valid when trying the regular fault path while holding the
+ * mmap_sem.
+ * The call to put_vma(vma) must be made after checking the vma's fields, as
+ * the vma may be freed by put_vma(). In such a case it is expected that false
+ * is returned.
+ */
+bool can_reuse_spf_vma(struct vm_area_struct *vma, unsigned long address)
+{
+ bool ret;
+
+ ret = !RB_EMPTY_NODE(&vma->vm_rb) &&
+ vma->vm_start <= address && address < vma->vm_end;
put_vma(vma);
return ret;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v6 23/24] x86/mm: Add speculative pagefault handling
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 <ldufour@linux.vnet.ibm.com>
---
arch/x86/mm/fault.c | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 06fe3d51d385..8db69a116521 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1242,6 +1242,9 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
unsigned long address)
{
struct vm_area_struct *vma;
+#ifdef CONFIG_SPF
+ struct vm_area_struct *spf_vma = NULL;
+#endif
struct task_struct *tsk;
struct mm_struct *mm;
int fault, major = 0;
@@ -1339,6 +1342,27 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
if (error_code & X86_PF_INSTR)
flags |= FAULT_FLAG_INSTRUCTION;
+#ifdef CONFIG_SPF
+ if ((error_code & X86_PF_USER) && (atomic_read(&mm->mm_users) > 1)) {
+ fault = handle_speculative_fault(mm, address, flags,
+ &spf_vma);
+
+ if (!(fault & VM_FAULT_RETRY)) {
+ if (!(fault & VM_FAULT_ERROR)) {
+ perf_sw_event(PERF_COUNT_SW_SPF, 1,
+ regs, address);
+ goto done;
+ }
+ /*
+ * In case of error we need the pkey value, but
+ * can't get it from the spf_vma as it is only returned
+ * when VM_FAULT_RETRY is returned. So we have to
+ * retry the page fault with the mmap_sem grabbed.
+ */
+ }
+ }
+#endif /* CONFIG_SPF */
+
/*
* When running in the kernel we expect faults to occur only to
* addresses in user space. All other faults represent errors in
@@ -1372,7 +1396,16 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
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)) {
bad_area(regs, error_code, address);
return;
@@ -1458,6 +1491,9 @@ __do_page_fault(struct pt_regs *regs, unsigned long error_code,
return;
}
+#ifdef CONFIG_SPF
+done:
+#endif
/*
* Major/minor page fault accounting. If any of the events
* returned VM_FAULT_MAJOR, we account it as a major fault.
--
2.7.4
^ permalink raw reply related
* [PATCH v6 24/24] powerpc/mm: Add speculative page fault
From: Laurent Dufour @ 2018-01-12 17:26 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, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-1-git-send-email-ldufour@linux.vnet.ibm.com>
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 <ldufour@linux.vnet.ibm.com>
---
arch/powerpc/mm/fault.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 6e1e39035380..67b4c6aa7975 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -384,6 +384,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
unsigned long error_code)
{
struct vm_area_struct * vma;
+#ifdef CONFIG_SPF
+ struct vm_area_struct *spf_vma = NULL;
+#endif
struct mm_struct *mm = current->mm;
unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
int is_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);
+ goto done;
+ }
+ }
+#endif /* CONFIG_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
@@ -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))
return bad_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:
+#endif
if (unlikely(fault & VM_FAULT_ERROR))
return mm_fault_error(regs, address, fault);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 00/16] ASoC: fsl_ssi: Clean up - program flow level
From: Caleb Crome @ 2018-01-12 17:35 UTC (permalink / raw)
To: Nicolin Chen
Cc: Timur Tabi, Mark Brown, mail, linux-kernel, linuxppc-dev,
alsa-devel, Liam Girdwood, Fabio Estevam, Arnaud Mouiche, lukma,
Sascha Hauer
In-Reply-To: <1515652995-15996-1-git-send-email-nicoleotsuka@gmail.com>
On Wed, Jan 10, 2018 at 10:42 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> ==Change log==
> v2
> * Reworked the series by taking suggestions from Maciej
> + Added PATCH-01 to keep all ssi->i2s_net updated
> + Replaced bool tx with bool dir in PATCH-03 and PATCH-06
> + Moved all initial register configurations from dai probe() to
> platform probe() so as to let AC97 CODEC successfully probe.
> * Added Tested-by from Caleb for TDM test cases.
>
> ==Background==
> The fsl_ssi driver was designed for PPC originally and then it has
> been updated to support different modes for i.MX Series, including
> SDMA, I2S Master mode, AC97 and older i.MXs with FIQ, by different
> contributors for different use cases in different coding styles.
>
> Additionally, in order to fix/work-around hardware bugs and design
> flaws, the driver made a lot of compromise so now its program flow
> looks very complicated and it's getting hard to maintain or update.
>
> So I am going to clean up the driver on both coding style level and
> program flow level.
>
> ==Introduction==
> This series of patches is the second set to clean up fsl_ssi driver
> in the program flow level. Any patch here may impact a fundamental
> test case like playback or record.
>
> ==Verification==
> This series of patches require fully tested. I have done such tests
> on i.MX6SoloX with WM8962 using imx_v6_v7_defconfig as:
> - Playback via I2S Master and Slave mode
> - Record via I2S Master and Slave mode
> - Simultaneous playback and record via I2S Master and Slave mode
> - Background playback with foreground record (starting at different
> time) via I2S Master and Slave mode
> - Background record with foreground playback (starting at different
> time) via I2S Master and Slave mode
> * All tests above by hacking offline_config to true in imx51.
>
> Caleb has tested v1 with TDM lookback tests on i.MX6.
>
> Example of uncovered tests: AC97, PowerPC and FIQ.
>
> Nicolin Chen (16):
> ASoC: fsl_ssi: Keep ssi->i2s_net updated
> ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
> ASoC: fsl_ssi: Maintain a mask of active streams
> ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
> ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
> ASoC: fsl_ssi: Clean up helper functions of trigger()
> ASoC: fsl_ssi: Add DAIFMT define for AC97
> ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
> ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
> ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
> ASoC: fsl_ssi: Move one-time configurations to probe()
> ASoC: fsl_ssi: Setup AC97 in fsl_ssi_hw_init()
> ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
> ASoC: fsl_ssi: Remove cpu_dai_drv from fsl_ssi structure
> ASoC: fsl_ssi: Move DT related code to a separate probe()
> ASoC: fsl_ssi: Use ssi->streams instead of reading register
>
> sound/soc/fsl/fsl_ssi.c | 740 ++++++++++++++++++++++++------------------------
> 1 file changed, 369 insertions(+), 371 deletions(-)
>
> --
> 2.7.4
>
Tested again, just to be sure... All looks good.
Tested-by: Caleb Crome <caleb@crome.org>
^ permalink raw reply
* Re: [PATCH v6 18/24] mm: Try spin lock in speculative path
From: Matthew Wilcox @ 2018-01-12 18:18 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack, benh, mpe,
paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, linux-kernel, linux-mm, haren,
khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-19-git-send-email-ldufour@linux.vnet.ibm.com>
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?
^ permalink raw reply
* Re: [PATCH v6 16/24] mm: Protect mm_rb tree with a rwlock
From: Matthew Wilcox @ 2018-01-12 18:48 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack, benh, mpe,
paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, linux-kernel, linux-mm, haren,
khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-17-git-send-email-ldufour@linux.vnet.ibm.com>
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.
^ permalink raw reply
* Re: [PATCH v6 01/24] x86/mm: Define CONFIG_SPF
From: Thomas Gleixner @ 2018-01-12 18:57 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, linux-kernel, linux-mm, haren,
khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-2-git-send-email-ldufour@linux.vnet.ibm.com>
On Fri, 12 Jan 2018, Laurent Dufour wrote:
> Introduce CONFIG_SPF which turns on the Speculative Page Fault handler when
> building for 64bits with SMP.
>
> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> ---
> arch/x86/Kconfig | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index a317d5594b6a..d74353b85aaf 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2882,6 +2882,10 @@ config X86_DMA_REMAP
> config HAVE_GENERIC_GUP
> def_bool y
>
> +config SPF
> + def_bool y
> + depends on X86_64 && SMP
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
^ permalink raw reply
* Re: [PATCH v6 22/24] mm: Speculative page fault handler return VMA
From: Matthew Wilcox @ 2018-01-12 19:02 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack, benh, mpe,
paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, linux-kernel, linux-mm, haren,
khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1515777968-867-23-git-send-email-ldufour@linux.vnet.ibm.com>
On Fri, Jan 12, 2018 at 06:26:06PM +0100, Laurent Dufour wrote:
> @@ -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 ;-)
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Simplify _PAGE_RO handling in page table dump
From: christophe leroy @ 2018-01-12 19:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Scott Wood
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20170509141626.F30E767954@pc13941vm.idsi0.si.c-s.fr>
Le 09/05/2017 à 16:16, Christophe Leroy a écrit :
> Commit fd893fe56a130 ("powerpc/mm: Fix missing page attributes in
> page table dump") added support of _PAGE_RO attribute.
>
> This patch makes it more simple
Superseeded by https://patchwork.ozlabs.org/patch/859896/
Christophe
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/mm/dump_linuxpagetables.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
> index d659345a98d6..eeef51107cff 100644
> --- a/arch/powerpc/mm/dump_linuxpagetables.c
> +++ b/arch/powerpc/mm/dump_linuxpagetables.c
> @@ -121,13 +121,8 @@ static const struct flag_info flag_array[] = {
> .set = "user",
> .clear = " ",
> }, {
> -#if _PAGE_RO == 0
> - .mask = _PAGE_RW,
> + .mask = _PAGE_RW | _PAGE_RO,
> .val = _PAGE_RW,
> -#else
> - .mask = _PAGE_RO,
> - .val = 0,
> -#endif
> .set = "rw",
> .clear = "ro",
> }, {
>
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
^ permalink raw reply
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Arnd Bergmann @ 2018-01-12 19:43 UTC (permalink / raw)
To: Segher Boessenkool
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu, kbuild-all
In-Reply-To: <20180112163946.GE21977@gate.crashing.org>
On Fri, Jan 12, 2018 at 5:39 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>> or why the aes_generic implementation needs this on
>> powerpc when built with 'gcc -Os'. FWIW, the -Os change was needed
>> to work around a possible kernel stack overflow that can happen with
>> gcc-7.2, see https://patchwork.kernel.org/patch/10143607/ and
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356
>
> The _x versions are smaller but slower; that's why they are used with -Os.
> Apparently nothing else was built with -Os (and the other needed flags)
> before.
Ah, that explains it, the definition is in arch/powerpc/lib/crtsavres.S,
but inside of #ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE.
We could theoretically work around it by turning that into
"#if defined(CONFIG_CC_OPTIMIZE_FOR_SIZE) ||
defined(CONFIG_CRYPTO_AES)", but that seems rather ugly.
My earlier patch already tried to be more specific, turning very
specific optimizations off rather than moving from -O2 to -Os,
but that turned out to lead to significantly worse performance,
where -Os improved performance slightly. Is there a way
to ask powerpc compilers to use mostly -Os but not the
specific thing that makes it link to _restgpr_31_x?
Arnd
^ permalink raw reply
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Segher Boessenkool @ 2018-01-12 20:41 UTC (permalink / raw)
To: Arnd Bergmann
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu, kbuild-all
In-Reply-To: <CAK8P3a39=PVpwiSk_3CjRnYBawhXqMatHKSsHCd2KHCgR+Zmyg@mail.gmail.com>
On Fri, Jan 12, 2018 at 08:43:21PM +0100, Arnd Bergmann wrote:
> On Fri, Jan 12, 2018 at 5:39 PM, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
>
> >> or why the aes_generic implementation needs this on
> >> powerpc when built with 'gcc -Os'. FWIW, the -Os change was needed
> >> to work around a possible kernel stack overflow that can happen with
> >> gcc-7.2, see https://patchwork.kernel.org/patch/10143607/ and
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356
> >
> > The _x versions are smaller but slower; that's why they are used with -Os.
> > Apparently nothing else was built with -Os (and the other needed flags)
> > before.
>
> Ah, that explains it, the definition is in arch/powerpc/lib/crtsavres.S,
> but inside of #ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE.
Ah ok. Right.
> We could theoretically work around it by turning that into
> "#if defined(CONFIG_CC_OPTIMIZE_FOR_SIZE) ||
> defined(CONFIG_CRYPTO_AES)", but that seems rather ugly.
>
> My earlier patch already tried to be more specific, turning very
> specific optimizations off rather than moving from -O2 to -Os,
> but that turned out to lead to significantly worse performance,
> where -Os improved performance slightly. Is there a way
> to ask powerpc compilers to use mostly -Os but not the
> specific thing that makes it link to _restgpr_31_x?
There is no such thing, sorry. Would be very hard to implement, and
older compilers will never get it, so it won't help you anyway :-(
Maybe for now just enable it in crtsavres.S always, with a comment?
That -Os workaround is hopefully not going to live long either...
Segher
^ permalink raw reply
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Arnd Bergmann @ 2018-01-12 21:29 UTC (permalink / raw)
To: Segher Boessenkool
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu, kbuild-all
In-Reply-To: <20180112204154.GM21977@gate.crashing.org>
On Fri, Jan 12, 2018 at 9:41 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Fri, Jan 12, 2018 at 08:43:21PM +0100, Arnd Bergmann wrote:
>> On Fri, Jan 12, 2018 at 5:39 PM, Segher Boessenkool
>> We could theoretically work around it by turning that into
>> "#if defined(CONFIG_CC_OPTIMIZE_FOR_SIZE) ||
>> defined(CONFIG_CRYPTO_AES)", but that seems rather ugly.
>>
>> My earlier patch already tried to be more specific, turning very
>> specific optimizations off rather than moving from -O2 to -Os,
>> but that turned out to lead to significantly worse performance,
>> where -Os improved performance slightly. Is there a way
>> to ask powerpc compilers to use mostly -Os but not the
>> specific thing that makes it link to _restgpr_31_x?
>
> There is no such thing, sorry. Would be very hard to implement, and
> older compilers will never get it, so it won't help you anyway :-(
We use -Os only for gcc-7.1 and higher, where it produces faster
code for AES and avoids running into
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356
> Maybe for now just enable it in crtsavres.S always, with a comment?
> That -Os workaround is hopefully not going to live long either...
It depends on whether or how soon someone comes up with a
better fix for PR83356.
gcc-8.0.0 is currently not affected by it, so we could limit the
workaround (and the hack in crtsavres.S) to gcc-7-only.
Arnd
^ permalink raw reply
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Segher Boessenkool @ 2018-01-12 21:41 UTC (permalink / raw)
To: Arnd Bergmann
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu
In-Reply-To: <CAK8P3a2YVVseBFNoDbwOtqR3-JHFuZ-WZD6-PzztNTO_X_soaw@mail.gmail.com>
On Fri, Jan 12, 2018 at 10:29:01PM +0100, Arnd Bergmann wrote:
> On Fri, Jan 12, 2018 at 9:41 PM, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Fri, Jan 12, 2018 at 08:43:21PM +0100, Arnd Bergmann wrote:
> >> On Fri, Jan 12, 2018 at 5:39 PM, Segher Boessenkool
>
> >> We could theoretically work around it by turning that into
> >> "#if defined(CONFIG_CC_OPTIMIZE_FOR_SIZE) ||
> >> defined(CONFIG_CRYPTO_AES)", but that seems rather ugly.
> >>
> >> My earlier patch already tried to be more specific, turning very
> >> specific optimizations off rather than moving from -O2 to -Os,
> >> but that turned out to lead to significantly worse performance,
> >> where -Os improved performance slightly. Is there a way
> >> to ask powerpc compilers to use mostly -Os but not the
> >> specific thing that makes it link to _restgpr_31_x?
> >
> > There is no such thing, sorry. Would be very hard to implement, and
> > older compilers will never get it, so it won't help you anyway :-(
>
> We use -Os only for gcc-7.1 and higher, where it produces faster
> code for AES and avoids running into
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356
>
> > Maybe for now just enable it in crtsavres.S always, with a comment?
> > That -Os workaround is hopefully not going to live long either...
>
> It depends on whether or how soon someone comes up with a
> better fix for PR83356.
> gcc-8.0.0 is currently not affected by it, so we could limit the
> workaround (and the hack in crtsavres.S) to gcc-7-only.
I guess you could enable the _x routines whenever you use ubsan? Ubsan
will cause much bigger code growth than the handful of insns in those
routines?
Segher
^ permalink raw reply
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Arnd Bergmann @ 2018-01-12 21:45 UTC (permalink / raw)
To: Segher Boessenkool
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu
In-Reply-To: <20180112214111.GQ21977@gate.crashing.org>
On Fri, Jan 12, 2018 at 10:41 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Fri, Jan 12, 2018 at 10:29:01PM +0100, Arnd Bergmann wrote:
>> On Fri, Jan 12, 2018 at 9:41 PM, Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>> > On Fri, Jan 12, 2018 at 08:43:21PM +0100, Arnd Bergmann wrote:
>> >> On Fri, Jan 12, 2018 at 5:39 PM, Segher Boessenkool
>>
>> >> We could theoretically work around it by turning that into
>> >> "#if defined(CONFIG_CC_OPTIMIZE_FOR_SIZE) ||
>> >> defined(CONFIG_CRYPTO_AES)", but that seems rather ugly.
>> >>
>> >> My earlier patch already tried to be more specific, turning very
>> >> specific optimizations off rather than moving from -O2 to -Os,
>> >> but that turned out to lead to significantly worse performance,
>> >> where -Os improved performance slightly. Is there a way
>> >> to ask powerpc compilers to use mostly -Os but not the
>> >> specific thing that makes it link to _restgpr_31_x?
>> >
>> > There is no such thing, sorry. Would be very hard to implement, and
>> > older compilers will never get it, so it won't help you anyway :-(
>>
>> We use -Os only for gcc-7.1 and higher, where it produces faster
>> code for AES and avoids running into
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356
>>
>> > Maybe for now just enable it in crtsavres.S always, with a comment?
>> > That -Os workaround is hopefully not going to live long either...
>>
>> It depends on whether or how soon someone comes up with a
>> better fix for PR83356.
>> gcc-8.0.0 is currently not affected by it, so we could limit the
>> workaround (and the hack in crtsavres.S) to gcc-7-only.
>
> I guess you could enable the _x routines whenever you use ubsan? Ubsan
> will cause much bigger code growth than the handful of insns in those
> routines?
Right, that could work, too. My patch that Herbert merged intentionally
used -Os also for non-UBSAN builds because it turned out to
be much faster (see gcc PR83651), but we could revert that back
to the default and only use the -Os for UBSAN, essentially
addressing only PR83356 but not PR83651.
Arnd
^ permalink raw reply
* Re: [cryptodev:master 130/134] aes_generic.c:undefined reference to `_restgpr_31_x'
From: Segher Boessenkool @ 2018-01-12 22:10 UTC (permalink / raw)
To: Arnd Bergmann
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
kbuild test robot, linuxppc-dev, Herbert Xu
In-Reply-To: <CAK8P3a3XKwrVpXH9=8n1=qAKtqHE+1sh6OhaJ_39GZcWSP-j=Q@mail.gmail.com>
On Fri, Jan 12, 2018 at 10:45:31PM +0100, Arnd Bergmann wrote:
> > I guess you could enable the _x routines whenever you use ubsan? Ubsan
> > will cause much bigger code growth than the handful of insns in those
> > routines?
>
> Right, that could work, too. My patch that Herbert merged intentionally
> used -Os also for non-UBSAN builds because it turned out to
> be much faster (see gcc PR83651),
"Much"?
-Os is *slower* with 8.0, 5% faster with 7.2, 4% faster with 7.1,
slower with 7.0 and 6.3. Your numbers, #c1.
Anf this is the generic code of course, which is slow anyway (not to
mention insecure).
> but we could revert that back
> to the default and only use the -Os for UBSAN, essentially
> addressing only PR83356 but not PR83651.
Segher
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox