All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups
@ 2020-08-25 18:38 Pekka Enberg
  2020-08-25 18:39 ` [PATCH 1/8] riscv/mm/fault: Move no context handling to no_context() Pekka Enberg
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:38 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

This patch series cleans up the do_page_fault() function by replacing
gotos with function calls, similar to what the x86 architecture does.
The motivation for this series is to make thepage fault handling code
easier to read and reason about.

The use of gotos has no advantage in kernel code size either. In fact,
the bloatometer script shows a tiny decrease in kernel text size:

  add/remove: 1/0 grow/shrink: 0/2 up/down: 96/-99 (-3)
  Function                                     old     new   delta
  no_context.part                                -      96     +96
  __func__                                      14      13      -1
  do_page_fault                                778     680     -98
  Total: Before=792, After=789, chg -0.38%

Please note tht x86 also moves these functions out-of-line with the
"noinline" annotation, which supposedly decreases stack usage in
do_page_fault() at the expense of slighly larger kernel code size.

However, in my testing, I was able to reduce stack usage at maximum by
16 bytes, at the expense of much larger kernel code size, so I am
keeping the functions inline, and letting the compiler do its job.

The patch series has been tested on QEMU.

You can pull the series from:

  git@github.com:penberg/linux.git penberg/riscv/mm-fault-cleanups

Pekka Enberg (8):
  riscv/mm/fault: Move no context handling to no_context()
  riscv/mm/fault: Move bad area handling to bad_area()
  riscv/mm/fault: Move vmalloc fault handling to vmalloc_fault()
  riscv/mm/fault: Simplify fault error handling
  riscv/mm/fault: Move fault error handling to mm_fault_error()
  riscv/mm/fault: Simplify mm_fault_error()
  riscv/mm/fault: Move FAULT_FLAG_WRITE handling in do_page_fault()
  riscv/mm/fault: Move access error check to function

 arch/riscv/mm/fault.c | 337 +++++++++++++++++++++++-------------------
 1 file changed, 189 insertions(+), 148 deletions(-)

-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/8] riscv/mm/fault: Move no context handling to no_context()
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
@ 2020-08-25 18:39 ` Pekka Enberg
  2020-08-25 18:39 ` [PATCH 2/8] riscv/mm/fault: Move bad area handling to bad_area() Pekka Enberg
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:39 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

This patch moves the no context handling in do_page_fault() to
no_context() function and converts gotos to calls to the new function.
---
 arch/riscv/mm/fault.c | 83 +++++++++++++++++++++++++++----------------
 1 file changed, 52 insertions(+), 31 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index f5c2e4a249eb..1612552478c5 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -19,6 +19,24 @@
 
 #include "../kernel/head.h"
 
+static inline void no_context(struct pt_regs *regs, unsigned long addr)
+{
+	/* Are we prepared to handle this kernel fault? */
+	if (fixup_exception(regs))
+		return;
+
+	/*
+	 * Oops. The kernel tried to access some bad page. We'll have to
+	 * terminate things with extreme prejudice.
+	 */
+	bust_spinlocks(1);
+	pr_alert("Unable to handle kernel %s at virtual address " REG_FMT "\n",
+		(addr < PAGE_SIZE) ? "NULL pointer dereference" :
+		"paging request", addr);
+	die(regs, "Oops");
+	do_exit(SIGKILL);
+}
+
 /*
  * This routine handles page faults.  It determines the address and the
  * problem, and then passes it off to one of the appropriate routines.
@@ -59,8 +77,10 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	 * If we're in an interrupt, have no user context, or are running
 	 * in an atomic region, then we must not take the fault.
 	 */
-	if (unlikely(faulthandler_disabled() || !mm))
-		goto no_context;
+	if (unlikely(faulthandler_disabled() || !mm)) {
+		no_context(regs, addr);
+		return;
+	}
 
 	if (user_mode(regs))
 		flags |= FAULT_FLAG_USER;
@@ -153,21 +173,8 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 		return;
 	}
 
-no_context:
-	/* Are we prepared to handle this kernel fault? */
-	if (fixup_exception(regs))
-		return;
-
-	/*
-	 * Oops. The kernel tried to access some bad page. We'll have to
-	 * terminate things with extreme prejudice.
-	 */
-	bust_spinlocks(1);
-	pr_alert("Unable to handle kernel %s at virtual address " REG_FMT "\n",
-		(addr < PAGE_SIZE) ? "NULL pointer dereference" :
-		"paging request", addr);
-	die(regs, "Oops");
-	do_exit(SIGKILL);
+	no_context(regs, addr);
+	return;
 
 	/*
 	 * We ran out of memory, call the OOM killer, and return the userspace
@@ -175,16 +182,20 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	 */
 out_of_memory:
 	mmap_read_unlock(mm);
-	if (!user_mode(regs))
-		goto no_context;
+	if (!user_mode(regs)) {
+		no_context(regs, addr);
+		return;
+	}
 	pagefault_out_of_memory();
 	return;
 
 do_sigbus:
 	mmap_read_unlock(mm);
 	/* Kernel mode? Handle exceptions or die */
-	if (!user_mode(regs))
-		goto no_context;
+	if (!user_mode(regs)) {
+		no_context(regs, addr);
+		return;
+	}
 	do_trap(regs, SIGBUS, BUS_ADRERR, addr);
 	return;
 
@@ -213,19 +224,25 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 		pgd = (pgd_t *)pfn_to_virt(csr_read(CSR_SATP)) + index;
 		pgd_k = init_mm.pgd + index;
 
-		if (!pgd_present(*pgd_k))
-			goto no_context;
+		if (!pgd_present(*pgd_k)) {
+			no_context(regs, addr);
+			return;
+		}
 		set_pgd(pgd, *pgd_k);
 
 		p4d = p4d_offset(pgd, addr);
 		p4d_k = p4d_offset(pgd_k, addr);
-		if (!p4d_present(*p4d_k))
-			goto no_context;
+		if (!p4d_present(*p4d_k)) {
+			no_context(regs, addr);
+			return;
+		}
 
 		pud = pud_offset(p4d, addr);
 		pud_k = pud_offset(p4d_k, addr);
-		if (!pud_present(*pud_k))
-			goto no_context;
+		if (!pud_present(*pud_k)) {
+			no_context(regs, addr);
+			return;
+		}
 
 		/*
 		 * Since the vmalloc area is global, it is unnecessary
@@ -233,8 +250,10 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 		 */
 		pmd = pmd_offset(pud, addr);
 		pmd_k = pmd_offset(pud_k, addr);
-		if (!pmd_present(*pmd_k))
-			goto no_context;
+		if (!pmd_present(*pmd_k)) {
+			no_context(regs, addr);
+			return;
+		}
 		set_pmd(pmd, *pmd_k);
 
 		/*
@@ -244,8 +263,10 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 		 * silently loop forever.
 		 */
 		pte_k = pte_offset_kernel(pmd_k, addr);
-		if (!pte_present(*pte_k))
-			goto no_context;
+		if (!pte_present(*pte_k)) {
+			no_context(regs, addr);
+			return;
+		}
 
 		/*
 		 * The kernel assumes that TLBs don't cache invalid
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/8] riscv/mm/fault: Move bad area handling to bad_area()
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
  2020-08-25 18:39 ` [PATCH 1/8] riscv/mm/fault: Move no context handling to no_context() Pekka Enberg
