From 450e5b6f654b52bd7495e84cd46dd37d7e184415 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 14 May 2019 14:22:47 -0700 Subject: ARC: mm: do_page_fault refactor #1: remove label @good_area Invert the condition for stack expansion. No functional change Signed-off-by: Vineet Gupta --- arch/arc/mm/fault.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 8cca03480bb2..be8ea91fcc8b 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -97,21 +97,19 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) flags |= FAULT_FLAG_USER; retry: down_read(&mm->mmap_sem); + vma = find_vma(mm, address); if (!vma) goto bad_area; - if (vma->vm_start <= address) - goto good_area; - if (!(vma->vm_flags & VM_GROWSDOWN)) - goto bad_area; - if (expand_stack(vma, address)) - goto bad_area; + if (unlikely(address < vma->vm_start)) { + if (!(vma->vm_flags & VM_GROWSDOWN) || expand_stack(vma, address)) + goto bad_area; + } /* * Ok, we have a good vm_area for this memory access, so * we can handle it.. */ -good_area: si_code = SEGV_ACCERR; /* Handle protection violation, execute on heap or stack */ -- cgit v1.2.3 From 13e2cc1240eb14d1a08b2c32f88b25bf20210ebc Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 14 May 2019 16:07:24 -0700 Subject: ARC: mm: do_page_fault refactor #2: remove short lived variable Compiler will do this anyways, still.. No functional change. Signed-off-by: Vineet Gupta --- arch/arc/mm/fault.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index be8ea91fcc8b..a3a292c58e50 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -64,23 +64,18 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) struct task_struct *tsk = current; struct mm_struct *mm = tsk->mm; int si_code = SEGV_MAPERR; - int ret; vm_fault_t fault; int write = regs->ecr_cause & ECR_C_PROTV_STORE; /* ST/EX */ unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; /* - * We fault-in kernel-space virtual memory on-demand. The - * 'reference' page table is init_mm.pgd. - * * NOTE! We MUST NOT take any locks for this case. We may * be in an interrupt or a critical region, and should * only copy the information from the master page table, * nothing more. */ if (address >= VMALLOC_START && !user_mode(regs)) { - ret = handle_kernel_vaddr_fault(address); - if (unlikely(ret)) + if (unlikely(handle_kernel_vaddr_fault(address))) goto no_context; else return; -- cgit v1.2.3 From 85c5e33763a731967ca59085ffe6e694f872d38e Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 14 May 2019 14:25:54 -0700 Subject: ARC: mm: do_page_fault refactor #3: tidyup vma access permission code The coding pattern to NOT intialize variables at declaration time but rather near code which makes us eof them makes it much easier to grok the overall logic, specially when the init is not simply 0 or 1 Signed-off-by: Vineet Gupta --- arch/arc/mm/fault.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index a3a292c58e50..8c7c81ce7f6a 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -64,9 +64,9 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) struct task_struct *tsk = current; struct mm_struct *mm = tsk->mm; int si_code = SEGV_MAPERR; + unsigned int write = 0, exec = 0, mask; vm_fault_t fault; - int write = regs->ecr_cause & ECR_C_PROTV_STORE; /* ST/EX */ - unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; + unsigned int flags; /* * NOTE! We MUST NOT take any locks for this case. We may @@ -88,8 +88,18 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) if (faulthandler_disabled() || !mm) goto no_context; + if (regs->ecr_cause & ECR_C_PROTV_STORE) /* ST/EX */ + write = 1; + else if ((regs->ecr_vec == ECR_V_PROTV) && + (regs->ecr_cause == ECR_C_PROTV_INST_FETCH)) + exec = 1; + + flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; if (user_mode(regs)) flags |= FAULT_FLAG_USER; + if (write) + flags |= FAULT_FLAG_WRITE; + retry: down_read(&mm->mmap_sem); @@ -102,24 +112,17 @@ retry: } /* - * Ok, we have a good vm_area for this memory access, so - * we can handle it.. + * vm_area is good, now check permissions for this memory access */ - si_code = SEGV_ACCERR; - - /* Handle protection violation, execute on heap or stack */ - - if ((regs->ecr_vec == ECR_V_PROTV) && - (regs->ecr_cause == ECR_C_PROTV_INST_FETCH)) + mask = VM_READ; + if (write) + mask = VM_WRITE; + if (exec) + mask = VM_EXEC; + + if (!(vma->vm_flags & mask)) { + si_code = SEGV_ACCERR; goto bad_area; - - if (write) { - if (!(vma->vm_flags & VM_WRITE)) - goto bad_area; - flags |= FAULT_FLAG_WRITE; - } else { - if (!(vma->vm_flags & (VM_READ | VM_EXEC))) - goto bad_area; } /* -- cgit v1.2.3 From 02c88d142ea6e64b0f81dcf3687a889d8a3556ba Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 14 May 2019 14:35:45 -0700 Subject: ARC: mm: do_page_fault refactor #4: consolidate retry related logic stats update code can now elide "retry" check and additional level of indentation since all retry handling is done ahead of it already Signed-off-by: Vineet Gupta --- arch/arc/mm/fault.c | 60 +++++++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 29 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 8c7c81ce7f6a..4597b4886edd 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -65,8 +65,8 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) struct mm_struct *mm = tsk->mm; int si_code = SEGV_MAPERR; unsigned int write = 0, exec = 0, mask; - vm_fault_t fault; - unsigned int flags; + vm_fault_t fault; /* handle_mm_fault() output */ + unsigned int flags; /* handle_mm_fault() input */ /* * NOTE! We MUST NOT take any locks for this case. We may @@ -125,49 +125,51 @@ retry: goto bad_area; } - /* - * If for any reason at all we couldn't handle the fault, - * make sure we exit gracefully rather than endlessly redo - * the fault. - */ fault = handle_mm_fault(vma, address, flags); - if (fatal_signal_pending(current)) { + /* + * Fault retry nuances + */ + if (unlikely(fault & VM_FAULT_RETRY)) { /* - * if fault retry, mmap_sem already relinquished by core mm - * so OK to return to user mode (with signal handled first) + * If fault needs to be retried, handle any pending signals + * first (by returning to user mode). + * mmap_sem already relinquished by core mm for RETRY case */ - if (fault & VM_FAULT_RETRY) { + if (fatal_signal_pending(current)) { if (!user_mode(regs)) goto no_context; return; } + /* + * retry state machine + */ + if (flags & FAULT_FLAG_ALLOW_RETRY) { + flags &= ~FAULT_FLAG_ALLOW_RETRY; + flags |= FAULT_FLAG_TRIED; + goto retry; + } } + /* + * Major/minor page fault accounting + * (in case of retry we only land here once) + */ perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address); if (likely(!(fault & VM_FAULT_ERROR))) { - if (flags & FAULT_FLAG_ALLOW_RETRY) { - /* To avoid updating stats twice for retry case */ - if (fault & VM_FAULT_MAJOR) { - tsk->maj_flt++; - perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MAJ, 1, - regs, address); - } else { - tsk->min_flt++; - perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MIN, 1, - regs, address); - } - - if (fault & VM_FAULT_RETRY) { - flags &= ~FAULT_FLAG_ALLOW_RETRY; - flags |= FAULT_FLAG_TRIED; - goto retry; - } + if (fault & VM_FAULT_MAJOR) { + tsk->maj_flt++; + perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MAJ, 1, + regs, address); + } else { + tsk->min_flt++; + perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MIN, 1, + regs, address); } - /* Fault Handled Gracefully */ + /* Normal return path: fault Handled Gracefully */ up_read(&mm->mmap_sem); return; } -- cgit v1.2.3 From d0542c7eacd5b507fa53570b610706df122a2f37 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 14 May 2019 14:45:44 -0700 Subject: ARC: mm: do_page_fault refactor #5: scoot no_context to end This is different than the rest of signal handling stuff No functional change Signed-off-by: Vineet Gupta --- arch/arc/mm/fault.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 4597b4886edd..b107e45cce94 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -198,20 +198,6 @@ bad_area: return; } -no_context: - /* Are we prepared to handle this kernel fault? - * - * (The kernel has valid exception-points in the source - * when it accesses user-memory. When it fails in one - * of those points, we find it in a table and do a jump - * to some fixup code that loads an appropriate error - * code) - */ - if (fixup_exception(regs)) - return; - - die("Oops", regs, address); - out_of_memory: up_read(&mm->mmap_sem); @@ -230,4 +216,11 @@ do_sigbus: tsk->thread.fault_address = address; force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)address, tsk); + return; + +no_context: + if (fixup_exception(regs)) + return; + + die("Oops", regs, address); } -- cgit v1.2.3 From 98cb57ad70fb7c8a9c030d3e83fe66b546906e28 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 14 May 2019 15:10:45 -0700 Subject: ARC: mm: do_page_fault refactor #6: error handlers to use same pattern - up_read - if !user_mode - whatever error handling Signed-off-by: Vineet Gupta --- arch/arc/mm/fault.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index b107e45cce94..2672ce24d741 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -191,22 +191,21 @@ retry: bad_area: up_read(&mm->mmap_sem); - /* User mode accesses just cause a SIGSEGV */ - if (user_mode(regs)) { - tsk->thread.fault_address = address; - force_sig_fault(SIGSEGV, si_code, (void __user *)address, tsk); - return; - } + if (!user_mode(regs)) + goto no_context; + + tsk->thread.fault_address = address; + force_sig_fault(SIGSEGV, si_code, (void __user *)address, tsk); + return; out_of_memory: up_read(&mm->mmap_sem); - if (user_mode(regs)) { - pagefault_out_of_memory(); - return; - } + if (!user_mode(regs)) + goto no_context; - goto no_context; + pagefault_out_of_memory(); + return; do_sigbus: up_read(&mm->mmap_sem); -- cgit v1.2.3 From 5e91bf5ce9b8740076f5283f1ec3a5b023950920 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 14 May 2019 15:55:31 -0700 Subject: ARC: mm: do_page_fault refactor #7: fold the various error handling - single up_read() call vs. 4 - so much easier on eyes Technically it seems like @bad_area label moved up, but even in old regime, it was a special case of delivering SIGSEGV unconditionally which we now do as well, although with checks. Also note that @fault needs to be initialized since we can land in @bad_area (which reads it) without setting it up with return value of handle_mm_fault() - failing to do so did bite us although as a side effect of different patch: see [1] [1]: http://lists.infradead.org/pipermail/linux-snps-arc/2019-May/005803.html Signed-off-by: Vineet Gupta --- arch/arc/mm/fault.c | 48 ++++++++++++++---------------------------------- 1 file changed, 14 insertions(+), 34 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 2672ce24d741..6a78a2d776a9 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -63,9 +63,9 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) struct vm_area_struct *vma = NULL; struct task_struct *tsk = current; struct mm_struct *mm = tsk->mm; - int si_code = SEGV_MAPERR; + int sig, si_code = SEGV_MAPERR; unsigned int write = 0, exec = 0, mask; - vm_fault_t fault; /* handle_mm_fault() output */ + vm_fault_t fault = VM_FAULT_SIGSEGV; /* handle_mm_fault() output */ unsigned int flags; /* handle_mm_fault() input */ /* @@ -174,47 +174,27 @@ retry: return; } - if (fault & VM_FAULT_OOM) - goto out_of_memory; - else if (fault & VM_FAULT_SIGSEGV) - goto bad_area; - else if (fault & VM_FAULT_SIGBUS) - goto do_sigbus; - - /* no man's land */ - BUG(); - - /* - * Something tried to access memory that isn't in our memory map.. - * Fix it, but check if it's kernel or user first.. - */ bad_area: up_read(&mm->mmap_sem); if (!user_mode(regs)) goto no_context; - tsk->thread.fault_address = address; - force_sig_fault(SIGSEGV, si_code, (void __user *)address, tsk); - return; - -out_of_memory: - up_read(&mm->mmap_sem); - - if (!user_mode(regs)) - goto no_context; - - pagefault_out_of_memory(); - return; - -do_sigbus: - up_read(&mm->mmap_sem); + if (fault & VM_FAULT_OOM) { + pagefault_out_of_memory(); + return; + } - if (!user_mode(regs)) - goto no_context; + if (fault & VM_FAULT_SIGBUS) { + sig = SIGBUS; + si_code = BUS_ADRERR; + } + else { + sig = SIGSEGV; + } tsk->thread.fault_address = address; - force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)address, tsk); + force_sig_fault(sig, si_code, (void __user *)address, tsk); return; no_context: -- cgit v1.2.3 From 926150db8558dca59617c8786c3f91c239290ee1 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 14 May 2019 16:28:30 -0700 Subject: ARC: mm: do_page_fault refactor #8: release mmap_sem sooner In case of successful page fault handling, this patch releases mmap_sem before updating the perf stat event for major/minor faults. So even though the contention reduction is NOT super high, it is still an improvement. There's an additional code size improvement as we only have 2 up_read() calls now. Note to myself: -------------- 1. Given the way it is done, we are forced to move @bad_area label earlier causing the various "goto bad_area" cases to hit perf stat code. - PERF_COUNT_SW_PAGE_FAULTS is NOW updated for access errors which is what arm/arm64 seem to be doing as well (with slightly different code) - PERF_COUNT_SW_PAGE_FAULTS_{MAJ,MIN} must NOT be updated for the error case which is guarded by now setting @fault initial value to VM_FAULT_ERROR which serves both cases when handle_mm_fault() returns error or is not called at all. 2. arm/arm64 use two homebrew fault flags VM_FAULT_BAD{MAP,MAPACCESS} which I was inclined to add too but seems not needed for ARC - given that we have everything is 1 function we can still use goto - we setup si_code at the right place (arm* do that in the end) - we init fault already to error value which guards entry into perf stats event update Cc: Peter Zijlstra Signed-off-by: Vineet Gupta --- arch/arc/mm/fault.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 6a78a2d776a9..e7df5ef3877a 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -152,6 +152,9 @@ retry: } } +bad_area: + up_read(&mm->mmap_sem); + /* * Major/minor page fault accounting * (in case of retry we only land here once) @@ -170,13 +173,9 @@ retry: } /* Normal return path: fault Handled Gracefully */ - up_read(&mm->mmap_sem); return; } -bad_area: - up_read(&mm->mmap_sem); - if (!user_mode(regs)) goto no_context; -- cgit v1.2.3 From 23c0cbd0c75c3b564850294427fd2be2bc2a015b Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 9 Apr 2019 19:16:37 -0700 Subject: ARCv2: entry: push out the Z flag unclobber from common EXCEPTION_PROLOGUE Upon a taken interrupt/exception from User mode, HS hardware auto sets Z flag. This helps shave a few instructions from EXCEPTION_PROLOGUE by eliding re-reading ERSTATUS and some bit fiddling. However TLB Miss Exception handler can clobber the CPU flags and still end up in EXCEPTION_PROLOGUE in the slow path handling TLB handling case: EV_TLBMissD do_slow_path_pf EV_TLBProtV (aliased to call_do_page_fault) EXCEPTION_PROLOGUE As a result, EXCEPTION_PROLOGUE need to "unclobber" the Z flag which this patch changes. It is now pushed out to TLB Miss Exception handler. The reasons beings: - The flag restoration is only needed for slowpath TLB Miss Exception handling, but currently being in EXCEPTION_PROLOGUE penalizes all exceptions such as ProtV and syscall Trap, where Z flag is already as expected. - Pushing unclobber out to where it was clobbered is much cleaner and also serves to document the fact. - Makes EXCEPTION_PROLGUE similar to INTERRUPT_PROLOGUE so easier to refactor the common parts which is what this series aims to do Signed-off-by: Vineet Gupta --- arch/arc/include/asm/entry-arcv2.h | 8 -------- arch/arc/mm/tlbex.S | 11 +++++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) (limited to 'arch/arc/mm') diff --git a/arch/arc/include/asm/entry-arcv2.h b/arch/arc/include/asm/entry-arcv2.h index 1c3520d1fa42..3209a6762960 100644 --- a/arch/arc/include/asm/entry-arcv2.h +++ b/arch/arc/include/asm/entry-arcv2.h @@ -225,14 +225,6 @@ ; -- for interrupts, regs above are auto-saved by h/w in that order -- ; Now do what ISR prologue does (manually save r12, sp, fp, gp, r25) - ; - ; Set Z flag if this was from U mode (expected by INTERRUPT_PROLOGUE) - ; Although H/w exception micro-ops do set Z flag for U mode (just like - ; for interrupts), it could get clobbered in case we soft land here from - ; a TLB Miss exception handler (tlbex.S) - - and r10, r10, STATUS_U_MASK - xor.f 0, r10, STATUS_U_MASK INTERRUPT_PROLOGUE exception diff --git a/arch/arc/mm/tlbex.S b/arch/arc/mm/tlbex.S index 471a97bf492d..c55d95dd2f39 100644 --- a/arch/arc/mm/tlbex.S +++ b/arch/arc/mm/tlbex.S @@ -393,6 +393,17 @@ EV_TLBMissD_fast_ret: ; additional label for VDK OS-kit instrumentation ;-------- Common routine to call Linux Page Fault Handler ----------- do_slow_path_pf: +#ifdef CONFIG_ISA_ARCV2 + ; Set Z flag if exception in U mode. Hardware micro-ops do this on any + ; taken interrupt/exception, and thus is already the case at the entry + ; above, but ensuing code would have already clobbered. + ; EXCEPTION_PROLOGUE called in slow path, relies on correct Z flag set + + lr r2, [erstatus] + and r2, r2, STATUS_U_MASK + bxor.f 0, r2, STATUS_U_BIT +#endif + ; Restore the 4-scratch regs saved by fast path miss handler TLBMISS_RESTORE_REGS -- cgit v1.2.3