mm: Page fault enhancements
Subsystems (31)
Peter Xu
reviewerv2 cover mm: Page fault enhancements
v2:
- resent previous version, rebase only
This series is split out of userfaultfd-wp series to only cover the general page fault changes, since it seems to make sense itself.
Basically it does two things:
(a) Allows the page fault handlers to be more interactive on not only SIGKILL, but also the rest of userspace signals (especially for user-mode faults), and,
(b) Allows the page fault retry (VM_FAULT_RETRY) to happen for more than once.
I'm keeping the CC list as in uffd-wp v5, hopefully I'm not sending too much spams...
And, instead of writting again the cover letter, I'm just copy-pasting my previous link here which has more details on why we do this:
https://patchwork.kernel.org/cover/10691991/
The major change from that latest version should be that we introduced a new page fault flag FAULT_FLAG_INTERRUPTIBLE as suggested by Linus
[1] to represents that we would like the fault handler to respond to non-fatal signals. Also, we're more careful now on when to do the immediate return of the page fault for such signals. For example, now we'll only check against signal_pending() for user-mode page faults and we keep the kernel-mode page fault patch untouched for it. More information can be found in separate patches.
The patchset is only lightly tested on x86.
All comments are greatly welcomed. Thanks,
[1] https://lkml.org/lkml/2019/6/25/1382
Peter Xu (7):
mm/gup: Rename "nonblocking" to "locked" where proper
mm: Introduce FAULT_FLAG_DEFAULT
mm: Introduce FAULT_FLAG_INTERRUPTIBLE
mm: Return faster for non-fatal signals in user mode faults
userfaultfd: Don't retake mmap_sem to emulate NOPAGE
mm: Allow VM_FAULT_RETRY for multiple times mm/gup: Allow VM_FAULT_RETRY for multiple times
arch/alpha/mm/fault.c | 7 +-- arch/arc/mm/fault.c | 8 +++- arch/arm/mm/fault.c | 14 +++--- arch/arm64/mm/fault.c | 16 +++---- arch/hexagon/mm/vm_fault.c | 6 +-- arch/ia64/mm/fault.c | 6 +-- arch/m68k/mm/fault.c | 10 ++-- arch/microblaze/mm/fault.c | 6 +-- arch/mips/mm/fault.c | 6 +-- arch/nds32/mm/fault.c | 12 ++--- arch/nios2/mm/fault.c | 8 ++-- arch/openrisc/mm/fault.c | 6 +-- arch/parisc/mm/fault.c | 9 ++-- arch/powerpc/mm/fault.c | 10 ++-- arch/riscv/mm/fault.c | 12 ++--- arch/s390/mm/fault.c | 11 ++--- arch/sh/mm/fault.c | 7 ++- arch/sparc/mm/fault_32.c | 5 +- arch/sparc/mm/fault_64.c | 6 +-- arch/um/kernel/trap.c | 7 +-- arch/unicore32/mm/fault.c | 11 ++--- arch/x86/mm/fault.c | 6 +-- arch/xtensa/mm/fault.c | 6 +-- drivers/gpu/drm/ttm/ttm_bo_vm.c | 12 +++-- fs/userfaultfd.c | 28 +----------- include/linux/mm.h | 81 +++++++++++++++++++++++++++++---- include/linux/sched/signal.h | 12 +++++ mm/filemap.c | 2 +- mm/gup.c | 61 ++++++++++++++----------- mm/hugetlb.c | 14 +++--- mm/shmem.c | 2 +- 31 files changed, 227 insertions(+), 180 deletions(-)
Signature · 1 line
2.21.0
Peter Xu
reviewerPATCH v2 1/7 mm/gup: Rename "nonblocking" to "locked" where proper
There's plenty of places around __get_user_pages() that has a parameter "nonblocking" which does not really mean that "it won't block" (because it can really block) but instead it shows whether the mmap_sem is released by up_read() during the page fault handling mostly when VM_FAULT_RETRY is returned.
We have the correct naming in e.g. get_user_pages_locked() or get_user_pages_remote() as "locked", however there're still many places that are using the "nonblocking" as name.
Renaming the places to "locked" where proper to better suite the functionality of the variable. While at it, fixing up some of the comments accordingly.
--- mm/gup.c | 44 +++++++++++++++++++++----------------------- mm/hugetlb.c | 8 ++++---- 2 files changed, 25 insertions(+), 27 deletions(-)
mm/gup.c +21 -23
@@ -622,12 +622,12 @@ static int get_gate_page(struct mm_struct *mm, unsigned long address, }/* -* mmap_sem must be held on entry. If @nonblocking != NULL and -* *@flags does not include FOLL_NOWAIT, the mmap_sem may be released. -* If it is, *@nonblocking will be set to 0 and -EBUSY returned. +* mmap_sem must be held on entry. If @locked != NULL and *@flags +* does not include FOLL_NOWAIT, the mmap_sem may be released. If it +* is, *@locked will be set to 0 and -EBUSY returned. */ static int faultin_page (struct task_struct *tsk,struct vm_area_struct *vma, -unsigned long address,unsigned int *flags,int *nonblocking) +unsigned long address,unsigned int *flags,int *locked) {unsigned int fault_flags =0 ;vm_fault_t ret;
@@ -639,7 +639,7 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma, fault_flags |=FAULT_FLAG_WRITE ;if (*flags &FOLL_REMOTE ) fault_flags |=FAULT_FLAG_REMOTE ; -if (nonblocking) +if (locked) fault_flags |=FAULT_FLAG_ALLOW_RETRY ;if (*flags &FOLL_NOWAIT ) fault_flags |=FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_RETRY_NOWAIT ;
@@ -665,8 +665,8 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma, }if (ret &VM_FAULT_RETRY ) { -if (nonblocking && !(fault_flags &FAULT_FLAG_RETRY_NOWAIT )) - *nonblocking =0 ; +if (locked && !(fault_flags &FAULT_FLAG_RETRY_NOWAIT )) + *locked =0 ;return -EBUSY ; }
@@ -743,7 +743,7 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)* only intends to ensure the pages are faulted in. * @vmas: array of pointers to vmas corresponding to each page. * Or NULL if the caller does not require them. -* @nonblocking: whether waiting for disk IO or mmap_sem contention +* @locked: whether we're still with the mmap_sem held * * Returns number of pages pinned. This may be fewer than the number * requested. If nr_pages is 0 or negative, returns 0. If no pages
@@ -772,13 +772,11 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)* appropriate) must be called after the page is finished with, and * before put_page is called. * -* If @nonblocking != NULL, __get_user_pages will not wait for disk IO -* or mmap_sem contention, and if waiting is needed to pin all pages, -* *@nonblocking will be set to 0. Further, if @gup_flags does not -* include FOLL_NOWAIT, the mmap_sem will be released via up_read() in -* this case. +* If @locked != NULL, *@locked will be set to 0 when mmap_sem is +* released by an up_read(). That can happen if @gup_flags does not +* have FOLL_NOWAIT. * -* A caller using such a combination of @nonblocking and @gup_flags +* A caller using such a combination of @locked and @gup_flags * must therefore hold the mmap_sem for reading only, and recognize * when it's been released. Otherwise, it must be held for either * reading or writing and will not be released.
@@ -790,7 +788,7 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)static long __get_user_pages (struct task_struct *tsk,struct mm_struct *mm,unsigned long start,unsigned long nr_pages,unsigned int gup_flags,struct page **pages, -struct vm_area_struct **vmas,int *nonblocking) +struct vm_area_struct **vmas,int *locked) {long ret =0 , i =0 ;struct vm_area_struct *vma =NULL ;
@@ -834,7 +832,7 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,if (is_vm_hugetlb_page (vma)) { i =follow_hugetlb_page (mm, vma, pages, vmas, &start, &nr_pages, i, - gup_flags, nonblocking); + gup_flags, locked);continue ; } }
@@ -852,7 +850,7 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm, page =follow_page_mask (vma, start, foll_flags, &ctx);if (!page) { ret =faultin_page (tsk, vma, start, &foll_flags, - nonblocking); + locked);switch (ret) {case 0 : goto retry;
@@ -1178,7 +1176,7 @@ EXPORT_SYMBOL(get_user_pages_remote);* @vma: target vma * @start: start address * @end: end address -* @nonblocking: +* @locked: whether the mmap_sem is still held * * This takes care of mlocking the pages too if VM_LOCKED is set. *
@@ -1186,14 +1184,14 @@ EXPORT_SYMBOL(get_user_pages_remote);* * vma->vm_mm->mmap_sem must be held. * -* If @nonblocking is NULL, it may be held for read or write and will +* If @locked is NULL, it may be held for read or write and will * be unperturbed. * -* If @nonblocking is non-NULL, it must held for read only and may be -* released. If it's released, *@nonblocking will be set to 0. +* If @locked is non-NULL, it must held for read only and may be +* released. If it's released, *@locked will be set to 0. */ long populate_vma_page_range (struct vm_area_struct *vma, -unsigned long start,unsigned long end,int *nonblocking) +unsigned long start,unsigned long end,int *locked) {struct mm_struct *mm = vma->vm_mm;unsigned long nr_pages = (end - start) /PAGE_SIZE ;
@@ -1228,7 +1226,7 @@ long populate_vma_page_range(struct vm_area_struct *vma,* not result in a stack expansion that recurses back here. */ return __get_user_pages (current, mm, start, nr_pages, gup_flags, -NULL ,NULL , nonblocking); +NULL ,NULL , locked); }/*
mm/hugetlb.c +4 -4
@@ -4251,7 +4251,7 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,long follow_hugetlb_page (struct mm_struct *mm,struct vm_area_struct *vma,struct page **pages,struct vm_area_struct **vmas,unsigned long *position,unsigned long *nr_pages, -long i,unsigned int flags,int *nonblocking) +long i,unsigned int flags,int *locked) {unsigned long pfn_offset;unsigned long vaddr = *position;
@@ -4322,7 +4322,7 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,spin_unlock (ptl);if (flags &FOLL_WRITE ) fault_flags |=FAULT_FLAG_WRITE ; -if (nonblocking) +if (locked) fault_flags |=FAULT_FLAG_ALLOW_RETRY ;if (flags &FOLL_NOWAIT ) fault_flags |= FAULT_FLAG_ALLOW_RETRY |
@@ -4339,9 +4339,9 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,break ; }if (ret &VM_FAULT_RETRY ) { -if (nonblocking && +if (locked && !(fault_flags &FAULT_FLAG_RETRY_NOWAIT )) - *nonblocking =0 ; + *locked =0 ; *nr_pages =0 ;/* * VM_FAULT_RETRY must not return an
Signature · 1 line
2.21.0
Re: [PATCH v2 1/7] mm/gup: Rename "nonblocking" to "locked" where proper
Quoted 179 lines · Peter Xu
There's plenty of places around
__get_user_pages()that has a parameter "nonblocking" which does not really mean that "it won't block" (because it can really block) but instead it shows whether themmap_semis released byup_read()during the page fault handling mostly whenVM_FAULT_RETRYis returned.We have the correct naming in e.g.
get_user_pages_locked()orget_user_pages_remote()as "locked", however there're still many places that are using the "nonblocking" as name.Renaming the places to "locked" where proper to better suite the functionality of the variable. While at it, fixing up some of the comments accordingly.
Reviewed-by Mike Rapoport <[email protected]>Reviewed-by Jerome Glisse <[email protected]>Signed-off-by Peter Xu <[email protected]>--- mm/gup.c | 44 +++++++++++++++++++++----------------------- mm/hugetlb.c | 8 ++++---- 2 files changed, 25 insertions(+), 27 deletions(-)mm/gup.c +21
-23@@ -622,12 +622,12 @@ static int get_gate_page(struct mm_struct *mm, unsigned long address, }/* -* mmap_sem must be held on entry. If @nonblocking != NULL and -* *@flags does not include FOLL_NOWAIT, the mmap_sem may be released. -* If it is, *@nonblocking will be set to 0 and -EBUSY returned. +* mmap_sem must be held on entry. If @locked != NULL and *@flags +* does not include FOLL_NOWAIT, the mmap_sem may be released. If it +* is, *@locked will be set to 0 and -EBUSY returned. */ static int faultin_page (struct task_struct *tsk,struct vm_area_struct *vma, -unsigned long address,unsigned int *flags,int *nonblocking) +unsigned long address,unsigned int *flags,int *locked) {unsigned int fault_flags =0 ;vm_fault_t ret;@@ -639,7 +639,7 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma, fault_flags |=FAULT_FLAG_WRITE ;if (*flags &FOLL_REMOTE ) fault_flags |=FAULT_FLAG_REMOTE ; -if (nonblocking) +if (locked) fault_flags |=FAULT_FLAG_ALLOW_RETRY ;if (*flags &FOLL_NOWAIT ) fault_flags |=FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_RETRY_NOWAIT ;@@ -665,8 +665,8 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma, }if (ret &VM_FAULT_RETRY ) { -if (nonblocking && !(fault_flags &FAULT_FLAG_RETRY_NOWAIT )) - *nonblocking =0 ; +if (locked && !(fault_flags &FAULT_FLAG_RETRY_NOWAIT )) + *locked =0 ;return -EBUSY ; }@@ -743,7 +743,7 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)* only intends to ensure the pages are faulted in. * @vmas: array of pointers to vmas corresponding to each page. * Or NULL if the caller does not require them. -* @nonblocking: whether waiting for disk IO or mmap_sem contention +* @locked: whether we're still with the mmap_sem held * * Returns number of pages pinned. This may be fewer than the number * requested. If nr_pages is 0 or negative, returns 0. If no pages @@ -772,13 +772,11 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)* appropriate) must be called after the page is finished with, and * before put_page is called. * -* If @nonblocking != NULL, __get_user_pages will not wait for disk IO -* or mmap_sem contention, and if waiting is needed to pin all pages, -* *@nonblocking will be set to 0. Further, if @gup_flags does not -* include FOLL_NOWAIT, the mmap_sem will be released via up_read() in -* this case. +* If @locked != NULL, *@locked will be set to 0 when mmap_sem is +* released by an up_read(). That can happen if @gup_flags does not +* have FOLL_NOWAIT. * -* A caller using such a combination of @nonblocking and @gup_flags +* A caller using such a combination of @locked and @gup_flags * must therefore hold the mmap_sem for reading only, and recognize * when it's been released. Otherwise, it must be held for either * reading or writing and will not be released. @@ -790,7 +788,7 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)static long __get_user_pages (struct task_struct *tsk,struct mm_struct *mm,unsigned long start,unsigned long nr_pages,unsigned int gup_flags,struct page **pages, -struct vm_area_struct **vmas,int *nonblocking) +struct vm_area_struct **vmas,int *locked) {long ret =0 , i =0 ;struct vm_area_struct *vma =NULL ;@@ -834,7 +832,7 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,if (is_vm_hugetlb_page (vma)) { i =follow_hugetlb_page (mm, vma, pages, vmas, &start, &nr_pages, i, - gup_flags, nonblocking); + gup_flags, locked);continue ; } }@@ -852,7 +850,7 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm, page =follow_page_mask (vma, start, foll_flags, &ctx);if (!page) { ret =faultin_page (tsk, vma, start, &foll_flags, - nonblocking); + locked);switch (ret) {case 0 : goto retry;@@ -1178,7 +1176,7 @@ EXPORT_SYMBOL(get_user_pages_remote);* @vma: target vma * @start: start address * @end: end address -* @nonblocking: +* @locked: whether the mmap_sem is still held * * This takes care of mlocking the pages too if VM_LOCKED is set. * @@ -1186,14 +1184,14 @@ EXPORT_SYMBOL(get_user_pages_remote);* * vma->vm_mm->mmap_sem must be held. * -* If @nonblocking is NULL, it may be held for read or write and will +* If @locked is NULL, it may be held for read or write and will * be unperturbed. * -* If @nonblocking is non-NULL, it must held for read only and may be -* released. If it's released, *@nonblocking will be set to 0. +* If @locked is non-NULL, it must held for read only and may be +* released. If it's released, *@locked will be set to 0. */ long populate_vma_page_range (struct vm_area_struct *vma, -unsigned long start,unsigned long end,int *nonblocking) +unsigned long start,unsigned long end,int *locked) {struct mm_struct *mm = vma->vm_mm;unsigned long nr_pages = (end - start) /PAGE_SIZE ;@@ -1228,7 +1226,7 @@ long populate_vma_page_range(struct vm_area_struct *vma,* not result in a stack expansion that recurses back here. */ return __get_user_pages (current, mm, start, nr_pages, gup_flags, -NULL ,NULL , nonblocking); +NULL ,NULL , locked); }/* mm/hugetlb.c +2
-2@@ -4251,7 +4251,7 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,long follow_hugetlb_page (struct mm_struct *mm,struct vm_area_struct *vma,struct page **pages,struct vm_area_struct **vmas,unsigned long *position,unsigned long *nr_pages, -long i,unsigned int flags,int *nonblocking) +long i,unsigned int flags,int *locked) {unsigned long pfn_offset;unsigned long vaddr = *position;@@ -4322,7 +4322,7 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,spin_unlock (ptl);if (flags &FOLL_WRITE ) fault_flags |=FAULT_FLAG_WRITE ; -if (nonblocking) +if (locked) fault_flags |=FAULT_FLAG_ALLOW_RETRY ;if (flags &FOLL_NOWAIT ) fault_flags |= FAULT_FLAG_ALLOW_RETRY |@@ -4339,9 +4339,9 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,break ;
mm/hugetlb.c long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, · line 4347
Quoted 10 lines
mm/hugetlb.c +2
-2}if (ret &VM_FAULT_RETRY ) { -if (nonblocking && +if (locked && !(fault_flags &FAULT_FLAG_RETRY_NOWAIT )) - *nonblocking =0 ; + *locked =0 ; *nr_pages =0 ;/* * VM_FAULT_RETRY must not return an
Signature · 3 lines
Thanks, David / dhildenb
Peter Xu
reviewerPATCH v2 2/7 mm: Introduce FAULT_FLAG_DEFAULT
In reply to Peter XuAlthough there're tons of arch-specific page fault handlers, most of them are still sharing the same initial value of the page fault flags. Say, merely all of the page fault handlers would allow the fault to be retried, and they also allow the fault to respond to SIGKILL.
Let's define a default value for the fault flags to replace those initial page fault flags that were copied over. With this, it'll be far easier to introduce new fault flag that can be used by all the architectures instead of touching all the archs.
--- arch/alpha/mm/fault.c | 2 +- arch/arc/mm/fault.c | 2 +- arch/arm/mm/fault.c | 2 +- arch/arm64/mm/fault.c | 2 +- arch/hexagon/mm/vm_fault.c | 2 +- arch/ia64/mm/fault.c | 2 +- arch/m68k/mm/fault.c | 2 +- arch/microblaze/mm/fault.c | 2 +- arch/mips/mm/fault.c | 2 +- arch/nds32/mm/fault.c | 2 +- arch/nios2/mm/fault.c | 2 +- arch/openrisc/mm/fault.c | 2 +- arch/parisc/mm/fault.c | 2 +- arch/powerpc/mm/fault.c | 2 +- arch/riscv/mm/fault.c | 2 +- arch/s390/mm/fault.c | 2 +- arch/sh/mm/fault.c | 2 +- arch/sparc/mm/fault_32.c | 2 +- arch/sparc/mm/fault_64.c | 2 +- arch/um/kernel/trap.c | 2 +- arch/unicore32/mm/fault.c | 2 +- arch/x86/mm/fault.c | 2 +- arch/xtensa/mm/fault.c | 2 +- include/linux/mm.h | 7 +++++++ 24 files changed, 30 insertions(+), 23 deletions(-)
arch/alpha/mm/fault.c +1 -1
@@ -89,7 +89,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr,const struct exception_table_entry *fixup;int si_code =SEGV_MAPERR ;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;/* As of EV6, a load into $31/$f31 is a prefetch, and never faults (or is suppressed by the PALcode). Support that for older CPUs
arch/arc/mm/fault.c +1 -1
@@ -94,7 +94,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) (regs->ecr_cause == ECR_C_PROTV_INST_FETCH)) exec =1 ; - flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; + flags =FAULT_FLAG_DEFAULT ;if (user_mode (regs)) flags |=FAULT_FLAG_USER ;if (write)
arch/arm/mm/fault.c +1 -1
@@ -241,7 +241,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)struct mm_struct *mm;int sig, code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;if (kprobe_page_fault (regs, fsr))return 0 ;
arch/arm64/mm/fault.c +1 -1
@@ -410,7 +410,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,struct mm_struct *mm = current->mm;vm_fault_t fault, major =0 ;unsigned long vm_flags =VM_READ |VM_WRITE ; -unsigned int mm_flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int mm_flags =FAULT_FLAG_DEFAULT ;if (kprobe_page_fault (regs, esr))return 0 ;
arch/hexagon/mm/vm_fault.c +1 -1
@@ -41,7 +41,7 @@ void do_page_fault(unsigned long address, long cause, struct pt_regs *regs)int si_code =SEGV_MAPERR ;vm_fault_t fault;const struct exception_table_entry *fixup; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;/* * If we're in an interrupt or have no user context,
arch/ia64/mm/fault.c +1 -1
@@ -65,7 +65,7 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *restruct mm_struct *mm = current->mm;unsigned long mask;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; mask = ((((isr >>IA64_ISR_X_BIT ) &1UL ) <<VM_EXEC_BIT ) | (((isr >>IA64_ISR_W_BIT ) &1UL ) <<VM_WRITE_BIT ));
arch/m68k/mm/fault.c +1 -1
@@ -71,7 +71,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,struct mm_struct *mm = current->mm;struct vm_area_struct * vma;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;pr_debug ("do page fault:\nregs->sr=%#x, regs->pc=%#lx, address=%#lx, %ld, %p\n" , regs->sr, regs->pc, address, error_code, mm ? mm->pgd :NULL );
arch/microblaze/mm/fault.c +1 -1
@@ -91,7 +91,7 @@ void do_page_fault(struct pt_regs *regs, unsigned long address,int code =SEGV_MAPERR ;int is_write = error_code &ESR_S ;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; regs->ear = address; regs->esr = error_code;
arch/mips/mm/fault.c +1 -1
@@ -44,7 +44,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,const int field =sizeof (unsigned long ) *2 ;int si_code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);
arch/nds32/mm/fault.c +1 -1
@@ -76,7 +76,7 @@ void do_page_fault(unsigned long entry, unsigned long addr,int si_code;vm_fault_t fault;unsigned int mask =VM_READ |VM_WRITE |VM_EXEC ; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; error_code = error_code & (ITYPE_mskINST | ITYPE_mskETYPE); tsk = current;
arch/nios2/mm/fault.c +1 -1
@@ -47,7 +47,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long cause,struct mm_struct *mm = tsk->mm;int code =SEGV_MAPERR ;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; cause >>=2 ;
arch/openrisc/mm/fault.c +1 -1
@@ -50,7 +50,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long address,struct vm_area_struct *vma;int si_code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; tsk = current;
arch/parisc/mm/fault.c +1 -1
@@ -274,7 +274,7 @@ void do_page_fault(struct pt_regs *regs, unsigned long code,if (!mm) goto no_context; - flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; + flags =FAULT_FLAG_DEFAULT ;if (user_mode (regs)) flags |=FAULT_FLAG_USER ;
arch/powerpc/mm/fault.c +1 -1
@@ -435,7 +435,7 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, {struct vm_area_struct * vma;struct mm_struct *mm = current->mm; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;int is_exec =TRAP (regs) ==0x400 ;int is_user =user_mode (regs);int is_write =page_fault_is_write (error_code);
arch/riscv/mm/fault.c +1 -1
@@ -28,7 +28,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs)struct vm_area_struct *vma;struct mm_struct *mm;unsigned long addr, cause; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;int code =SEGV_MAPERR ;vm_fault_t fault;
arch/s390/mm/fault.c +1 -1
@@ -429,7 +429,7 @@ static inline vm_fault_t do_exception(struct pt_regs *regs, int access) address = trans_exc_code & __FAIL_ADDR_MASK;perf_sw_event (PERF_COUNT_SW_PAGE_FAULTS ,1 , regs, address); - flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; + flags =FAULT_FLAG_DEFAULT ;if (user_mode (regs)) flags |=FAULT_FLAG_USER ;if (access ==VM_WRITE || (trans_exc_code & store_indication) ==0x400 )
arch/sh/mm/fault.c +1 -1
@@ -380,7 +380,7 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,struct mm_struct *mm;struct vm_area_struct * vma;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; tsk = current; mm = tsk->mm;
arch/sparc/mm/fault_32.c +1 -1
@@ -168,7 +168,7 @@ asmlinkage void do_sparc_fault(struct pt_regs *regs, int text_fault, int write,int from_user = !(regs->psr &PSR_PS );int code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;if (text_fault) address = regs->pc;
arch/sparc/mm/fault_64.c +1 -1
@@ -267,7 +267,7 @@ asmlinkage void __kprobes do_sparc64_fault(struct pt_regs *regs)int si_code, fault_code;vm_fault_t fault;unsigned long address, mm_rss; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; fault_code =get_thread_fault_code ();
arch/um/kernel/trap.c +1 -1
@@ -32,7 +32,7 @@ int handle_page_fault(unsigned long address, unsigned long ip,pmd_t *pmd;pte_t *pte;int err = -EFAULT ; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; *code_out =SEGV_MAPERR ;
arch/unicore32/mm/fault.c +1 -1
@@ -202,7 +202,7 @@ static int do_pf(unsigned long addr, unsigned int fsr, struct pt_regs *regs)struct mm_struct *mm;int sig, code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; tsk = current; mm = tsk->mm;
arch/x86/mm/fault.c +1 -1
@@ -1287,7 +1287,7 @@ void do_user_addr_fault(struct pt_regs *regs,struct task_struct *tsk;struct mm_struct *mm;vm_fault_t fault, major =0 ; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; tsk = current; mm = tsk->mm;
arch/xtensa/mm/fault.c +1 -1
@@ -43,7 +43,7 @@ void do_page_fault(struct pt_regs *regs)int is_write, is_exec;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; code =SEGV_MAPERR ;
include/linux/mm.h +7 -0
@@ -393,6 +393,13 @@ extern pgprot_t protection_map[16];#define FAULT_FLAG_REMOTE 0x80/* faulting for non current tsk/mm */ #define FAULT_FLAG_INSTRUCTION 0x100/* The fault was during an instruction fetch */ +/* +* The default fault flags that should be used by most of the +* arch-specific page fault handlers. +*/ +#define FAULT_FLAG_DEFAULT (FAULT_FLAG_ALLOW_RETRY | \ + FAULT_FLAG_KILLABLE) +#define FAULT_FLAG_TRACE \ { FAULT_FLAG_WRITE, "WRITE" }, \ { FAULT_FLAG_MKWRITE, "MKWRITE" }, \
Signature · 1 line
2.21.0
Re: [PATCH v2 2/7] mm: Introduce FAULT_FLAG_DEFAULT
Quoted 346 lines · Peter Xu
Although there're tons of arch-specific page fault handlers, most of them are still sharing the same initial value of the page fault flags. Say, merely all of the page fault handlers would allow the fault to be retried, and they also allow the fault to respond to SIGKILL.
Let's define a default value for the fault flags to replace those initial page fault flags that were copied over. With this, it'll be far easier to introduce new fault flag that can be used by all the architectures instead of touching all the archs.
Signed-off-by Peter Xu <[email protected]>--- arch/alpha/mm/fault.c | 2 +- arch/arc/mm/fault.c | 2 +- arch/arm/mm/fault.c | 2 +- arch/arm64/mm/fault.c | 2 +- arch/hexagon/mm/vm_fault.c | 2 +- arch/ia64/mm/fault.c | 2 +- arch/m68k/mm/fault.c | 2 +- arch/microblaze/mm/fault.c | 2 +- arch/mips/mm/fault.c | 2 +- arch/nds32/mm/fault.c | 2 +- arch/nios2/mm/fault.c | 2 +- arch/openrisc/mm/fault.c | 2 +- arch/parisc/mm/fault.c | 2 +- arch/powerpc/mm/fault.c | 2 +- arch/riscv/mm/fault.c | 2 +- arch/s390/mm/fault.c | 2 +- arch/sh/mm/fault.c | 2 +- arch/sparc/mm/fault_32.c | 2 +- arch/sparc/mm/fault_64.c | 2 +- arch/um/kernel/trap.c | 2 +- arch/unicore32/mm/fault.c | 2 +- arch/x86/mm/fault.c | 2 +- arch/xtensa/mm/fault.c | 2 +- include/linux/mm.h | 7 +++++++ 24 files changed, 30 insertions(+), 23 deletions(-)arch/alpha/mm/fault.c +1
-1@@ -89,7 +89,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr,const struct exception_table_entry *fixup;int si_code =SEGV_MAPERR ;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;/* As of EV6, a load into $31/$f31 is a prefetch, and never faults (or is suppressed by the PALcode). Support that for older CPUs arch/arc/mm/fault.c +1
-1@@ -94,7 +94,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) (regs->ecr_cause == ECR_C_PROTV_INST_FETCH)) exec =1 ; - flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; + flags =FAULT_FLAG_DEFAULT ;if (user_mode (regs)) flags |=FAULT_FLAG_USER ;if (write)arch/arm/mm/fault.c +1
-1@@ -241,7 +241,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)struct mm_struct *mm;int sig, code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;if (kprobe_page_fault (regs, fsr))return 0 ;arch/arm64/mm/fault.c +1
-1@@ -410,7 +410,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,struct mm_struct *mm = current->mm;vm_fault_t fault, major =0 ;unsigned long vm_flags =VM_READ |VM_WRITE ; -unsigned int mm_flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int mm_flags =FAULT_FLAG_DEFAULT ;if (kprobe_page_fault (regs, esr))return 0 ;arch/hexagon/mm/vm_fault.c +1
-1@@ -41,7 +41,7 @@ void do_page_fault(unsigned long address, long cause, struct pt_regs *regs)int si_code =SEGV_MAPERR ;vm_fault_t fault;const struct exception_table_entry *fixup; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;/* * If we're in an interrupt or have no user context, arch/ia64/mm/fault.c +1
-1@@ -65,7 +65,7 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *restruct mm_struct *mm = current->mm;unsigned long mask;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; mask = ((((isr >>IA64_ISR_X_BIT ) &1UL ) <<VM_EXEC_BIT ) | (((isr >>IA64_ISR_W_BIT ) &1UL ) <<VM_WRITE_BIT ));arch/m68k/mm/fault.c +1
-1@@ -71,7 +71,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,struct mm_struct *mm = current->mm;struct vm_area_struct * vma;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;pr_debug ("do page fault:\nregs->sr=%#x, regs->pc=%#lx, address=%#lx, %ld, %p\n" , regs->sr, regs->pc, address, error_code, mm ? mm->pgd :NULL );arch/microblaze/mm/fault.c +1
-1@@ -91,7 +91,7 @@ void do_page_fault(struct pt_regs *regs, unsigned long address,int code =SEGV_MAPERR ;int is_write = error_code &ESR_S ;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; regs->ear = address; regs->esr = error_code;arch/mips/mm/fault.c +1
-1@@ -44,7 +44,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,const int field =sizeof (unsigned long ) *2 ;int si_code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);arch/nds32/mm/fault.c +1
-1@@ -76,7 +76,7 @@ void do_page_fault(unsigned long entry, unsigned long addr,int si_code;vm_fault_t fault;unsigned int mask =VM_READ |VM_WRITE |VM_EXEC ; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; error_code = error_code & (ITYPE_mskINST | ITYPE_mskETYPE); tsk = current;arch/nios2/mm/fault.c +1
-1@@ -47,7 +47,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long cause,struct mm_struct *mm = tsk->mm;int code =SEGV_MAPERR ;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; cause >>=2 ;arch/openrisc/mm/fault.c +1
-1@@ -50,7 +50,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long address,struct vm_area_struct *vma;int si_code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; tsk = current;arch/parisc/mm/fault.c +1
-1@@ -274,7 +274,7 @@ void do_page_fault(struct pt_regs *regs, unsigned long code,if (!mm) goto no_context; - flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; + flags =FAULT_FLAG_DEFAULT ;if (user_mode (regs)) flags |=FAULT_FLAG_USER ;arch/powerpc/mm/fault.c +1
-1@@ -435,7 +435,7 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, {struct vm_area_struct * vma;struct mm_struct *mm = current->mm; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;int is_exec =TRAP (regs) ==0x400 ;int is_user =user_mode (regs);int is_write =page_fault_is_write (error_code);arch/riscv/mm/fault.c +1
-1@@ -28,7 +28,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs)struct vm_area_struct *vma;struct mm_struct *mm;unsigned long addr, cause; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;int code =SEGV_MAPERR ;vm_fault_t fault;arch/s390/mm/fault.c +1
-1@@ -429,7 +429,7 @@ static inline vm_fault_t do_exception(struct pt_regs *regs, int access) address = trans_exc_code & __FAIL_ADDR_MASK;perf_sw_event (PERF_COUNT_SW_PAGE_FAULTS ,1 , regs, address); - flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; + flags =FAULT_FLAG_DEFAULT ;if (user_mode (regs)) flags |=FAULT_FLAG_USER ;if (access ==VM_WRITE || (trans_exc_code & store_indication) ==0x400 )arch/sh/mm/fault.c +1
-1@@ -380,7 +380,7 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,struct mm_struct *mm;struct vm_area_struct * vma;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; tsk = current; mm = tsk->mm;arch/sparc/mm/fault_32.c +1
-1@@ -168,7 +168,7 @@ asmlinkage void do_sparc_fault(struct pt_regs *regs, int text_fault, int write,int from_user = !(regs->psr &PSR_PS );int code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ;if (text_fault) address = regs->pc;arch/sparc/mm/fault_64.c +1
-1@@ -267,7 +267,7 @@ asmlinkage void __kprobes do_sparc64_fault(struct pt_regs *regs)int si_code, fault_code;vm_fault_t fault;unsigned long address, mm_rss; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; fault_code =get_thread_fault_code ();arch/um/kernel/trap.c +1
-1@@ -32,7 +32,7 @@ int handle_page_fault(unsigned long address, unsigned long ip,pmd_t *pmd;pte_t *pte;int err = -EFAULT ; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; *code_out =SEGV_MAPERR ;arch/unicore32/mm/fault.c +1
-1@@ -202,7 +202,7 @@ static int do_pf(unsigned long addr, unsigned int fsr, struct pt_regs *regs)struct mm_struct *mm;int sig, code;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; tsk = current; mm = tsk->mm;arch/x86/mm/fault.c +1
-1@@ -1287,7 +1287,7 @@ void do_user_addr_fault(struct pt_regs *regs,struct task_struct *tsk;struct mm_struct *mm;vm_fault_t fault, major =0 ; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; tsk = current; mm = tsk->mm;arch/xtensa/mm/fault.c +1
-1@@ -43,7 +43,7 @@ void do_page_fault(struct pt_regs *regs)int is_write, is_exec;vm_fault_t fault; -unsigned int flags =FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_KILLABLE ; +unsigned int flags =FAULT_FLAG_DEFAULT ; code =SEGV_MAPERR ;include/linux/mm.h +0
-0@@ -393,6 +393,13 @@ extern pgprot_t protection_map[16];#define FAULT_FLAG_REMOTE 0x80/* faulting for non current tsk/mm */ #define FAULT_FLAG_INSTRUCTION 0x100/* The fault was during an instruction fetch */
include/linux/mm.h extern pgprot_t protection_map[16]; · line 405
Quoted 10 lines
include/linux/mm.h +7
-0+/* +* The default fault flags that should be used by most of the +* arch-specific page fault handlers. +*/ +#define FAULT_FLAG_DEFAULT (FAULT_FLAG_ALLOW_RETRY | \ + FAULT_FLAG_KILLABLE) +#define FAULT_FLAG_TRACE \ { FAULT_FLAG_WRITE, "WRITE" }, \ { FAULT_FLAG_MKWRITE, "MKWRITE" }, \
Signature · 3 lines
Thanks, David / dhildenb
Peter Xu
reviewerPATCH v2 3/7 mm: Introduce FAULT_FLAG_INTERRUPTIBLE
In reply to Peter Xuhandle_userfaultfd() is currently the only one place in the kernel page fault procedures that can respond to non-fatal userspace signals. It was trying to detect such an allowance by checking against USER & KILLABLE flags, which was "un-official".
In this patch, we introduced a new flag (FAULT_FLAG_INTERRUPTIBLE) to show that the fault handler allows the fault procedure to respond even to non-fatal signals. Meanwhile, add this new flag to the default fault flags so that all the page fault handlers can benefit from the new flag. With that, replacing the userfault check to this one.
Since the line is getting even longer, clean up the fault flags a bit too to ease TTY users.
Although we've got a new flag and applied it, we shouldn't have any functional change with this patch so far.
--- fs/userfaultfd.c | 4 +--- include/linux/mm.h | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 14 deletions(-)
fs/userfaultfd.c +1 -3
@@ -462,9 +462,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason) uwq.ctx = ctx; uwq.waken = false; - return_to_userland = - (vmf->flags & (FAULT_FLAG_USER |FAULT_FLAG_KILLABLE )) == - (FAULT_FLAG_USER |FAULT_FLAG_KILLABLE ); + return_to_userland = vmf->flags &FAULT_FLAG_INTERRUPTIBLE ; blocking_state = return_to_userland ?TASK_INTERRUPTIBLE :TASK_KILLABLE ;
include/linux/mm.h +28 -11
@@ -383,22 +383,38 @@ extern unsigned int kobjsize(const void *objp);*/ extern pgprot_t protection_map[16 ]; -#define FAULT_FLAG_WRITE 0x01/* Fault was a write access */ -#define FAULT_FLAG_MKWRITE 0x02/* Fault was mkwrite of existing pte */ -#define FAULT_FLAG_ALLOW_RETRY 0x04/* Retry fault if blocking */ -#define FAULT_FLAG_RETRY_NOWAIT 0x08/* Don't drop mmap_sem and wait when retrying */ -#define FAULT_FLAG_KILLABLE 0x10/* The fault task is in SIGKILL killable region */ -#define FAULT_FLAG_TRIED 0x20/* Second try */ -#define FAULT_FLAG_USER 0x40/* The fault originated in userspace */ -#define FAULT_FLAG_REMOTE 0x80/* faulting for non current tsk/mm */ -#define FAULT_FLAG_INSTRUCTION 0x100/* The fault was during an instruction fetch */ +/** +* Fault flag definitions. +* +* @FAULT_FLAG_WRITE: Fault was a write fault. +* @FAULT_FLAG_MKWRITE: Fault was mkwrite of existing PTE. +* @FAULT_FLAG_ALLOW_RETRY: Allow to retry the fault if blocked. +* @FAULT_FLAG_RETRY_NOWAIT: Don't drop mmap_sem and wait when retrying. +* @FAULT_FLAG_KILLABLE: The fault task is in SIGKILL killable region. +* @FAULT_FLAG_TRIED: The fault has been tried once. +* @FAULT_FLAG_USER: The fault originated in userspace. +* @FAULT_FLAG_REMOTE: The fault is not for current task/mm. +* @FAULT_FLAG_INSTRUCTION: The fault was during an instruction fetch. +* @FAULT_FLAG_INTERRUPTIBLE: The fault can be interrupted by non-fatal signals. +*/ +#define FAULT_FLAG_WRITE 0x01 +#define FAULT_FLAG_MKWRITE 0x02 +#define FAULT_FLAG_ALLOW_RETRY 0x04 +#define FAULT_FLAG_RETRY_NOWAIT 0x08 +#define FAULT_FLAG_KILLABLE 0x10 +#define FAULT_FLAG_TRIED 0x20 +#define FAULT_FLAG_USER 0x40 +#define FAULT_FLAG_REMOTE 0x80 +#define FAULT_FLAG_INSTRUCTION 0x100 +#define FAULT_FLAG_INTERRUPTIBLE 0x200/* * The default fault flags that should be used by most of the * arch-specific page fault handlers. */ #define FAULT_FLAG_DEFAULT (FAULT_FLAG_ALLOW_RETRY | \ - FAULT_FLAG_KILLABLE) + FAULT_FLAG_KILLABLE | \ + FAULT_FLAG_INTERRUPTIBLE)#define FAULT_FLAG_TRACE \ { FAULT_FLAG_WRITE, "WRITE" }, \
@@ -409,7 +425,8 @@ extern pgprot_t protection_map[16]; { FAULT_FLAG_TRIED, "TRIED" }, \ { FAULT_FLAG_USER, "USER" }, \ { FAULT_FLAG_REMOTE, "REMOTE" }, \ - { FAULT_FLAG_INSTRUCTION, "INSTRUCTION" } + { FAULT_FLAG_INSTRUCTION, "INSTRUCTION" }, \ + { FAULT_FLAG_INTERRUPTIBLE, "INTERRUPTIBLE" }/* * vm_fault is filled by the the pagefault handler and passed to the vma's
Signature · 1 line
2.21.0
Re: [PATCH v2 3/7] mm: Introduce FAULT_FLAG_INTERRUPTIBLE
Quoted 71 lines · Peter Xu
handle_userfaultfd()is currently the only one place in the kernel page fault procedures that can respond to non-fatal userspace signals. It was trying to detect such an allowance by checking against USER & KILLABLE flags, which was "un-official".In this patch, we introduced a new flag (
FAULT_FLAG_INTERRUPTIBLE) to show that the fault handler allows the fault procedure to respond even to non-fatal signals. Meanwhile, add this new flag to the default fault flags so that all the page fault handlers can benefit from the new flag. With that, replacing the userfault check to this one.Since the line is getting even longer, clean up the fault flags a bit too to ease TTY users.
Although we've got a new flag and applied it, we shouldn't have any functional change with this patch so far.
Suggested-by Linus Torvalds <[email protected]>Signed-off-by Peter Xu <[email protected]>--- fs/userfaultfd.c | 4 +--- include/linux/mm.h | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 14 deletions(-)fs/userfaultfd.c +1
-3@@ -462,9 +462,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason) uwq.ctx = ctx; uwq.waken = false; - return_to_userland = - (vmf->flags & (FAULT_FLAG_USER |FAULT_FLAG_KILLABLE )) == - (FAULT_FLAG_USER |FAULT_FLAG_KILLABLE ); + return_to_userland = vmf->flags &FAULT_FLAG_INTERRUPTIBLE ; blocking_state = return_to_userland ?TASK_INTERRUPTIBLE :TASK_KILLABLE ;include/linux/mm.h +15
-9@@ -383,22 +383,38 @@ extern unsigned int kobjsize(const void *objp);*/ extern pgprot_t protection_map[16 ]; -#define FAULT_FLAG_WRITE 0x01/* Fault was a write access */ -#define FAULT_FLAG_MKWRITE 0x02/* Fault was mkwrite of existing pte */ -#define FAULT_FLAG_ALLOW_RETRY 0x04/* Retry fault if blocking */ -#define FAULT_FLAG_RETRY_NOWAIT 0x08/* Don't drop mmap_sem and wait when retrying */ -#define FAULT_FLAG_KILLABLE 0x10/* The fault task is in SIGKILL killable region */ -#define FAULT_FLAG_TRIED 0x20/* Second try */ -#define FAULT_FLAG_USER 0x40/* The fault originated in userspace */ -#define FAULT_FLAG_REMOTE 0x80/* faulting for non current tsk/mm */ -#define FAULT_FLAG_INSTRUCTION 0x100/* The fault was during an instruction fetch */ +/** +* Fault flag definitions. +* +* @FAULT_FLAG_WRITE: Fault was a write fault. +* @FAULT_FLAG_MKWRITE: Fault was mkwrite of existing PTE. +* @FAULT_FLAG_ALLOW_RETRY: Allow to retry the fault if blocked. +* @FAULT_FLAG_RETRY_NOWAIT: Don't drop mmap_sem and wait when retrying. +* @FAULT_FLAG_KILLABLE: The fault task is in SIGKILL killable region. +* @FAULT_FLAG_TRIED: The fault has been tried once. +* @FAULT_FLAG_USER: The fault originated in userspace. +* @FAULT_FLAG_REMOTE: The fault is not for current task/mm. +* @FAULT_FLAG_INSTRUCTION: The fault was during an instruction fetch. +* @FAULT_FLAG_INTERRUPTIBLE: The fault can be interrupted by non-fatal signals. +*/ +#define FAULT_FLAG_WRITE 0x01
include/linux/mm.h extern unsigned int kobjsize(const void *objp); · line 410
Quoted 10 lines
include/linux/mm.h +9
-0+#define FAULT_FLAG_MKWRITE 0x02 +#define FAULT_FLAG_ALLOW_RETRY 0x04 +#define FAULT_FLAG_RETRY_NOWAIT 0x08 +#define FAULT_FLAG_KILLABLE 0x10 +#define FAULT_FLAG_TRIED 0x20 +#define FAULT_FLAG_USER 0x40 +#define FAULT_FLAG_REMOTE 0x80 +#define FAULT_FLAG_INSTRUCTION 0x100 +#define FAULT_FLAG_INTERRUPTIBLE 0x200
I'd probably split off the unrelated doc changes. Just a matter of taste.
include/linux/mm.h extern unsigned int kobjsize(const void *objp); · line 417
Quoted 8 lines
include/linux/mm.h +2
-1/* * The default fault flags that should be used by most of the * arch-specific page fault handlers. */ #define FAULT_FLAG_DEFAULT (FAULT_FLAG_ALLOW_RETRY | \ - FAULT_FLAG_KILLABLE) + FAULT_FLAG_KILLABLE | \ + FAULT_FLAG_INTERRUPTIBLE)
So by default, all faults are marked interruptible, also !FAULT_FLAG_USER. I assume the trick right now is that handle_userfault() will indeed only be called on user faults and the flag is used nowhere else ;)
Would it make sense to name it FAULT_FLAG_USER_INTERRUPTIBLE, to stress that the flag only applies to user faults? (or am I missing something and this could also apply to !user faults somewhen in the future?
(I am no expert on the fault paths yet, so sorry for the silly questions)
Quoted 14 lines
include/linux/mm.h +2
-1#define FAULT_FLAG_TRACE \ { FAULT_FLAG_WRITE, "WRITE" }, \@@ -409,7 +425,8 @@ extern pgprot_t protection_map[16]; { FAULT_FLAG_TRIED, "TRIED" }, \ { FAULT_FLAG_USER, "USER" }, \ { FAULT_FLAG_REMOTE, "REMOTE" }, \ - { FAULT_FLAG_INSTRUCTION, "INSTRUCTION" } + { FAULT_FLAG_INSTRUCTION, "INSTRUCTION" }, \ + { FAULT_FLAG_INTERRUPTIBLE, "INTERRUPTIBLE" }/* * vm_fault is filled by the the pagefault handler and passed to the vma's
Signature · 3 lines
Thanks, David / dhildenb
Peter Xu
reviewerRe: [PATCH v2 3/7] mm: Introduce FAULT_FLAG_INTERRUPTIBLE
Quoted 76 lines · David Hildenbrand
Quoted 75 lines · Peter Xu
handle_userfaultfd()is currently the only one place in the kernel page fault procedures that can respond to non-fatal userspace signals. It was trying to detect such an allowance by checking against USER & KILLABLE flags, which was "un-official".In this patch, we introduced a new flag (
FAULT_FLAG_INTERRUPTIBLE) to show that the fault handler allows the fault procedure to respond even to non-fatal signals. Meanwhile, add this new flag to the default fault flags so that all the page fault handlers can benefit from the new flag. With that, replacing the userfault check to this one.Since the line is getting even longer, clean up the fault flags a bit too to ease TTY users.
Although we've got a new flag and applied it, we shouldn't have any functional change with this patch so far.
Suggested-by Linus Torvalds <[email protected]>Signed-off-by Peter Xu <[email protected]>--- fs/userfaultfd.c | 4 +--- include/linux/mm.h | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 14 deletions(-)fs/userfaultfd.c +1
-3@@ -462,9 +462,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason) uwq.ctx = ctx; uwq.waken = false; - return_to_userland = - (vmf->flags & (FAULT_FLAG_USER |FAULT_FLAG_KILLABLE )) == - (FAULT_FLAG_USER |FAULT_FLAG_KILLABLE ); + return_to_userland = vmf->flags &FAULT_FLAG_INTERRUPTIBLE ; blocking_state = return_to_userland ?TASK_INTERRUPTIBLE :TASK_KILLABLE ;include/linux/mm.h +19
-9@@ -383,22 +383,38 @@ extern unsigned int kobjsize(const void *objp);*/ extern pgprot_t protection_map[16 ]; -#define FAULT_FLAG_WRITE 0x01/* Fault was a write access */ -#define FAULT_FLAG_MKWRITE 0x02/* Fault was mkwrite of existing pte */ -#define FAULT_FLAG_ALLOW_RETRY 0x04/* Retry fault if blocking */ -#define FAULT_FLAG_RETRY_NOWAIT 0x08/* Don't drop mmap_sem and wait when retrying */ -#define FAULT_FLAG_KILLABLE 0x10/* The fault task is in SIGKILL killable region */ -#define FAULT_FLAG_TRIED 0x20/* Second try */ -#define FAULT_FLAG_USER 0x40/* The fault originated in userspace */ -#define FAULT_FLAG_REMOTE 0x80/* faulting for non current tsk/mm */ -#define FAULT_FLAG_INSTRUCTION 0x100/* The fault was during an instruction fetch */ +/** +* Fault flag definitions. +* +* @FAULT_FLAG_WRITE: Fault was a write fault. +* @FAULT_FLAG_MKWRITE: Fault was mkwrite of existing PTE. +* @FAULT_FLAG_ALLOW_RETRY: Allow to retry the fault if blocked. +* @FAULT_FLAG_RETRY_NOWAIT: Don't drop mmap_sem and wait when retrying. +* @FAULT_FLAG_KILLABLE: The fault task is in SIGKILL killable region. +* @FAULT_FLAG_TRIED: The fault has been tried once. +* @FAULT_FLAG_USER: The fault originated in userspace. +* @FAULT_FLAG_REMOTE: The fault is not for current task/mm. +* @FAULT_FLAG_INSTRUCTION: The fault was during an instruction fetch. +* @FAULT_FLAG_INTERRUPTIBLE: The fault can be interrupted by non-fatal signals. +*/ +#define FAULT_FLAG_WRITE 0x01 +#define FAULT_FLAG_MKWRITE 0x02 +#define FAULT_FLAG_ALLOW_RETRY 0x04 +#define FAULT_FLAG_RETRY_NOWAIT 0x08 +#define FAULT_FLAG_KILLABLE 0x10
Quoted 8 lines
include/linux/mm.h extern unsigned int kobjsize(const void *objp); · line 410
Quoted 6 lines
include/linux/mm.h +5
-0+#define FAULT_FLAG_TRIED 0x20 +#define FAULT_FLAG_USER 0x40 +#define FAULT_FLAG_REMOTE 0x80 +#define FAULT_FLAG_INSTRUCTION 0x100 +#define FAULT_FLAG_INTERRUPTIBLE 0x200I'd probably split off the unrelated doc changes. Just a matter of taste.
The thing is that it's not really a document change but only a format change (when I wanted to add the new macro it's easily getting out of 80 chars so I simply reformatted all the rest to make them look similar). I'm afraid that could be too trivial to change the format as a single patch, but I can do it if anyone else also thinks it proper.
Quoted 14 lines
include/linux/mm.h extern unsigned int kobjsize(const void *objp); · line 417
Quoted 9 lines
include/linux/mm.h +2
-1/* * The default fault flags that should be used by most of the * arch-specific page fault handlers. */ #define FAULT_FLAG_DEFAULT (FAULT_FLAG_ALLOW_RETRY | \ - FAULT_FLAG_KILLABLE) + FAULT_FLAG_KILLABLE | \ + FAULT_FLAG_INTERRUPTIBLE)So by default, all faults are marked interruptible, also !FAULT_FLAG_USER. I assume the trick right now is that
handle_userfault()will indeed only be called on user faults and the flag is used nowhere else ;)
Sorry if this is confusing, but FAULT_FLAG_DEFAULT is just a macro to make the patchset easier so we define this initial flags for most of the archs (say, there can be some arch that does not use this default value, but the fact is most archs are indeed using the same flags hence we define it here now).
And, userfaultfd can also handle kernel faults. For FAULT_FLAG_USER, it will be set if the fault comes from userspace (in do_user_addr_fault()).
Quoted 4 lines
Would it make sense to name it
FAULT_FLAG_USER_INTERRUPTIBLE, to stress that the flag only applies to user faults? (or am I missing something and this could also apply to !user faults somewhen in the future?
As mentioned above, uffd can handle kernel faults. And, for what I understand, it's not really directly related to user fault or not at all, instead its more or less match with TASK_{INTERRUPTIBLE|KILLABLE} on what kind of signals we care about during the fault processing. So it seems to me that it's two different things.
Quoted 2 lines
(I am no expert on the fault paths yet, so sorry for the silly questions)
(I only hope that I'm not providing silly answers. :)
Thanks,
Signature · 1 line
Peter Xu
Re: [PATCH v2 3/7] mm: Introduce FAULT_FLAG_INTERRUPTIBLE
Quoted 114 lines · Peter Xu
Quoted 76 lines · David Hildenbrand
Quoted 75 lines · Peter Xu
handle_userfaultfd()is currently the only one place in the kernel page fault procedures that can respond to non-fatal userspace signals. It was trying to detect such an allowance by checking against USER & KILLABLE flags, which was "un-official".In this patch, we introduced a new flag (
FAULT_FLAG_INTERRUPTIBLE) to show that the fault handler allows the fault procedure to respond even to non-fatal signals. Meanwhile, add this new flag to the default fault flags so that all the page fault handlers can benefit from the new flag. With that, replacing the userfault check to this one.Since the line is getting even longer, clean up the fault flags a bit too to ease TTY users.
Although we've got a new flag and applied it, we shouldn't have any functional change with this patch so far.
Suggested-by Linus Torvalds <[email protected]>Signed-off-by Peter Xu <[email protected]>--- fs/userfaultfd.c | 4 +--- include/linux/mm.h | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 14 deletions(-)fs/userfaultfd.c +1
-3@@ -462,9 +462,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason) uwq.ctx = ctx; uwq.waken = false; - return_to_userland = - (vmf->flags & (FAULT_FLAG_USER |FAULT_FLAG_KILLABLE )) == - (FAULT_FLAG_USER |FAULT_FLAG_KILLABLE ); + return_to_userland = vmf->flags &FAULT_FLAG_INTERRUPTIBLE ; blocking_state = return_to_userland ?TASK_INTERRUPTIBLE :TASK_KILLABLE ;include/linux/mm.h +19
-9@@ -383,22 +383,38 @@ extern unsigned int kobjsize(const void *objp);*/ extern pgprot_t protection_map[16 ]; -#define FAULT_FLAG_WRITE 0x01/* Fault was a write access */ -#define FAULT_FLAG_MKWRITE 0x02/* Fault was mkwrite of existing pte */ -#define FAULT_FLAG_ALLOW_RETRY 0x04/* Retry fault if blocking */ -#define FAULT_FLAG_RETRY_NOWAIT 0x08/* Don't drop mmap_sem and wait when retrying */ -#define FAULT_FLAG_KILLABLE 0x10/* The fault task is in SIGKILL killable region */ -#define FAULT_FLAG_TRIED 0x20/* Second try */ -#define FAULT_FLAG_USER 0x40/* The fault originated in userspace */ -#define FAULT_FLAG_REMOTE 0x80/* faulting for non current tsk/mm */ -#define FAULT_FLAG_INSTRUCTION 0x100/* The fault was during an instruction fetch */ +/** +* Fault flag definitions. +* +* @FAULT_FLAG_WRITE: Fault was a write fault. +* @FAULT_FLAG_MKWRITE: Fault was mkwrite of existing PTE. +* @FAULT_FLAG_ALLOW_RETRY: Allow to retry the fault if blocked. +* @FAULT_FLAG_RETRY_NOWAIT: Don't drop mmap_sem and wait when retrying. +* @FAULT_FLAG_KILLABLE: The fault task is in SIGKILL killable region. +* @FAULT_FLAG_TRIED: The fault has been tried once. +* @FAULT_FLAG_USER: The fault originated in userspace. +* @FAULT_FLAG_REMOTE: The fault is not for current task/mm. +* @FAULT_FLAG_INSTRUCTION: The fault was during an instruction fetch. +* @FAULT_FLAG_INTERRUPTIBLE: The fault can be interrupted by non-fatal signals. +*/ +#define FAULT_FLAG_WRITE 0x01 +#define FAULT_FLAG_MKWRITE 0x02 +#define FAULT_FLAG_ALLOW_RETRY 0x04 +#define FAULT_FLAG_RETRY_NOWAIT 0x08 +#define FAULT_FLAG_KILLABLE 0x10Quoted 8 lines
include/linux/mm.h extern unsigned int kobjsize(const void *objp); · line 410
Quoted 6 lines
include/linux/mm.h +5
-0+#define FAULT_FLAG_TRIED 0x20 +#define FAULT_FLAG_USER 0x40 +#define FAULT_FLAG_REMOTE 0x80 +#define FAULT_FLAG_INSTRUCTION 0x100 +#define FAULT_FLAG_INTERRUPTIBLE 0x200I'd probably split off the unrelated doc changes. Just a matter of taste.
The thing is that it's not really a document change but only a format change (when I wanted to add the new macro it's easily getting out of 80 chars so I simply reformatted all the rest to make them look similar). I'm afraid that could be too trivial to change the format as a single patch, but I can do it if anyone else also thinks it proper.
Quoted 14 lines
include/linux/mm.h extern unsigned int kobjsize(const void *objp); · line 417
Quoted 9 lines
include/linux/mm.h +2
-1/* * The default fault flags that should be used by most of the * arch-specific page fault handlers. */ #define FAULT_FLAG_DEFAULT (FAULT_FLAG_ALLOW_RETRY | \ - FAULT_FLAG_KILLABLE) + FAULT_FLAG_KILLABLE | \ + FAULT_FLAG_INTERRUPTIBLE)So by default, all faults are marked interruptible, also !FAULT_FLAG_USER. I assume the trick right now is that
handle_userfault()will indeed only be called on user faults and the flag is used nowhere else ;)Sorry if this is confusing, but
FAULT_FLAG_DEFAULTis just a macro to make the patchset easier so we define this initial flags for most of the archs (say, there can be some arch that does not use this default value, but the fact is most archs are indeed using the same flags hence we define it here now).
Quoted 4 lines
And, userfaultfd can also handle kernel faults. For
FAULT_FLAG_USER, it will be set if the fault comes from userspace (indo_user_addr_fault()).
Got it, sounds sane to me then.
Quoted 12 lines
Quoted 4 lines
Would it make sense to name it
FAULT_FLAG_USER_INTERRUPTIBLE, to stress that the flag only applies to user faults? (or am I missing something and this could also apply to !user faults somewhen in the future?As mentioned above, uffd can handle kernel faults. And, for what I understand, it's not really directly related to user fault or not at all, instead its more or less match with TASK_{INTERRUPTIBLE|KILLABLE} on what kind of signals we care about during the fault processing. So it seems to me that it's two different things.
Quoted 1 line
Quoted 3 lines
Quoted 1 line
(I am no expert on the fault paths yet, so sorry for the silly questions)
(I only hope that I'm not providing silly answers. :)
I highly doubt it :) Thanks!
Signature · 3 lines
Thanks, David / dhildenb
Peter Xu
reviewerPATCH v2 4/7 mm: Return faster for non-fatal signals in user mode faults
In reply to Peter XuThe idea comes from the upstream discussion between Linus and Andrea:
https://lkml.org/lkml/2017/10/30/560
A summary to the issue: there was a special path in handle_userfault() in the past that we'll return a VM_FAULT_NOPAGE when we detected non-fatal signals when waiting for userfault handling. We did that by reacquiring the mmap_sem before returning. However that brings a risk in that the vmas might have changed when we retake the mmap_sem and even we could be holding an invalid vma structure.
This patch is a preparation of removing that special path by allowing the page fault to return even faster if we were interrupted by a non-fatal signal during a user-mode page fault handling routine.
--- arch/alpha/mm/fault.c | 3 ++- arch/arc/mm/fault.c | 5 +++++ arch/arm/mm/fault.c | 9 +++++---- arch/arm64/mm/fault.c | 9 +++++---- arch/hexagon/mm/vm_fault.c | 3 ++- arch/ia64/mm/fault.c | 3 ++- arch/m68k/mm/fault.c | 5 +++-- arch/microblaze/mm/fault.c | 3 ++- arch/mips/mm/fault.c | 3 ++- arch/nds32/mm/fault.c | 9 +++++---- arch/nios2/mm/fault.c | 3 ++- arch/openrisc/mm/fault.c | 3 ++- arch/parisc/mm/fault.c | 3 ++- arch/powerpc/mm/fault.c | 2 ++ arch/riscv/mm/fault.c | 5 +++-- arch/s390/mm/fault.c | 4 ++-- arch/sh/mm/fault.c | 4 ++++ arch/sparc/mm/fault_32.c | 2 +- arch/sparc/mm/fault_64.c | 3 ++- arch/um/kernel/trap.c | 4 +++- arch/unicore32/mm/fault.c | 5 +++-- arch/x86/mm/fault.c | 2 ++ arch/xtensa/mm/fault.c | 3 ++- include/linux/sched/signal.h | 12 ++++++++++++ 24 files changed, 75 insertions(+), 32 deletions(-)
arch/alpha/mm/fault.c +2 -1
@@ -150,7 +150,8 @@ do_page_fault(unsigned long address, unsigned long mmcsr,the fault. */ fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/arc/mm/fault.c +5 -0
@@ -142,6 +142,11 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) goto no_context;return ; } + +/* Allow user to handle non-fatal signals first */ +if (signal_pending (current) &&user_mode (regs)) +return ; +/* * retry state machine */
arch/arm/mm/fault.c +5 -4
@@ -291,14 +291,15 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) fault =__do_page_fault (mm, addr, fsr, flags, tsk); -/* If we need to retry but a fatal signal is pending, handle the +/* If we need to retry but a signal is pending, try to handle the * signal first. We do not need to release the mmap_sem because * it would already be released in __lock_page_or_retry in * mm/filemap.c. */ -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) { -if (!user_mode (regs)) +if (unlikely (fault &VM_FAULT_RETRY &&signal_pending (current))) { +if (fatal_signal_pending (current) && !user_mode (regs)) goto no_context; -return 0 ; +if (user_mode (regs)) +return 0 ; }/*
arch/arm64/mm/fault.c +5 -4
@@ -479,15 +479,16 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,if (fault &VM_FAULT_RETRY ) {/* -* If we need to retry but a fatal signal is pending, +* If we need to retry but a signal is pending, try to * handle the signal first. We do not need to release * the mmap_sem because it would already be released * in __lock_page_or_retry in mm/filemap.c. */ -if (fatal_signal_pending (current)) { -if (!user_mode (regs)) +if (signal_pending (current)) { +if (fatal_signal_pending (current) && !user_mode (regs)) goto no_context; -return 0 ; +if (user_mode (regs)) +return 0 ; }/*
arch/hexagon/mm/vm_fault.c +2 -1
@@ -91,7 +91,8 @@ void do_page_fault(unsigned long address, long cause, struct pt_regs *regs) fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;/* The most common case -- we are done. */
arch/ia64/mm/fault.c +2 -1
@@ -141,7 +141,8 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *re*/ fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/m68k/mm/fault.c +3 -2
@@ -138,8 +138,9 @@ int do_page_fault(struct pt_regs *regs, unsigned long address, fault =handle_mm_fault (vma, address, flags);pr_debug ("handle_mm_fault returns %x\n" , fault); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) -return 0 ; +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs))) +return ;if (unlikely (fault &VM_FAULT_ERROR )) {if (fault &VM_FAULT_OOM )
arch/microblaze/mm/fault.c +2 -1
@@ -217,7 +217,8 @@ void do_page_fault(struct pt_regs *regs, unsigned long address,*/ fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/mips/mm/fault.c +2 -1
@@ -154,7 +154,8 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,*/ fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;perf_sw_event (PERF_COUNT_SW_PAGE_FAULTS ,1 , regs, address);
arch/nds32/mm/fault.c +5 -4
@@ -206,14 +206,15 @@ void do_page_fault(unsigned long entry, unsigned long addr, fault =handle_mm_fault (vma, addr, flags);/* -* If we need to retry but a fatal signal is pending, handle the +* If we need to retry but a signal is pending, try to handle the * signal first. We do not need to release the mmap_sem because it * would already be released in __lock_page_or_retry in mm/filemap.c. */ -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) { -if (!user_mode (regs)) +if ((fault &VM_FAULT_RETRY ) &&signal_pending (current)) { +if (fatal_signal_pending (current) && !user_mode (regs)) goto no_context; -return ; +if (user_mode (regs)) +return ; }if (unlikely (fault &VM_FAULT_ERROR )) {
arch/nios2/mm/fault.c +2 -1
@@ -133,7 +133,8 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long cause,*/ fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/openrisc/mm/fault.c +2 -1
@@ -161,7 +161,8 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long address, fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/parisc/mm/fault.c +2 -1
@@ -304,7 +304,8 @@ void do_page_fault(struct pt_regs *regs, unsigned long code, fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/powerpc/mm/fault.c +2 -0
@@ -596,6 +596,8 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,*/ flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ; +if (is_user &&signal_pending (current)) +return 0 ;if (!fatal_signal_pending (current)) goto retry; }
arch/riscv/mm/fault.c +3 -2
@@ -111,11 +111,12 @@ asmlinkage void do_page_fault(struct pt_regs *regs) fault =handle_mm_fault (vma, addr, flags);/* -* If we need to retry but a fatal signal is pending, handle the +* If we need to retry but a signal is pending, try to handle the * signal first. We do not need to release the mmap_sem because it * would already be released in __lock_page_or_retry in mm/filemap.c. */ -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (tsk)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/s390/mm/fault.c +2 -2
@@ -480,8 +480,8 @@ static inline vm_fault_t do_exception(struct pt_regs *regs, int access)* the fault. */ fault =handle_mm_fault (vma, address, flags); -/* No reason to continue if interrupted by SIGKILL. */ -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) { +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs))) { fault =VM_FAULT_SIGNAL ;if (flags &FAULT_FLAG_RETRY_NOWAIT ) goto out_up;
arch/sh/mm/fault.c +4 -0
@@ -489,6 +489,10 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,* have already released it in __lock_page_or_retry * in mm/filemap.c. */ + +if (user_mode (regs) &&signal_pending (tsk)) +return ; + goto retry; } }
arch/sparc/mm/fault_32.c +1 -1
@@ -237,7 +237,7 @@ asmlinkage void do_sparc_fault(struct pt_regs *regs, int text_fault, int write,*/ fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) &&fault_should_check_signal (from_user))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/sparc/mm/fault_64.c +2 -1
@@ -421,7 +421,8 @@ asmlinkage void __kprobes do_sparc64_fault(struct pt_regs *regs) fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (flags &FAULT_FLAG_USER )) goto exit_exception;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/um/kernel/trap.c +3 -1
@@ -76,7 +76,9 @@ int handle_page_fault(unsigned long address, unsigned long ip, fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) + +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (is_user)) goto out_nosemaphore;if (unlikely (fault &VM_FAULT_ERROR )) {
arch/unicore32/mm/fault.c +3 -2
@@ -246,11 +246,12 @@ static int do_pf(unsigned long addr, unsigned int fsr, struct pt_regs *regs) fault =__do_pf (mm, addr, fsr, flags, tsk); -/* If we need to retry but a fatal signal is pending, handle the +/* If we need to retry but a signal is pending, try to handle the * signal first. We do not need to release the mmap_sem because * it would already be released in __lock_page_or_retry in * mm/filemap.c. */ -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return 0 ;if (!(fault &VM_FAULT_ERROR ) && (flags &FAULT_FLAG_ALLOW_RETRY )) {
arch/x86/mm/fault.c +2 -0
@@ -1451,6 +1451,8 @@ void do_user_addr_fault(struct pt_regs *regs,if (flags &FAULT_FLAG_ALLOW_RETRY ) { flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ; +if ((flags &FAULT_FLAG_USER ) &&signal_pending (tsk)) +return ;if (!fatal_signal_pending (tsk)) goto retry; }
arch/xtensa/mm/fault.c +2 -1
@@ -110,7 +110,8 @@ void do_page_fault(struct pt_regs *regs)*/ fault =handle_mm_fault (vma, address, flags); -if ((fault &VM_FAULT_RETRY ) &&fatal_signal_pending (current)) +if ((fault &VM_FAULT_RETRY ) && +fault_should_check_signal (user_mode (regs)))return ;if (unlikely (fault &VM_FAULT_ERROR )) {
include/linux/sched/signal.h +12 -0
@@ -377,6 +377,18 @@ static inline int signal_pending_state(long state, struct task_struct *p)return (state &TASK_INTERRUPTIBLE ) ||__fatal_signal_pending (p); } +/* +* This should only be used in fault handlers to decide whether we +* should stop the current fault routine to handle the signals +* instead. It should normally be used when a signal interrupted a +* page fault which can lead to a VM_FAULT_RETRY. +*/ +static inline bool fault_should_check_signal (bool is_user) +{ +return (fatal_signal_pending (current) || + (is_user &&signal_pending (current))); +} +/* * Reevaluate whether the task has signals pending delivery. * Wake the task if so.
Signature · 1 line
2.21.0
Peter Xu
reviewerPATCH v2 5/7 userfaultfd: Don't retake mmap_sem to emulate NOPAGE
In reply to Peter XuThe idea comes from the upstream discussion between Linus and Andrea:
https://lkml.org/lkml/2017/10/30/560
A summary to the issue: there was a special path in handle_userfault() in the past that we'll return a VM_FAULT_NOPAGE when we detected non-fatal signals when waiting for userfault handling. We did that by reacquiring the mmap_sem before returning. However that brings a risk in that the vmas might have changed when we retake the mmap_sem and even we could be holding an invalid vma structure.
This patch removes the risk path in handle_userfault() then we will be sure that the callers of handle_mm_fault() will know that the VMAs might have changed. Meanwhile with previous patch we don't lose responsiveness as well since the core mm code now can handle the nonfatal userspace signals quickly even if we return VM_FAULT_RETRY.
--- fs/userfaultfd.c | 24 ------------------------ 1 file changed, 24 deletions(-)
fs/userfaultfd.c +0 -24
@@ -522,30 +522,6 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason)__set_current_state (TASK_RUNNING ); -if (return_to_userland) { -if (signal_pending (current) && - !fatal_signal_pending (current)) { -/* -* If we got a SIGSTOP or SIGCONT and this is -* a normal userland page fault, just let -* userland return so the signal will be -* handled and gdb debugging works. The page -* fault code immediately after we return from -* this function is going to release the -* mmap_sem and it's not depending on it -* (unlike gup would if we were not to return -* VM_FAULT_RETRY). -* -* If a fatal signal is pending we still take -* the streamlined VM_FAULT_RETRY failure path -* and there's no need to retake the mmap_sem -* in such case. -*/ -down_read (&mm->mmap_sem); - ret =VM_FAULT_NOPAGE ; - } - } -/* * Here we race with the list_del; list_add in * userfaultfd_ctx_read(), however because we don't ever run
Signature · 1 line
2.21.0
Peter Xu
reviewerPATCH v2 6/7 mm: Allow VM_FAULT_RETRY for multiple times
In reply to Peter XuThe idea comes from a discussion between Linus and Andrea [1].
Before this patch we only allow a page fault to retry once. We achieved this by clearing the FAULT_FLAG_ALLOW_RETRY flag when doing handle_mm_fault() the second time. This was majorly used to avoid unexpected starvation of the system by looping over forever to handle the page fault on a single page. However that should hardly happen, and after all for each code path to return a VM_FAULT_RETRY we'll first wait for a condition (during which time we should possibly yield the cpu) to happen before VM_FAULT_RETRY is really returned.
This patch removes the restriction by keeping the FAULT_FLAG_ALLOW_RETRY flag when we receive VM_FAULT_RETRY. It means that the page fault handler now can retry the page fault for multiple times if necessary without the need to generate another page fault event. Meanwhile we still keep the FAULT_FLAG_TRIED flag so page fault handler can still identify whether a page fault is the first attempt or not.
Then we'll have these combinations of fault flags (only considering ALLOW_RETRY flag and TRIED flag):
- ALLOW_RETRY and !TRIED: this means the page fault allows to retry, and this is the first try
- ALLOW_RETRY and TRIED: this means the page fault allows to
retry, and this is not the first try
- !ALLOW_RETRY and !TRIED: this means the page fault does not allow to retry at all
- !ALLOW_RETRY and TRIED: this is forbidden and should never be used
In existing code we have multiple places that has taken special care of the first condition above by checking against (fault_flags & FAULT_FLAG_ALLOW_RETRY). This patch introduces a simple helper to detect the first retry of a page fault by checking against both (fault_flags & FAULT_FLAG_ALLOW_RETRY) and !(fault_flag & FAULT_FLAG_TRIED) because now even the 2nd try will have the ALLOW_RETRY set, then use that helper in all existing special paths. One example is in __lock_page_or_retry(), now we'll drop the mmap_sem only in the first attempt of page fault and we'll keep it in follow up retries, so old locking behavior will be retained.
This will be a nice enhancement for current code [2] at the same time a supporting material for the future userfaultfd-writeprotect work, since in that work there will always be an explicit userfault writeprotect retry for protected pages, and if that cannot resolve the page fault (e.g., when userfaultfd-writeprotect is used in conjunction with swapped pages) then we'll possibly need a 3rd retry of the page fault. It might also benefit other potential users who will have similar requirement like userfault write-protection.
GUP code is not touched yet and will be covered in follow up patch.
Please read the thread below for more information.
[1] https://lkml.org/lkml/2017/11/2/833
[2] https://lkml.org/lkml/2018/12/30/64
--- arch/alpha/mm/fault.c | 2 +- arch/arc/mm/fault.c | 1 - arch/arm/mm/fault.c | 3 --- arch/arm64/mm/fault.c | 5 ----- arch/hexagon/mm/vm_fault.c | 1 - arch/ia64/mm/fault.c | 1 - arch/m68k/mm/fault.c | 3 --- arch/microblaze/mm/fault.c | 1 - arch/mips/mm/fault.c | 1 - arch/nds32/mm/fault.c | 1 - arch/nios2/mm/fault.c | 3 --- arch/openrisc/mm/fault.c | 1 - arch/parisc/mm/fault.c | 4 +--- arch/powerpc/mm/fault.c | 6 ------ arch/riscv/mm/fault.c | 5 ----- arch/s390/mm/fault.c | 5 +---- arch/sh/mm/fault.c | 1 - arch/sparc/mm/fault_32.c | 1 - arch/sparc/mm/fault_64.c | 1 - arch/um/kernel/trap.c | 1 - arch/unicore32/mm/fault.c | 4 +--- arch/x86/mm/fault.c | 2 -- arch/xtensa/mm/fault.c | 1 - drivers/gpu/drm/ttm/ttm_bo_vm.c | 12 ++++++++--- include/linux/mm.h | 37 +++++++++++++++++++++++++++++++++ mm/filemap.c | 2 +- mm/shmem.c | 2 +- 27 files changed, 52 insertions(+), 55 deletions(-)
arch/alpha/mm/fault.c +1 -1
@@ -170,7 +170,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr,else current->min_flt++;if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; + flags |=FAULT_FLAG_TRIED ;/* No need to up_read(&mm->mmap_sem) as we would * have already released it in __lock_page_or_retry
arch/arc/mm/fault.c +0 -1
@@ -151,7 +151,6 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)* retry state machine */ if (flags &FAULT_FLAG_ALLOW_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ; goto retry; }
arch/arm/mm/fault.c +0 -3
@@ -320,9 +320,6 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) regs, addr); }if (fault &VM_FAULT_RETRY ) { -/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk -* of starvation. */ - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ; goto retry; }
arch/arm64/mm/fault.c +0 -5
@@ -491,12 +491,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,return 0 ; } -/* -* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk of -* starvation. -*/ if (mm_flags &FAULT_FLAG_ALLOW_RETRY ) { - mm_flags &= ~FAULT_FLAG_ALLOW_RETRY ; mm_flags |=FAULT_FLAG_TRIED ; goto retry; }
arch/hexagon/mm/vm_fault.c +0 -1
@@ -103,7 +103,6 @@ void do_page_fault(unsigned long address, long cause, struct pt_regs *regs)else current->min_flt++;if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ; goto retry; }
arch/ia64/mm/fault.c +0 -1
@@ -168,7 +168,6 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *reelse current->min_flt++;if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/* No need to up_read(&mm->mmap_sem) as we would
arch/m68k/mm/fault.c +0 -3
@@ -163,9 +163,6 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,else current->min_flt++;if (fault &VM_FAULT_RETRY ) { -/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk -* of starvation. */ - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/*
arch/microblaze/mm/fault.c +0 -1
@@ -237,7 +237,6 @@ void do_page_fault(struct pt_regs *regs, unsigned long address,else current->min_flt++;if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/*
arch/mips/mm/fault.c +0 -1
@@ -179,7 +179,6 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write, tsk->min_flt++; }if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/*
arch/nds32/mm/fault.c +0 -1
@@ -243,7 +243,6 @@ void do_page_fault(unsigned long entry, unsigned long addr, 1, regs, addr); }if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/* No need to up_read(&mm->mmap_sem) as we would
arch/nios2/mm/fault.c +0 -3
@@ -158,9 +158,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long cause,else current->min_flt++;if (fault &VM_FAULT_RETRY ) { -/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk -* of starvation. */ - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/*
arch/openrisc/mm/fault.c +0 -1
@@ -182,7 +182,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long address,else tsk->min_flt++;if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/* No need to up_read(&mm->mmap_sem) as we would
arch/parisc/mm/fault.c +1 -3
@@ -329,14 +329,12 @@ void do_page_fault(struct pt_regs *regs, unsigned long code,else current->min_flt++;if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; -/* * No need to up_read(&mm->mmap_sem) as we would * have already released it in __lock_page_or_retry * in mm/filemap.c. */ - + flags |=FAULT_FLAG_TRIED ; goto retry; } }
arch/powerpc/mm/fault.c +0 -6
@@ -588,13 +588,7 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,* case. */ if (unlikely (fault &VM_FAULT_RETRY )) { -/* We retry only once */ if (flags &FAULT_FLAG_ALLOW_RETRY ) { -/* -* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk -* of starvation. -*/ - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;if (is_user &&signal_pending (current))return 0 ;
arch/riscv/mm/fault.c +0 -5
@@ -143,11 +143,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs) 1, regs, addr); }if (fault &VM_FAULT_RETRY ) { -/* -* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk -* of starvation. -*/ - flags &= ~(FAULT_FLAG_ALLOW_RETRY ); flags |=FAULT_FLAG_TRIED ;/*
arch/s390/mm/fault.c +1 -4
@@ -514,10 +514,7 @@ static inline vm_fault_t do_exception(struct pt_regs *regs, int access) fault =VM_FAULT_PFAULT ; goto out_up; } -/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk -* of starvation. */ - flags &= ~(FAULT_FLAG_ALLOW_RETRY | -FAULT_FLAG_RETRY_NOWAIT ); + flags &= ~FAULT_FLAG_RETRY_NOWAIT ; flags |=FAULT_FLAG_TRIED ;down_read (&mm->mmap_sem); goto retry;
arch/sh/mm/fault.c +0 -1
@@ -481,7 +481,6 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs, regs, address); }if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/*
arch/sparc/mm/fault_32.c +0 -1
@@ -261,7 +261,6 @@ asmlinkage void do_sparc_fault(struct pt_regs *regs, int text_fault, int write, 1, regs, address); }if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/* No need to up_read(&mm->mmap_sem) as we would
arch/sparc/mm/fault_64.c +0 -1
@@ -446,7 +446,6 @@ asmlinkage void __kprobes do_sparc64_fault(struct pt_regs *regs) 1, regs, address); }if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/* No need to up_read(&mm->mmap_sem) as we would
arch/um/kernel/trap.c +0 -1
@@ -98,7 +98,6 @@ int handle_page_fault(unsigned long address, unsigned long ip,else current->min_flt++;if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ; goto retry;
arch/unicore32/mm/fault.c +1 -3
@@ -260,9 +260,7 @@ static int do_pf(unsigned long addr, unsigned int fsr, struct pt_regs *regs)else tsk->min_flt++;if (fault &VM_FAULT_RETRY ) { -/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk -* of starvation. */ - flags &= ~FAULT_FLAG_ALLOW_RETRY ; + flags |=FAULT_FLAG_TRIED ; goto retry; } }
arch/x86/mm/fault.c +0 -2
@@ -1447,9 +1447,7 @@ void do_user_addr_fault(struct pt_regs *regs,* that we made any progress. Handle this case first. */ if (unlikely (fault &VM_FAULT_RETRY )) { -/* Retry at most once */ if (flags &FAULT_FLAG_ALLOW_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;if ((flags &FAULT_FLAG_USER ) &&signal_pending (tsk))return ;
arch/xtensa/mm/fault.c +0 -1
@@ -129,7 +129,6 @@ void do_page_fault(struct pt_regs *regs)else current->min_flt++;if (fault &VM_FAULT_RETRY ) { - flags &= ~FAULT_FLAG_ALLOW_RETRY ; flags |=FAULT_FLAG_TRIED ;/* No need to up_read(&mm->mmap_sem) as we would
drivers/gpu/drm/ttm/ttm_bo_vm.c +9 -3
@@ -61,9 +61,10 @@ static vm_fault_t ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,/* * If possible, avoid waiting for GPU with mmap_sem -* held. +* held. We only do this if the fault allows retry and this +* is the first attempt. */ -if (vmf->flags &FAULT_FLAG_ALLOW_RETRY ) { +if (fault_flag_allow_retry_first (vmf->flags)) { ret =VM_FAULT_RETRY ;if (vmf->flags &FAULT_FLAG_RETRY_NOWAIT ) goto out_unlock;
@@ -132,7 +133,12 @@ static vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf)* for the buffer to become unreserved. */ if (unlikely (!reservation_object_trylock (bo->resv))) { -if (vmf->flags &FAULT_FLAG_ALLOW_RETRY ) { +/* +* If the fault allows retry and this is the first +* fault attempt, we try to release the mmap_sem +* before waiting +*/ +if (fault_flag_allow_retry_first (vmf->flags)) {if (!(vmf->flags &FAULT_FLAG_RETRY_NOWAIT )) {ttm_bo_get (bo);up_read (&vmf->vma->vm_mm->mmap_sem);
include/linux/mm.h +37 -0
@@ -396,6 +396,25 @@ extern pgprot_t protection_map[16];* @FAULT_FLAG_REMOTE: The fault is not for current task/mm. * @FAULT_FLAG_INSTRUCTION: The fault was during an instruction fetch. * @FAULT_FLAG_INTERRUPTIBLE: The fault can be interrupted by non-fatal signals. +* +* About @FAULT_FLAG_ALLOW_RETRY and @FAULT_FLAG_TRIED: we can specify +* whether we would allow page faults to retry by specifying these two +* fault flags correctly. Currently there can be three legal combinations: +* +* (a) ALLOW_RETRY and !TRIED: this means the page fault allows retry, and +* this is the first try +* +* (b) ALLOW_RETRY and TRIED: this means the page fault allows retry, and +* we've already tried at least once +* +* (c) !ALLOW_RETRY and !TRIED: this means the page fault does not allow retry +* +* The unlisted combination (!ALLOW_RETRY && TRIED) is illegal and should never +* be used. Note that page faults can be allowed to retry for multiple times, +* in which case we'll have an initial fault with flags (a) then later on +* continuous faults with flags (b). We should always try to detect pending +* signals before a retry to make sure the continuous page faults can still be +* interrupted if necessary. */ #define FAULT_FLAG_WRITE 0x01#define FAULT_FLAG_MKWRITE 0x02
@@ -416,6 +435,24 @@ extern pgprot_t protection_map[16]; FAULT_FLAG_KILLABLE | \ FAULT_FLAG_INTERRUPTIBLE) +/** +* fault_flag_allow_retry_first - check ALLOW_RETRY the first time +* +* This is mostly used for places where we want to try to avoid taking +* the mmap_sem for too long a time when waiting for another condition +* to change, in which case we can try to be polite to release the +* mmap_sem in the first round to avoid potential starvation of other +* processes that would also want the mmap_sem. +* +* Return: true if the page fault allows retry and this is the first +* attempt of the fault handling; false otherwise. +*/ +static inline bool fault_flag_allow_retry_first (unsigned int flags) +{ +return (flags &FAULT_FLAG_ALLOW_RETRY ) && + (!(flags &FAULT_FLAG_TRIED )); +} +#define FAULT_FLAG_TRACE \ { FAULT_FLAG_WRITE, "WRITE" }, \ { FAULT_FLAG_MKWRITE, "MKWRITE" }, \
mm/filemap.c +1 -1
@@ -1399,7 +1399,7 @@ EXPORT_SYMBOL_GPL(__lock_page_killable);int __lock_page_or_retry (struct page *page,struct mm_struct *mm,unsigned int flags) { -if (flags &FAULT_FLAG_ALLOW_RETRY ) { +if (fault_flag_allow_retry_first (flags)) {/* * CAUTION! In this case, mmap_sem is not released * even though return 0.
mm/shmem.c +1 -1
@@ -2011,7 +2011,7 @@ static vm_fault_t shmem_fault(struct vm_fault *vmf)DEFINE_WAIT_FUNC (shmem_fault_wait, synchronous_wake_function); ret =VM_FAULT_NOPAGE ; -if ((vmf->flags &FAULT_FLAG_ALLOW_RETRY ) && +if (fault_flag_allow_retry_first (vmf->flags) && !(vmf->flags &FAULT_FLAG_RETRY_NOWAIT )) {/* It's polite to up mmap_sem if we can */ up_read (&vma->vm_mm->mmap_sem);
Signature · 1 line
2.21.0
Peter Xu
reviewerPATCH v2 7/7 mm/gup: Allow VM_FAULT_RETRY for multiple times
In reply to Peter XuThis is the gup counterpart of the change that allows the VM_FAULT_RETRY to happen for more than once.
--- mm/gup.c | 17 +++++++++++++---- mm/hugetlb.c | 6 ++++-- 2 files changed, 17 insertions(+), 6 deletions(-)
mm/gup.c +13 -4
@@ -644,7 +644,10 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma,if (*flags &FOLL_NOWAIT ) fault_flags |=FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_RETRY_NOWAIT ;if (*flags &FOLL_TRIED ) { -VM_WARN_ON_ONCE (fault_flags &FAULT_FLAG_ALLOW_RETRY ); +/* +* Note: FAULT_FLAG_ALLOW_RETRY and FAULT_FLAG_TRIED +* can co-exist +*/ fault_flags |=FAULT_FLAG_TRIED ; }
@@ -1059,17 +1062,23 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,if (likely (pages)) pages += ret; start += ret <<PAGE_SHIFT ; + lock_dropped = true; +retry:/* * Repeat on the address that fired VM_FAULT_RETRY -* without FAULT_FLAG_ALLOW_RETRY but with +* with both FAULT_FLAG_ALLOW_RETRY and * FAULT_FLAG_TRIED. */ *locked =1 ; - lock_dropped = true;down_read (&mm->mmap_sem); ret =__get_user_pages (tsk, mm, start,1 , flags |FOLL_TRIED , - pages,NULL ,NULL ); + pages,NULL , locked); +if (!*locked) { +/* Continue to retry until we succeeded */ +BUG_ON (ret !=0 ); + goto retry; + }if (ret !=1 ) {BUG_ON (ret >1 );if (!pages_done)
mm/hugetlb.c +4 -2
@@ -4328,8 +4328,10 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, fault_flags |=FAULT_FLAG_ALLOW_RETRY |FAULT_FLAG_RETRY_NOWAIT ;if (flags &FOLL_TRIED ) { -VM_WARN_ON_ONCE (fault_flags & -FAULT_FLAG_ALLOW_RETRY ); +/* +* Note: FAULT_FLAG_ALLOW_RETRY and +* FAULT_FLAG_TRIED can co-exist +*/ fault_flags |=FAULT_FLAG_TRIED ; } ret =hugetlb_fault (mm, vma, vaddr, fault_flags);
Signature · 1 line
2.21.0
Quoted 3 lines · Peter Xu
This series is split out of userfaultfd-wp series to only cover the general page fault changes, since it seems to make sense itself.
The series continues to look sane to me, but I'd like VM people to take a look. I see a few reviewed-by's, it would be nice to see more comments from people. I'd like to see Andrea in particular say "yeah, this looks all good to me".
Also a question on how this will get to me - it smells like Andrew's -mm tree to me, both from a VM and a userfaultfd angle (and looking around, at least a couple of previous patches by Peter have gone that way).
And it would be lovely to have actual _numbers_ for the alleged latency improvements. I 100% believe them, but still, numbers rule.
Talking about latency, what about that retry loop in gup()? That's the one I'm not at all convinced about. It doesn't check for signals, so if there is some retry logic, it loops forever. Hmm?
Linus
Peter Xu
reviewerQuoted 9 lines · Linus Torvalds
Quoted 3 lines · Peter Xu
This series is split out of userfaultfd-wp series to only cover the general page fault changes, since it seems to make sense itself.
The series continues to look sane to me, but I'd like VM people to take a look. I see a few reviewed-by's, it would be nice to see more comments from people. I'd like to see Andrea in particular say "yeah, this looks all good to me".
Yes I agree. I would appreciate if either Andrea or any of the other mm experts can comment on this patchset.
Quoted 8 lines
Also a question on how this will get to me - it smells like Andrew's -mm tree to me, both from a VM and a userfaultfd angle (and looking around, at least a couple of previous patches by Peter have gone that way).
And it would be lovely to have actual _numbers_ for the alleged latency improvements. I 100% believe them, but still, numbers rule.
If the question was about the userspace signal handling - IMHO it's not really a latency number that I can measure, but it's some functional difference just like what dfa37dc3fc1f6f wanted to solve previously (though that solution seemed to be causing some other issue like what have been mentioned in the cover letter on invalid VMA access), while this series should be a cleaner approach.
To be clear about the functional differnce: if without the userspace non-fatal handling patch in this series ("mm: Return faster for non-fatal signals in user mode faults"), we can't use Ctrl-C to stop a program hanging in handle_userfault(), nor can we use gdb to attach to that process (we can do it if with dfa37dc3fc1f6f, but again it's not the clean approach). And, if with this whole series (hence with "mm:
Return faster for non-fatal signals in user mode faults"), we can do both (Ctrl-C to stop the process, or gdb attaching to that hanging process without hanging gdb).
Quoted 4 lines
Talking about latency, what about that retry loop in
gup()? That's the one I'm not at all convinced about. It doesn't check for signals, so if there is some retry logic, it loops forever. Hmm?
Hmm seems to be a valid point... IMHO it'll be fine for non-fatal signals, because GUPs will still be without FAULT_FLAG_INTERRUPTIBLE when calling handle_mm_fault(), hence the page fault logic should at least ignore non-fatal signals. However I agree that we probably need a check for fatal signals in __get_user_pages_locked() now.
Thanks,
[1] https://lkml.org/lkml/2017/11/2/833
[2] https://github.com/xzpeter/clibs/blob/master/gpl/userspace/uffd-test/uffd-test.c
Signature · 1 line
Peter Xu