@ 2020-08-25 18:39 ` Pekka Enberg
  2020-08-25 18:39 ` [PATCH 3/8] riscv/mm/fault: Move vmalloc fault handling to vmalloc_fault() Pekka Enberg
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:39 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

This patch moves the bad area handling in do_page_fault() to bad_area()
function and converts gotos to calls to the new function.
---
 arch/riscv/mm/fault.c | 67 ++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 1612552478c5..ac9a99255365 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -37,6 +37,22 @@ static inline void no_context(struct pt_regs *regs, unsigned long addr)
 	do_exit(SIGKILL);
 }
 
+static inline void bad_area(struct pt_regs *regs, struct mm_struct *mm, int code, unsigned long addr)
+{
+	/*
+	 * Something tried to access memory that isn't in our memory map.
+	 * Fix it, but check if it's kernel or user first.
+	 */
+	mmap_read_unlock(mm);
+	/* User mode accesses just cause a SIGSEGV */
+	if (user_mode(regs)) {
+		do_trap(regs, SIGSEGV, code, addr);
+		return;
+	}
+
+	no_context(regs, addr);
+}
+
 /*
  * This routine handles page faults.  It determines the address and the
  * problem, and then passes it off to one of the appropriate routines.
@@ -90,14 +106,20 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 retry:
 	mmap_read_lock(mm);
 	vma = find_vma(mm, addr);
-	if (unlikely(!vma))
-		goto bad_area;
+	if (unlikely(!vma)) {
+		bad_area(regs, mm, code, addr);
+		return;
+	}
 	if (likely(vma->vm_start <= addr))
 		goto good_area;
-	if (unlikely(!(vma->vm_flags & VM_GROWSDOWN)))
-		goto bad_area;
-	if (unlikely(expand_stack(vma, addr)))
-		goto bad_area;
+	if (unlikely(!(vma->vm_flags & VM_GROWSDOWN))) {
+		bad_area(regs, mm, code, addr);
+		return;
+	}
+	if (unlikely(expand_stack(vma, addr))) {
+		bad_area(regs, mm, code, addr);
+		return;
+	}
 
 	/*
 	 * Ok, we have a good vm_area for this memory access, so
@@ -108,16 +130,22 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 
 	switch (cause) {
 	case EXC_INST_PAGE_FAULT:
-		if (!(vma->vm_flags & VM_EXEC))
-			goto bad_area;
+		if (!(vma->vm_flags & VM_EXEC)) {
+			bad_area(regs, mm, code, addr);
+			return;
+		}
 		break;
 	case EXC_LOAD_PAGE_FAULT:
-		if (!(vma->vm_flags & VM_READ))
-			goto bad_area;
+		if (!(vma->vm_flags & VM_READ)) {
+			bad_area(regs, mm, code, addr);
+			return;
+		}
 		break;
 	case EXC_STORE_PAGE_FAULT:
-		if (!(vma->vm_flags & VM_WRITE))
-			goto bad_area;
+		if (!(vma->vm_flags & VM_WRITE)) {
+			bad_area(regs, mm, code, addr);
+			return;
+		}
 		flags |= FAULT_FLAG_WRITE;
 		break;
 	default:
@@ -161,21 +189,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	mmap_read_unlock(mm);
 	return;
 
-	/*
-	 * 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:
-	mmap_read_unlock(mm);
-	/* User mode accesses just cause a SIGSEGV */
-	if (user_mode(regs)) {
-		do_trap(regs, SIGSEGV, code, addr);
-		return;
-	}
-
-	no_context(regs, addr);
-	return;
-
 	/*
 	 * We ran out of memory, call the OOM killer, and return the userspace
 	 * (which will retry the fault, or kill us if we got oom-killed).
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/8] riscv/mm/fault: Move vmalloc fault handling to vmalloc_fault()
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
  2020-08-25 18:39 ` [PATCH 1/8] riscv/mm/fault: Move no context handling to no_context() Pekka Enberg
  2020-08-25 18:39 ` [PATCH 2/8] riscv/mm/fault: Move bad area handling to bad_area() Pekka Enberg
@ 2020-08-25 18:39 ` Pekka Enberg
  2020-08-25 18:39 ` [PATCH 4/8] riscv/mm/fault: Simplify fault error handling Pekka Enberg
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:39 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

This patch moves the vmalloc fault handling in do_page_fault() to
vmalloc_fault() function and converts gotos to calls to the new
function.
---
 arch/riscv/mm/fault.c | 164 +++++++++++++++++++++---------------------
 1 file changed, 82 insertions(+), 82 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index ac9a99255365..460ea1d6c24e 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -53,6 +53,84 @@ static inline void bad_area(struct pt_regs *regs, struct mm_struct *mm, int code
 	no_context(regs, addr);
 }
 
+static void inline vmalloc_fault(struct pt_regs *regs, int code, unsigned long addr)
+{
+	pgd_t *pgd, *pgd_k;
+	pud_t *pud, *pud_k;
+	p4d_t *p4d, *p4d_k;
+	pmd_t *pmd, *pmd_k;
+	pte_t *pte_k;
+	int index;
+
+	/* User mode accesses just cause a SIGSEGV */
+	if (user_mode(regs))
+		return do_trap(regs, SIGSEGV, code, addr);
+
+	/*
+	 * Synchronize this task's top level page-table
+	 * with the 'reference' page table.
+	 *
+	 * Do _not_ use "tsk->active_mm->pgd" here.
+	 * We might be inside an interrupt in the middle
+	 * of a task switch.
+	 */
+	index = pgd_index(addr);
+	pgd = (pgd_t *)pfn_to_virt(csr_read(CSR_SATP)) + index;
+	pgd_k = init_mm.pgd + index;
+
+	if (!pgd_present(*pgd_k)) {
+		no_context(regs, addr);
+		return;
+	}
+	set_pgd(pgd, *pgd_k);
+
+	p4d = p4d_offset(pgd, addr);
+	p4d_k = p4d_offset(pgd_k, addr);
+	if (!p4d_present(*p4d_k)) {
+		no_context(regs, addr);
+		return;
+	}
+
+	pud = pud_offset(p4d, addr);
+	pud_k = pud_offset(p4d_k, addr);
+	if (!pud_present(*pud_k)) {
+		no_context(regs, addr);
+		return;
+	}
+
+	/*
+	 * Since the vmalloc area is global, it is unnecessary
+	 * to copy individual PTEs
+	 */
+	pmd = pmd_offset(pud, addr);
+	pmd_k = pmd_offset(pud_k, addr);
+	if (!pmd_present(*pmd_k)) {
+		no_context(regs, addr);
+		return;
+	}
+	set_pmd(pmd, *pmd_k);
+
+	/*
+	 * Make sure the actual PTE exists as well to
+	 * catch kernel vmalloc-area accesses to non-mapped
+	 * addresses. If we don't do this, this will just
+	 * silently loop forever.
+	 */
+	pte_k = pte_offset_kernel(pmd_k, addr);
+	if (!pte_present(*pte_k)) {
+		no_context(regs, addr);
+		return;
+	}
+
+	/*
+	 * The kernel assumes that TLBs don't cache invalid
+	 * entries, but in RISC-V, SFENCE.VMA specifies an
+	 * ordering constraint, not a cache flush; it is
+	 * necessary even after writing invalid entries.
+	 */
+	local_flush_tlb_page(addr);
+}
+
 /*
  * This routine handles page faults.  It determines the address and the
  * problem, and then passes it off to one of the appropriate routines.
@@ -82,8 +160,10 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	 * only copy the information from the master page table,
 	 * nothing more.
 	 */
