Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 10 Sep 2015 17:46:49 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r287625 - in head/sys: amd64/amd64 arm/arm arm64/arm64 i386/i386 mips/mips powerpc/powerpc sparc64/sparc64
Message-ID:  <201509101746.t8AHknft012076@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Thu Sep 10 17:46:48 2015
New Revision: 287625
URL: https://svnweb.freebsd.org/changeset/base/287625

Log:
  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

Modified:
  head/sys/amd64/amd64/trap.c
  head/sys/arm/arm/trap-v6.c
  head/sys/arm/arm/trap.c
  head/sys/arm64/arm64/trap.c
  head/sys/i386/i386/trap.c
  head/sys/mips/mips/trap.c
  head/sys/powerpc/powerpc/trap.c
  head/sys/sparc64/sparc64/trap.c

Modified: head/sys/amd64/amd64/trap.c
==============================================================================
--- head/sys/amd64/amd64/trap.c	Thu Sep 10 16:13:44 2015	(r287624)
+++ head/sys/amd64/amd64/trap.c	Thu Sep 10 17:46:48 2015	(r287625)
@@ -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) {

Modified: head/sys/arm/arm/trap-v6.c
==============================================================================
--- head/sys/arm/arm/trap-v6.c	Thu Sep 10 16:13:44 2015	(r287624)
+++ head/sys/arm/arm/trap-v6.c	Thu Sep 10 17:46:48 2015	(r287625)
@@ -500,28 +500,9 @@ abort_handler(struct trapframe *tf, int 
 	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;

Modified: head/sys/arm/arm/trap.c
==============================================================================
--- head/sys/arm/arm/trap.c	Thu Sep 10 16:13:44 2015	(r287624)
+++ head/sys/arm/arm/trap.c	Thu Sep 10 17:46:48 2015	(r287625)
@@ -365,19 +365,8 @@ abort_handler(struct trapframe *tf, int 
 
 	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 
 	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;
 

Modified: head/sys/arm64/arm64/trap.c
==============================================================================
--- head/sys/arm64/arm64/trap.c	Thu Sep 10 16:13:44 2015	(r287624)
+++ head/sys/arm64/arm64/trap.c	Thu Sep 10 17:46:48 2015	(r287625)
@@ -190,29 +190,8 @@ data_abort(struct trapframe *frame, uint
 	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;

Modified: head/sys/i386/i386/trap.c
==============================================================================
--- head/sys/i386/i386/trap.c	Thu Sep 10 16:13:44 2015	(r287624)
+++ head/sys/i386/i386/trap.c	Thu Sep 10 17:46:48 2015	(r287625)
@@ -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) {

Modified: head/sys/mips/mips/trap.c
==============================================================================
--- head/sys/mips/mips/trap.c	Thu Sep 10 16:13:44 2015	(r287624)
+++ head/sys/mips/mips/trap.c	Thu Sep 10 17:46:48 2015	(r287625)
@@ -714,19 +714,7 @@ dofault:
 				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?
 			 */

Modified: head/sys/powerpc/powerpc/trap.c
==============================================================================
--- head/sys/powerpc/powerpc/trap.c	Thu Sep 10 16:13:44 2015	(r287624)
+++ head/sys/powerpc/powerpc/trap.c	Thu Sep 10 17:46:48 2015	(r287625)
@@ -704,9 +704,6 @@ trap_pfault(struct trapframe *frame, int
 #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
 	}
 	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);

Modified: head/sys/sparc64/sparc64/trap.c
==============================================================================
--- head/sys/sparc64/sparc64/trap.c	Thu Sep 10 16:13:44 2015	(r287624)
+++ head/sys/sparc64/sparc64/trap.c	Thu Sep 10 17:46:48 2015	(r287625)
@@ -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 tr
 			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 tr
 			}
 			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)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201509101746.t8AHknft012076>