From 1fa67124717f57cbece79264f31c7c3907864c47 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Thu, 10 Sep 2015 17:46:48 +0000 Subject: [PATCH] Do not hold the process around the vm_fault() call from the trap()s. The only operation which is prevented by the hold is the kernel stack swapout for the faulted thread, which should be fine to allow. Remove useless checks for NULL curproc or curproc->p_vmspace from the trap_pfault() wrappers on x86 and powerpc. Reviewed by: alc (previous version) Sponsored by: The FreeBSD Foundation MFC after: 2 weeks --- sys/amd64/amd64/trap.c | 34 +++------------------------------- sys/arm/arm/trap-v6.c | 23 ++--------------------- sys/arm/arm/trap.c | 23 ----------------------- sys/arm64/arm64/trap.c | 25 ++----------------------- sys/i386/i386/trap.c | 34 +++------------------------------- sys/mips/mips/trap.c | 12 ------------ sys/powerpc/powerpc/trap.c | 33 +++++---------------------------- sys/sparc64/sparc64/trap.c | 36 +++++++----------------------------- 8 files changed, 22 insertions(+), 198 deletions(-) diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c index fa74eb286a84..3bf63c36e5a8 100644 --- a/sys/amd64/amd64/trap.c +++ b/sys/amd64/amd64/trap.c @@ -625,7 +625,6 @@ trap_pfault(frame, usermode) int usermode; { vm_offset_t va; - struct vmspace *vm; vm_map_t map; int rv = 0; vm_prot_t ftype; @@ -687,14 +686,7 @@ trap_pfault(frame, usermode) map = kernel_map; } else { - /* - * This is a fault on non-kernel virtual memory. If either - * p or p->p_vmspace is NULL, then the fault is fatal. - */ - if (p == NULL || (vm = p->p_vmspace) == NULL) - goto nogo; - - map = &vm->vm_map; + map = &p->p_vmspace->vm_map; /* * When accessing a usermode address, kernel must be @@ -729,28 +721,8 @@ trap_pfault(frame, usermode) else ftype = VM_PROT_READ; - if (map != kernel_map) { - /* - * Keep swapout from messing with us during this - * critical time. - */ - PROC_LOCK(p); - ++p->p_lock; - PROC_UNLOCK(p); - - /* Fault in the user page: */ - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - - PROC_LOCK(p); - --p->p_lock; - PROC_UNLOCK(p); - } else { - /* - * Don't have to worry about process locking or stacks in the - * kernel. - */ - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - } + /* Fault in the page. */ + rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); if (rv == KERN_SUCCESS) { #ifdef HWPMC_HOOKS if (ftype == VM_PROT_READ || ftype == VM_PROT_WRITE) { diff --git a/sys/arm/arm/trap-v6.c b/sys/arm/arm/trap-v6.c index 09fc0fc9376d..b7b6629859d5 100644 --- a/sys/arm/arm/trap-v6.c +++ b/sys/arm/arm/trap-v6.c @@ -500,28 +500,9 @@ abort_handler(struct trapframe *tf, int prefetch) onfault = pcb->pcb_onfault; pcb->pcb_onfault = NULL; #endif - if (map != kernel_map) { - /* - * Keep swapout from messing with us during this - * critical time. - */ - PROC_LOCK(p); - ++p->p_lock; - PROC_UNLOCK(p); - /* Fault in the user page: */ - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - - PROC_LOCK(p); - --p->p_lock; - PROC_UNLOCK(p); - } else { - /* - * Don't have to worry about process locking or stacks in the - * kernel. - */ - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - } + /* Fault in the page. */ + rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); #ifdef INVARIANTS pcb->pcb_onfault = onfault; diff --git a/sys/arm/arm/trap.c b/sys/arm/arm/trap.c index 52a5baae0dfc..96602869ec73 100644 --- a/sys/arm/arm/trap.c +++ b/sys/arm/arm/trap.c @@ -365,19 +365,8 @@ abort_handler(struct trapframe *tf, int type) onfault = pcb->pcb_onfault; pcb->pcb_onfault = NULL; - if (map != kernel_map) { - PROC_LOCK(p); - p->p_lock++; - PROC_UNLOCK(p); - } error = vm_fault(map, va, ftype, VM_FAULT_NORMAL); pcb->pcb_onfault = onfault; - - if (map != kernel_map) { - PROC_LOCK(p); - p->p_lock--; - PROC_UNLOCK(p); - } if (__predict_true(error == 0)) goto out; fatal_pagefault: @@ -682,20 +671,8 @@ prefetch_abort_handler(struct trapframe *tf) if (pmap_fault_fixup(map->pmap, va, VM_PROT_READ, 1)) goto out; - if (map != kernel_map) { - PROC_LOCK(p); - p->p_lock++; - PROC_UNLOCK(p); - } - error = vm_fault(map, va, VM_PROT_READ | VM_PROT_EXECUTE, VM_FAULT_NORMAL); - if (map != kernel_map) { - PROC_LOCK(p); - p->p_lock--; - PROC_UNLOCK(p); - } - if (__predict_true(error == 0)) goto out; diff --git a/sys/arm64/arm64/trap.c b/sys/arm64/arm64/trap.c index 249322e6d050..2a3cfb9662f3 100644 --- a/sys/arm64/arm64/trap.c +++ b/sys/arm64/arm64/trap.c @@ -190,29 +190,8 @@ data_abort(struct trapframe *frame, uint64_t esr, int lower) va = trunc_page(far); ftype = ((esr >> 6) & 1) ? VM_PROT_READ | VM_PROT_WRITE : VM_PROT_READ; - if (map != kernel_map) { - /* - * Keep swapout from messing with us during this - * critical time. - */ - PROC_LOCK(p); - ++p->p_lock; - PROC_UNLOCK(p); - - /* Fault in the user page: */ - error = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - - PROC_LOCK(p); - --p->p_lock; - PROC_UNLOCK(p); - } else { - /* - * Don't have to worry about process locking or stacks in the - * kernel. - */ - error = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - } - + /* Fault in the page. */ + error = vm_fault(map, va, ftype, VM_FAULT_NORMAL); if (error != KERN_SUCCESS) { if (lower) { sig = SIGSEGV; diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c index 1e417bfd92bd..a3b1b0d2e9ec 100644 --- a/sys/i386/i386/trap.c +++ b/sys/i386/i386/trap.c @@ -782,7 +782,6 @@ trap_pfault(frame, usermode, eva) vm_offset_t eva; { vm_offset_t va; - struct vmspace *vm; vm_map_t map; int rv = 0; vm_prot_t ftype; @@ -852,14 +851,7 @@ trap_pfault(frame, usermode, eva) map = kernel_map; } else { - /* - * This is a fault on non-kernel virtual memory. If either - * p or p->p_vmspace is NULL, then the fault is fatal. - */ - if (p == NULL || (vm = p->p_vmspace) == NULL) - goto nogo; - - map = &vm->vm_map; + map = &p->p_vmspace->vm_map; /* * When accessing a user-space address, kernel must be @@ -888,28 +880,8 @@ trap_pfault(frame, usermode, eva) else ftype = VM_PROT_READ; - if (map != kernel_map) { - /* - * Keep swapout from messing with us during this - * critical time. - */ - PROC_LOCK(p); - ++p->p_lock; - PROC_UNLOCK(p); - - /* Fault in the user page: */ - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - - PROC_LOCK(p); - --p->p_lock; - PROC_UNLOCK(p); - } else { - /* - * Don't have to worry about process locking or stacks in the - * kernel. - */ - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - } + /* Fault in the page. */ + rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); if (rv == KERN_SUCCESS) { #ifdef HWPMC_HOOKS if (ftype == VM_PROT_READ || ftype == VM_PROT_WRITE) { diff --git a/sys/mips/mips/trap.c b/sys/mips/mips/trap.c index 98fe812dc93d..b1934638b2a3 100644 --- a/sys/mips/mips/trap.c +++ b/sys/mips/mips/trap.c @@ -714,19 +714,7 @@ trap(struct trapframe *trapframe) goto nogo; } - /* - * Keep swapout from messing with us during this - * critical time. - */ - PROC_LOCK(p); - ++p->p_lock; - PROC_UNLOCK(p); - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - - PROC_LOCK(p); - --p->p_lock; - PROC_UNLOCK(p); /* * XXXDTRACE: add dtrace_doubletrap_func here? */ diff --git a/sys/powerpc/powerpc/trap.c b/sys/powerpc/powerpc/trap.c index cabf78091ad8..29130eafb71d 100644 --- a/sys/powerpc/powerpc/trap.c +++ b/sys/powerpc/powerpc/trap.c @@ -704,9 +704,6 @@ trap_pfault(struct trapframe *frame, int user) #else if ((eva >> ADDR_SR_SHFT) == (USER_ADDR >> ADDR_SR_SHFT)) { #endif - if (p->p_vmspace == NULL) - return (SIGSEGV); - map = &p->p_vmspace->vm_map; #ifdef AIM @@ -720,31 +717,11 @@ trap_pfault(struct trapframe *frame, int user) } va = trunc_page(eva); - if (map != kernel_map) { - /* - * Keep swapout from messing with us during this - * critical time. - */ - PROC_LOCK(p); - ++p->p_lock; - PROC_UNLOCK(p); - - /* Fault in the user page: */ - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - - PROC_LOCK(p); - --p->p_lock; - PROC_UNLOCK(p); - /* - * XXXDTRACE: add dtrace_doubletrap_func here? - */ - } else { - /* - * Don't have to worry about process locking or stacks in the - * kernel. - */ - rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); - } + /* Fault in the page. */ + rv = vm_fault(map, va, ftype, VM_FAULT_NORMAL); + /* + * XXXDTRACE: add dtrace_doubletrap_func here? + */ if (rv == KERN_SUCCESS) return (0); diff --git a/sys/sparc64/sparc64/trap.c b/sys/sparc64/sparc64/trap.c index e9917e529535..96e72ed88e26 100644 --- a/sys/sparc64/sparc64/trap.c +++ b/sys/sparc64/sparc64/trap.c @@ -441,7 +441,7 @@ trap_cecc(void) static int trap_pfault(struct thread *td, struct trapframe *tf) { - struct vmspace *vm; + vm_map_t map; struct proc *p; vm_offset_t va; vm_prot_t prot; @@ -484,28 +484,8 @@ trap_pfault(struct thread *td, struct trapframe *tf) return (0); } - /* - * This is a fault on non-kernel virtual memory. - */ - vm = p->p_vmspace; - - /* - * Keep swapout from messing with us during this - * critical time. - */ - PROC_LOCK(p); - ++p->p_lock; - PROC_UNLOCK(p); - - /* Fault in the user page. */ - rv = vm_fault(&vm->vm_map, va, prot, VM_FAULT_NORMAL); - - /* - * Now the process can be swapped again. - */ - PROC_LOCK(p); - --p->p_lock; - PROC_UNLOCK(p); + /* This is a fault on non-kernel virtual memory. */ + map = &p->p_vmspace->vm_map; } else { /* * This is a fault on kernel virtual memory. Attempts to @@ -527,14 +507,12 @@ trap_pfault(struct thread *td, struct trapframe *tf) } vm_map_unlock_read(kernel_map); } - - /* - * We don't have to worry about process locking or stacks in - * the kernel. - */ - rv = vm_fault(kernel_map, va, prot, VM_FAULT_NORMAL); + map = kernel_map; } + /* Fault in the page. */ + rv = vm_fault(map, va, prot, VM_FAULT_NORMAL); + CTR3(KTR_TRAP, "trap_pfault: return td=%p va=%#lx rv=%d", td, va, rv); if (rv == KERN_SUCCESS)