-	if (unlikely((addr >= VMALLOC_START) && (addr <= VMALLOC_END)))
-		goto vmalloc_fault;
+	if (unlikely((addr >= VMALLOC_START) && (addr <= VMALLOC_END))) {
+		vmalloc_fault(regs, code, addr);
+		return;
+	}
 
 	/* Enable interrupts if they were enabled in the parent context. */
 	if (likely(regs->status & SR_PIE))
@@ -211,84 +291,4 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	}
 	do_trap(regs, SIGBUS, BUS_ADRERR, addr);
 	return;
-
-vmalloc_fault:
-	{
-		pgd_t *pgd, *pgd_k;
-		pud_t *pud, *pud_k;
-		p4d_t *p4d, *p4d_k;
-		pmd_t *pmd, *pmd_k;
-		pte_t *pte_k;
-		int index;
-
-		/* User mode accesses just cause a SIGSEGV */
-		if (user_mode(regs))
-			return do_trap(regs, SIGSEGV, code, addr);
-
-		/*
-		 * Synchronize this task's top level page-table
-		 * with the 'reference' page table.
-		 *
-		 * Do _not_ use "tsk->active_mm->pgd" here.
-		 * We might be inside an interrupt in the middle
-		 * of a task switch.
-		 */
-		index = pgd_index(addr);
-		pgd = (pgd_t *)pfn_to_virt(csr_read(CSR_SATP)) + index;
-		pgd_k = init_mm.pgd + index;
-
-		if (!pgd_present(*pgd_k)) {
-			no_context(regs, addr);
-			return;
-		}
-		set_pgd(pgd, *pgd_k);
-
-		p4d = p4d_offset(pgd, addr);
-		p4d_k = p4d_offset(pgd_k, addr);
-		if (!p4d_present(*p4d_k)) {
-			no_context(regs, addr);
-			return;
-		}
-
-		pud = pud_offset(p4d, addr);
-		pud_k = pud_offset(p4d_k, addr);
-		if (!pud_present(*pud_k)) {
-			no_context(regs, addr);
-			return;
-		}
-
-		/*
-		 * Since the vmalloc area is global, it is unnecessary
-		 * to copy individual PTEs
-		 */
-		pmd = pmd_offset(pud, addr);
-		pmd_k = pmd_offset(pud_k, addr);
-		if (!pmd_present(*pmd_k)) {
-			no_context(regs, addr);
-			return;
-		}
-		set_pmd(pmd, *pmd_k);
-
-		/*
-		 * Make sure the actual PTE exists as well to
-		 * catch kernel vmalloc-area accesses to non-mapped
-		 * addresses. If we don't do this, this will just
-		 * silently loop forever.
-		 */
-		pte_k = pte_offset_kernel(pmd_k, addr);
-		if (!pte_present(*pte_k)) {
-			no_context(regs, addr);
-			return;
-		}
-
-		/*
-		 * The kernel assumes that TLBs don't cache invalid
-		 * entries, but in RISC-V, SFENCE.VMA specifies an
-		 * ordering constraint, not a cache flush; it is
-		 * necessary even after writing invalid entries.
-		 */
-		local_flush_tlb_page(addr);
-
-		return;
-	}
 }
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/8] riscv/mm/fault: Simplify fault error handling
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
                   ` (2 preceding siblings ...)
  2020-08-25 18:39 ` [PATCH 3/8] riscv/mm/fault: Move vmalloc fault handling to vmalloc_fault() Pekka Enberg
