| Message ID | 20190416134522.17540-13-ldufour@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Speculative page faults | expand |
On Tue, Apr 16, 2019 at 03:45:03PM +0200, Laurent Dufour wrote: > The speculative page fault handler must be protected against anon_vma > changes. This is because page_add_new_anon_rmap() is called during the > speculative path. > > In addition, don't try speculative page fault if the VMA don't have an > anon_vma structure allocated because its allocation should be > protected by the mmap_sem. > > In __vma_adjust() when importer->anon_vma is set, there is no need to > protect against speculative page faults since speculative page fault > is aborted if the vma->anon_vma is not set. > > When calling page_add_new_anon_rmap() vma->anon_vma is necessarily > valid since we checked for it when locking the pte and the anon_vma is > removed once the pte is unlocked. So even if the speculative page > fault handler is running concurrently with do_unmap(), as the pte is > locked in unmap_region() - through unmap_vmas() - and the anon_vma > unlinked later, because we check for the vma sequence counter which is > updated in unmap_page_range() before locking the pte, and then in > free_pgtables() so when locking the pte the change will be detected. > > Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com> Reviewed-by: Jérôme Glisse <jglisse@redhat.com> > --- > mm/memory.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/memory.c b/mm/memory.c > index 423fa8ea0569..2cf7b6185daa 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -377,7 +377,9 @@ void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma, > * Hide vma from rmap and truncate_pagecache before freeing > * pgtables > */ > + vm_write_begin(vma); > unlink_anon_vmas(vma); > + vm_write_end(vma); > unlink_file_vma(vma); > > if (is_vm_hugetlb_page(vma)) { > @@ -391,7 +393,9 @@ void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma, > && !is_vm_hugetlb_page(next)) { > vma = next; > next = vma->vm_next; > + vm_write_begin(vma); > unlink_anon_vmas(vma); > + vm_write_end(vma); > unlink_file_vma(vma); > } > free_pgd_range(tlb, addr, vma->vm_end, > -- > 2.21.0 >
diff --git a/mm/memory.c b/mm/memory.c index 423fa8ea0569..2cf7b6185daa 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -377,7 +377,9 @@ void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma, * Hide vma from rmap and truncate_pagecache before freeing * pgtables */ + vm_write_begin(vma); unlink_anon_vmas(vma); + vm_write_end(vma); unlink_file_vma(vma); if (is_vm_hugetlb_page(vma)) { @@ -391,7 +393,9 @@ void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma, && !is_vm_hugetlb_page(next)) { vma = next; next = vma->vm_next; + vm_write_begin(vma); unlink_anon_vmas(vma); + vm_write_end(vma); unlink_file_vma(vma); } free_pgd_range(tlb, addr, vma->vm_end,
The speculative page fault handler must be protected against anon_vma changes. This is because page_add_new_anon_rmap() is called during the speculative path. In addition, don't try speculative page fault if the VMA don't have an anon_vma structure allocated because its allocation should be protected by the mmap_sem. In __vma_adjust() when importer->anon_vma is set, there is no need to protect against speculative page faults since speculative page fault is aborted if the vma->anon_vma is not set. When calling page_add_new_anon_rmap() vma->anon_vma is necessarily valid since we checked for it when locking the pte and the anon_vma is removed once the pte is unlocked. So even if the speculative page fault handler is running concurrently with do_unmap(), as the pte is locked in unmap_region() - through unmap_vmas() - and the anon_vma unlinked later, because we check for the vma sequence counter which is updated in unmap_page_range() before locking the pte, and then in free_pgtables() so when locking the pte the change will be detected. Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com> --- mm/memory.c | 4 ++++ 1 file changed, 4 insertions(+)