trap_pfault growstack vs vm_fault ordering?

[Venkatesh Srinivas <me@xxxxxxxxxxxxxxxxxxx>]

Hi,

Currently trap_pfault for each of the three DragonFly platforms
attempts to grow the stack region (via growstack()) before attempting
vm_fault. For any faults not in stack growth regions, this will result
in lots of work (3 crit_enter/crit_exit pairs, a number of zone
allocations). Would it be worth moving the stack growth call into
vm_fault, after the vm_map_lookup call?

Here is a patch that does this:
(to sys/platform/<T>/<A>/trap.c):
--- trap.c.orig	2010-02-26 06:17:34 -0800
+++ trap.c	2010-02-26 06:17:54 -0800
@@ -989,21 +989,6 @@
 		PHOLD(lp->lwp_proc);

- 		/*
-		 * Grow the stack if necessary
-		 */
-		/* grow_stack returns false only if va falls into
-		 * a growable stack region and the stack growth
-		 * fails.  It returns true if va was not within
-		 * a growable stack region, or if the stack
-		 * growth succeeded.
-		 */
-		if (!grow_stack(lp->lwp_proc, va)) {
-			rv = KERN_FAILURE;
-			PRELE(lp->lwp_proc);
-			goto nogo;
-		}
-
		/*
 		 * Issue fault
 		 */
 		fault_flags = 0;

To vm/vm_fault.c:
diff --git a/sys/vm/vm_fault.c b/sys/vm/vm_fault.c
index 0fae064..7fd0998 100644
--- a/sys/vm/vm_fault.c
+++ b/sys/vm/vm_fault.c
@@ -223,12 +223,14 @@ vm_fault(vm_map_t map, vm_offset_t vaddr,
vm_prot_t fault_type, int fault_flags)
 	int result;
 	vm_pindex_t first_pindex;
 	struct faultstate fs;
+	int growstack;

 	mycpu->gd_cnt.v_vm_faults++;

 	fs.didlimit = 0;
 	fs.hardfault = 0;
 	fs.fault_flags = fault_flags;
+	growstack = 1;

 RetryFault:
 	/*
@@ -256,10 +258,21 @@ RetryFault:
 	 * to do a user wiring we have more work to do.
 	 */
 	if (result != KERN_SUCCESS) {
-		if (result != KERN_PROTECTION_FAILURE)
-			return result;
-		if ((fs.fault_flags & VM_FAULT_WIRE_MASK) != VM_FAULT_USER_WIRE)
-			return result;
+		if (result != KERN_PROTECTION_FAILURE ||
+		    (fs.fault_flags & VM_FAULT_WIRE_MASK) != VM_FAULT_USER_WIRE)
+		{
+			if (growstack &&
+			    result == KERN_INVALID_ADDRESS &&
+			    map != &kernel_map &&
+			    curproc != NULL) {
+				result = vm_map_growstack(curproc, vaddr);
+				if (result != KERN_SUCCESS)
+					return (KERN_FAILURE);
+				growstack = 0;
+				goto RetryFault;
+			}
+			return (result);
+		}

 		/*
    		 * If we are user-wiring a r/w segment, and it is COW, then


I'm not currently setup to measure the speed differences or the
distribution of faults, to know whether this is a good idea... does
this seem reasonable?

Thanks,
-- vs