@ 2020-08-25 18:39 ` Pekka Enberg
  2020-08-25 18:39 ` [PATCH 5/8] riscv/mm/fault: Move fault error handling to mm_fault_error() Pekka Enberg
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:39 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

Move fault error handling after retry logic. This simplifies the code
flow and makes it easier to move fault error handling to its own
function.
---
 arch/riscv/mm/fault.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 460ea1d6c24e..bfb40927cb7a 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -247,14 +247,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	if (fault_signal_pending(fault, regs))
 		return;
 
-	if (unlikely(fault & VM_FAULT_ERROR)) {
-		if (fault & VM_FAULT_OOM)
-			goto out_of_memory;
-		else if (fault & VM_FAULT_SIGBUS)
-			goto do_sigbus;
-		BUG();
-	}
-
 	if (unlikely((fault & VM_FAULT_RETRY) && (flags & FAULT_FLAG_ALLOW_RETRY))) {
 		flags |= FAULT_FLAG_TRIED;
 
@@ -267,6 +259,14 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	}
 
 	mmap_read_unlock(mm);
+
+	if (unlikely(fault & VM_FAULT_ERROR)) {
+		if (fault & VM_FAULT_OOM)
+			goto out_of_memory;
+		else if (fault & VM_FAULT_SIGBUS)
+			goto do_sigbus;
+		BUG();
+	}
 	return;
 
 	/*
@@ -274,7 +274,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	 * (which will retry the fault, or kill us if we got oom-killed).
 	 */
 out_of_memory:
-	mmap_read_unlock(mm);
 	if (!user_mode(regs)) {
 		no_context(regs, addr);
 		return;
@@ -283,7 +282,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	return;
 
 do_sigbus:
-	mmap_read_unlock(mm);
 	/* Kernel mode? Handle exceptions or die */
 	if (!user_mode(regs)) {
 		no_context(regs, addr);
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/8] riscv/mm/fault: Move fault error handling to mm_fault_error()
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
                   ` (3 preceding siblings ...)
  2020-08-25 18:39 ` [PATCH 4/8] riscv/mm/fault: Simplify fault error handling Pekka Enberg
@ 2020-08-25 18:39 ` Pekka Enberg
  2020-08-25 18:39 ` [PATCH 6/8] riscv/mm/fault: Simplify mm_fault_error() Pekka Enberg
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:39 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

This patch moves the fault error handling to mm_fault_error() function
and converts gotos to calls to the new function.
---
 arch/riscv/mm/fault.c | 56 ++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index bfb40927cb7a..49b190d0c088 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -37,6 +37,36 @@ static inline void no_context(struct pt_regs *regs, unsigned long addr)
 	do_exit(SIGKILL);
 }
 
+static inline void mm_fault_error(struct pt_regs *regs, unsigned long addr, vm_fault_t fault)
+{
+	if (fault & VM_FAULT_OOM)
+		goto out_of_memory;
+	else if (fault & VM_FAULT_SIGBUS)
+		goto do_sigbus;
+	BUG();
+
+	/*
+	 * We ran out of memory, call the OOM killer, and return the userspace
+	 * (which will retry the fault, or kill us if we got oom-killed).
+	 */
+out_of_memory:
+	if (!user_mode(regs)) {
+		no_context(regs, addr);
+		return;
+	}
+	pagefault_out_of_memory();
+	return;
+
+do_sigbus:
+	/* Kernel mode? Handle exceptions or die */
+	if (!user_mode(regs)) {
+		no_context(regs, addr);
+		return;
+	}
+	do_trap(regs, SIGBUS, BUS_ADRERR, addr);
+	return;
+}
+
 static inline void bad_area(struct pt_regs *regs, struct mm_struct *mm, int code, unsigned long addr)
 {
 	/*
@@ -261,32 +291,8 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	mmap_read_unlock(mm);
 
 	if (unlikely(fault & VM_FAULT_ERROR)) {
-		if (fault & VM_FAULT_OOM)
-			goto out_of_memory;
-		else if (fault & VM_FAULT_SIGBUS)
-			goto do_sigbus;
-		BUG();
-	}
-	return;
-
-	/*
-	 * We ran out of memory, call the OOM killer, and return the userspace
-	 * (which will retry the fault, or kill us if we got oom-killed).
-	 */
-out_of_memory:
-	if (!user_mode(regs)) {
-		no_context(regs, addr);
+		mm_fault_error(regs, addr, fault);
 		return;
 	}
-	pagefault_out_of_memory();
-	return;
-
-do_sigbus:
-	/* Kernel mode? Handle exceptions or die */
-	if (!user_mode(regs)) {
-		no_context(regs, addr);
-		return;
-	}
-	do_trap(regs, SIGBUS, BUS_ADRERR, addr);
 	return;
 }
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/8] riscv/mm/fault: Simplify mm_fault_error()
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
                   ` (4 preceding siblings ...)
  2020-08-25 18:39 ` [PATCH 5/8] riscv/mm/fault: Move fault error handling to mm_fault_error() Pekka Enberg
@ 2020-08-25 18:39 ` Pekka Enberg
  2020-08-25 18:39 ` [PATCH 7/8] riscv/mm/fault: Move FAULT_FLAG_WRITE handling in do_page_fault() Pekka Enberg
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:39 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

Simplify the mm_fault_error() handling function by eliminating the
unnecessary gotos.
---
 arch/riscv/mm/fault.c | 41 ++++++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 49b190d0c088..3b430fb18de3 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -39,32 +39,27 @@ static inline void no_context(struct pt_regs *regs, unsigned long addr)
 
 static inline void mm_fault_error(struct pt_regs *regs, unsigned long addr, vm_fault_t fault)
 {
-	if (fault & VM_FAULT_OOM)
-		goto out_of_memory;
-	else if (fault & VM_FAULT_SIGBUS)
-		goto do_sigbus;
-	BUG();
-
-	/*
-	 * We ran out of memory, call the OOM killer, and return the userspace
-	 * (which will retry the fault, or kill us if we got oom-killed).
-	 */
-out_of_memory:
-	if (!user_mode(regs)) {
-		no_context(regs, addr);
+	if (fault & VM_FAULT_OOM) {
+		/*
+		 * We ran out of memory, call the OOM killer, and return the userspace
+		 * (which will retry the fault, or kill us if we got oom-killed).
+		 */
+		if (!user_mode(regs)) {
+			no_context(regs, addr);
+			return;
+		}
+		pagefault_out_of_memory();
 		return;
-	}
-	pagefault_out_of_memory();
-	return;
-
-do_sigbus:
-	/* Kernel mode? Handle exceptions or die */
-	if (!user_mode(regs)) {
-		no_context(regs, addr);
+	} else if (fault & VM_FAULT_SIGBUS) {
+		/* Kernel mode? Handle exceptions or die */
+		if (!user_mode(regs)) {
+			no_context(regs, addr);
+			return;
+		}
+		do_trap(regs, SIGBUS, BUS_ADRERR, addr);
 		return;
 	}
-	do_trap(regs, SIGBUS, BUS_ADRERR, addr);
-	return;
+	BUG();
 }
 
 static inline void bad_area(struct pt_regs *regs, struct mm_struct *mm, int code, unsigned long addr)
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/8] riscv/mm/fault: Move FAULT_FLAG_WRITE handling in do_page_fault()
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
                   ` (5 preceding siblings ...)
  2020-08-25 18:39 ` [PATCH 6/8] riscv/mm/fault: Simplify mm_fault_error() Pekka Enberg
@ 2020-08-25 18:39 ` Pekka Enberg
  2020-08-25 18:39 ` [PATCH 8/8] riscv/mm/fault: Move access error check to function Pekka Enberg
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:39 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

Let's handle the translation of EXC_STORE_PAGE_FAULT to FAULT_FLAG_WRITE
once before looking up the VMA. This makes it easier to extract access
error logic in the next patch.
---
 arch/riscv/mm/fault.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 3b430fb18de3..bdc70d3d507f 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -208,6 +208,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);
 
+	if (cause == EXC_STORE_PAGE_FAULT)
+		flags |= FAULT_FLAG_WRITE;
+
 retry:
 	mmap_read_lock(mm);
 	vma = find_vma(mm, addr);
@@ -251,7 +254,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 			bad_area(regs, mm, code, addr);
 			return;
 		}
-		flags |= FAULT_FLAG_WRITE;
 		break;
 	default:
 		panic("%s: unhandled cause %lu", __func__, cause);
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 8/8] riscv/mm/fault: Move access error check to function
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
                   ` (6 preceding siblings ...)
  2020-08-25 18:39 ` [PATCH 7/8] riscv/mm/fault: Move FAULT_FLAG_WRITE handling in do_page_fault() Pekka Enberg
@ 2020-08-25 18:39 ` Pekka Enberg
  2020-08-25 19:08 ` [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
  2020-09-04 17:37 ` Palmer Dabbelt
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 18:39 UTC (permalink / raw)
  To: linux-riscv; +Cc: Pekka Enberg, palmer

From: Pekka Enberg <penberg@kernel.org>

Move the access error check into a access_error() function to simplify
the control flow in do_page_fault().
---
 arch/riscv/mm/fault.c | 48 ++++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index bdc70d3d507f..a23eaf5ce95c 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -156,6 +156,30 @@ static void inline vmalloc_fault(struct pt_regs *regs, int code, unsigned long a
 	local_flush_tlb_page(addr);
 }
 
+static inline bool access_error(unsigned long cause, struct vm_area_struct *vma)
+{
+	switch (cause) {
+	case EXC_INST_PAGE_FAULT:
+		if (!(vma->vm_flags & VM_EXEC)) {
+			return true;
+		}
+		break;
+	case EXC_LOAD_PAGE_FAULT:
+		if (!(vma->vm_flags & VM_READ)) {
+			return true;
+		}
+		break;
+	case EXC_STORE_PAGE_FAULT:
+		if (!(vma->vm_flags & VM_WRITE)) {
+			return true;
+		}
+		break;
+	default:
+		panic("%s: unhandled cause %lu", __func__, cause);
+	}
+	return false;
+}
+
 /*
  * This routine handles page faults.  It determines the address and the
  * problem, and then passes it off to one of the appropriate routines.
@@ -236,27 +260,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 good_area:
 	code = SEGV_ACCERR;
 
-	switch (cause) {
-	case EXC_INST_PAGE_FAULT:
-		if (!(vma->vm_flags & VM_EXEC)) {
-			bad_area(regs, mm, code, addr);
-			return;
-		}
-		break;
-	case EXC_LOAD_PAGE_FAULT:
-		if (!(vma->vm_flags & VM_READ)) {
-			bad_area(regs, mm, code, addr);
-			return;
-		}
-		break;
-	case EXC_STORE_PAGE_FAULT:
-		if (!(vma->vm_flags & VM_WRITE)) {
-			bad_area(regs, mm, code, addr);
-			return;
-		}
-		break;
-	default:
-		panic("%s: unhandled cause %lu", __func__, cause);
+	if (unlikely(access_error(cause, vma))) {
+		bad_area(regs, mm, code, addr);
+		return;
 	}
 
 	/*
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
                   ` (7 preceding siblings ...)
  2020-08-25 18:39 ` [PATCH 8/8] riscv/mm/fault: Move access error check to function Pekka Enberg
@ 2020-08-25 19:08 ` Pekka Enberg
  2020-09-04 17:37 ` Palmer Dabbelt
  9 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2020-08-25 19:08 UTC (permalink / raw)
  To: linux-riscv; +Cc: Palmer Dabbelt

On Tue, Aug 25, 2020 at 9:39 PM Pekka Enberg <penberg@gmail.com> wrote:
>
> From: Pekka Enberg <penberg@kernel.org>
>
> This patch series cleans up the do_page_fault() function by replacing
> gotos with function calls, similar to what the x86 architecture does.
> The motivation for this series is to make thepage fault handling code
> easier to read and reason about.
>
> The use of gotos has no advantage in kernel code size either. In fact,
> the bloatometer script shows a tiny decrease in kernel text size:
>
>   add/remove: 1/0 grow/shrink: 0/2 up/down: 96/-99 (-3)
>   Function                                     old     new   delta
>   no_context.part                                -      96     +96
>   __func__                                      14      13      -1
>   do_page_fault                                778     680     -98
>   Total: Before=792, After=789, chg -0.38%
>
> Please note tht x86 also moves these functions out-of-line with the
> "noinline" annotation, which supposedly decreases stack usage in
> do_page_fault() at the expense of slighly larger kernel code size.
>
> However, in my testing, I was able to reduce stack usage at maximum by
> 16 bytes, at the expense of much larger kernel code size, so I am
> keeping the functions inline, and letting the compiler do its job.
>
> The patch series has been tested on QEMU.
>
> You can pull the series from:
>
>   git@github.com:penberg/linux.git penberg/riscv/mm-fault-cleanups

Of course I forgot the sign-off... :-/

Signed-off-by: Pekka Enberg <penberg@kernel.org>

(Updated the commits in the tree behind that git URL to also have the
sign-offs.)

- Pekka

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups
  2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
                   ` (8 preceding siblings ...)
  2020-08-25 19:08 ` [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
@ 2020-09-04 17:37 ` Palmer Dabbelt
  9 siblings, 0 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2020-09-04 17:37 UTC (permalink / raw)
  To: penberg; +Cc: penberg, linux-riscv

On Tue, 25 Aug 2020 11:38:59 PDT (-0700), penberg@gmail.com wrote:
> From: Pekka Enberg <penberg@kernel.org>
>
> This patch series cleans up the do_page_fault() function by replacing
> gotos with function calls, similar to what the x86 architecture does.
> The motivation for this series is to make thepage fault handling code
> easier to read and reason about.
>
> The use of gotos has no advantage in kernel code size either. In fact,
> the bloatometer script shows a tiny decrease in kernel text size:
>
>   add/remove: 1/0 grow/shrink: 0/2 up/down: 96/-99 (-3)
>   Function                                     old     new   delta
>   no_context.part                                -      96     +96
>   __func__                                      14      13      -1
>   do_page_fault                                778     680     -98
>   Total: Before=792, After=789, chg -0.38%
>
> Please note tht x86 also moves these functions out-of-line with the
> "noinline" annotation, which supposedly decreases stack usage in
> do_page_fault() at the expense of slighly larger kernel code size.
>
> However, in my testing, I was able to reduce stack usage at maximum by
> 16 bytes, at the expense of much larger kernel code size, so I am
> keeping the functions inline, and letting the compiler do its job.

This all seems fine to me, they're on for-next.

> The patch series has been tested on QEMU.
>
> You can pull the series from:
>
>   git@github.com:penberg/linux.git penberg/riscv/mm-fault-cleanups
>
> Pekka Enberg (8):
>   riscv/mm/fault: Move no context handling to no_context()
>   riscv/mm/fault: Move bad area handling to bad_area()
>   riscv/mm/fault: Move vmalloc fault handling to vmalloc_fault()
>   riscv/mm/fault: Simplify fault error handling
>   riscv/mm/fault: Move fault error handling to mm_fault_error()
>   riscv/mm/fault: Simplify mm_fault_error()
>   riscv/mm/fault: Move FAULT_FLAG_WRITE handling in do_page_fault()
>   riscv/mm/fault: Move access error check to function
>
>  arch/riscv/mm/fault.c | 337 +++++++++++++++++++++++-------------------
>  1 file changed, 189 insertions(+), 148 deletions(-)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-09-04 19:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-25 18:38 [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
2020-08-25 18:39 ` [PATCH 1/8] riscv/mm/fault: Move no context handling to no_context() Pekka Enberg
2020-08-25 18:39 ` [PATCH 2/8] riscv/mm/fault: Move bad area handling to bad_area() Pekka Enberg
2020-08-25 18:39 ` [PATCH 3/8] riscv/mm/fault: Move vmalloc fault handling to vmalloc_fault() Pekka Enberg
2020-08-25 18:39 ` [PATCH 4/8] riscv/mm/fault: Simplify fault error handling Pekka Enberg
2020-08-25 18:39 ` [PATCH 5/8] riscv/mm/fault: Move fault error handling to mm_fault_error() Pekka Enberg
2020-08-25 18:39 ` [PATCH 6/8] riscv/mm/fault: Simplify mm_fault_error() Pekka Enberg
2020-08-25 18:39 ` [PATCH 7/8] riscv/mm/fault: Move FAULT_FLAG_WRITE handling in do_page_fault() Pekka Enberg
2020-08-25 18:39 ` [PATCH 8/8] riscv/mm/fault: Move access error check to function Pekka Enberg
2020-08-25 19:08 ` [PATCH 0/8] riscv/mm/fault: Page fault handler cleanups Pekka Enberg
2020-09-04 17:37 ` Palmer Dabbelt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.