Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 18 Feb 2002 12:09:28 -0800 (PST)
From:      Matthew Dillon <dillon@apollo.backplane.com>
To:        current@freebsd.org
Subject:   Patch sets to date and timing tests with Giant out of userret. 
Message-ID:  <200202182009.g1IK9Sh36025@apollo.backplane.com>

next in thread | raw e-mail | index | archive | help
    Here are my giant related patches to date.  These are NOT all commit
    candidates.  Some are just hacks to test Giant removal.  Julian is
    responsible for the td_ucred persistance stuff.  The hack in userret()
    is just a hack to remove Giant when no signals are present in order to
    test performance.  Note that Julian hasn't updated persistent support
    for other cpu's so this patch only operates on i386.

    userret() appears to be the last major Giant-contention point in the
    syscall management path.

    getuid() performance, calls per second.
    current, SMP build, 2xCPU, invariants (+ patches below). 

    unpatched userret,	kern.giant.ucred=1 (default)
	1 process:	683K
	2 processes:	427K per process

    unpatched userret,	kern.giant.ucred=0
	1 process:	770K
	2 processes:	460K per process

    patched userret, kern.giant.ucred=1 (default)
	1 process:	739K
	2 processes:	480K per process

    patched userret, kern.giant.ucred=0
	1 process:	855K
	2 processes:	706K per process

    The 1 process vs 2 process numbers are getting a lot closer, but they
    are still not perfect.

						-Matt

Index: i386/i386/trap.c
===================================================================
RCS file: /home/ncvs/src/sys/i386/i386/trap.c,v
retrieving revision 1.212
diff -u -r1.212 trap.c
--- i386/i386/trap.c	17 Feb 2002 01:09:54 -0000	1.212
+++ i386/i386/trap.c	18 Feb 2002 19:46:50 -0000
@@ -255,7 +255,9 @@
 
 		sticks = td->td_kse->ke_sticks;
 		td->td_frame = &frame;
+#if 0
 		KASSERT(td->td_ucred == NULL, ("already have a ucred"));
+#endif
 		if (td->td_ucred != p->p_ucred) 
 			cred_update_thread(td);
 
@@ -643,7 +645,7 @@
 	userret(td, &frame, sticks);
 	mtx_assert(&Giant, MA_NOTOWNED);
 userout:
-#ifdef	INVARIANTS
+#if 0
 	mtx_lock(&Giant);
 	crfree(td->td_ucred);
 	mtx_unlock(&Giant);
@@ -954,7 +956,9 @@
 
 	sticks = td->td_kse->ke_sticks;
 	td->td_frame = &frame;
+#if 0
 	KASSERT(td->td_ucred == NULL, ("already have a ucred"));
+#endif
 	if (td->td_ucred != p->p_ucred) 
 		cred_update_thread(td);
 	params = (caddr_t)frame.tf_esp + sizeof(int);
@@ -1099,7 +1103,7 @@
 	 */
 	STOPEVENT(p, S_SCX, code);
 
-#ifdef	INVARIANTS
+#if 0
 	mtx_lock(&Giant);
 	crfree(td->td_ucred);
 	mtx_unlock(&Giant);
Index: kern/kern_fork.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_fork.c,v
retrieving revision 1.131
diff -u -r1.131 kern_fork.c
--- kern/kern_fork.c	17 Feb 2002 01:09:55 -0000	1.131
+++ kern/kern_fork.c	18 Feb 2002 19:46:51 -0000
@@ -799,7 +799,7 @@
 		kthread_exit(0);
 	}
 	PROC_UNLOCK(p);
-#ifdef	INVARIANTS
+#if 0
 	mtx_lock(&Giant);
 	crfree(td->td_ucred);
 	mtx_unlock(&Giant);
Index: kern/kern_mutex.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_mutex.c,v
retrieving revision 1.80
diff -u -r1.80 kern_mutex.c
--- kern/kern_mutex.c	18 Feb 2002 17:51:47 -0000	1.80
+++ kern/kern_mutex.c	18 Feb 2002 19:46:51 -0000
@@ -287,7 +287,9 @@
 _mtx_lock_sleep(struct mtx *m, int opts, const char *file, int line)
 {
 	struct thread *td = curthread;
+#if 0
 	struct ksegrp *kg = td->td_ksegrp;
+#endif
 
 	if ((m->mtx_lock & MTX_FLAGMASK) == (uintptr_t)td) {
 		m->mtx_recurse++;
@@ -312,6 +314,22 @@
 		 * the sched_lock.
 		 */
 		if ((v = m->mtx_lock) == MTX_UNOWNED) {
+			mtx_unlock_spin(&sched_lock);
+			continue;
+		}
+
+		/*
+		 * Check to see if there are any runnable processes.  If
+		 * there aren't and nobody is contesting the mutex (to avoid
+		 * starving a contester) and interrupts are enabled, then
+		 * we can safely spin.
+		 *
+		 * This prevents a silly-sleep-flip-flop situation on SMP
+		 * systems where two running processes need Giant (or any
+		 * other sleep mutex).
+		 */
+		if (td->td_critnest == 0 && (v & MTX_CONTESTED) == 0 &&
+		    procrunnable() == 0) {
 			mtx_unlock_spin(&sched_lock);
 			continue;
 		}
Index: kern/kern_prot.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_prot.c,v
retrieving revision 1.135
diff -u -r1.135 kern_prot.c
--- kern/kern_prot.c	17 Feb 2002 07:30:34 -0000	1.135
+++ kern/kern_prot.c	18 Feb 2002 19:46:51 -0000
@@ -228,14 +228,14 @@
 	struct thread *td;
 	struct getuid_args *uap;
 {
-	struct proc *p = td->td_proc;
+	int s;
 
-	mtx_lock(&Giant);
-	td->td_retval[0] = p->p_ucred->cr_ruid;
+	s = mtx_lock_giant(kern_giant_ucred);
+	td->td_retval[0] = td->td_ucred->cr_ruid;
 #if defined(COMPAT_43) || defined(COMPAT_SUNOS)
-	td->td_retval[1] = p->p_ucred->cr_uid;
+	td->td_retval[1] = td->td_ucred->cr_uid;
 #endif
-	mtx_unlock(&Giant);
+	mtx_unlock_giant(s);
 	return (0);
 }
 
@@ -253,9 +253,11 @@
 	struct thread *td;
 	struct geteuid_args *uap;
 {
-	mtx_lock(&Giant);
-	td->td_retval[0] = td->td_proc->p_ucred->cr_uid;
-	mtx_unlock(&Giant);
+	int s;
+
+	s = mtx_lock_giant(kern_giant_ucred);
+	td->td_retval[0] = td->td_ucred->cr_uid;
+	mtx_unlock_giant(s);
 	return (0);
 }
 
@@ -273,14 +275,14 @@
 	struct thread *td;
 	struct getgid_args *uap;
 {
-	struct proc *p = td->td_proc;
+	int s;
 
-	mtx_lock(&Giant);
-	td->td_retval[0] = p->p_ucred->cr_rgid;
+	s = mtx_lock_giant(kern_giant_ucred);
+	td->td_retval[0] = td->td_ucred->cr_rgid;
 #if defined(COMPAT_43) || defined(COMPAT_SUNOS)
-	td->td_retval[1] = p->p_ucred->cr_groups[0];
+	td->td_retval[1] = td->td_ucred->cr_groups[0];
 #endif
-	mtx_unlock(&Giant);
+	mtx_unlock_giant(s);
 	return (0);
 }
 
@@ -303,11 +305,11 @@
 	struct thread *td;
 	struct getegid_args *uap;
 {
-	struct proc *p = td->td_proc;
+	int s;
 
-	mtx_lock(&Giant);
-	td->td_retval[0] = p->p_ucred->cr_groups[0];
-	mtx_unlock(&Giant);
+	s = mtx_lock_giant(kern_giant_ucred);
+	td->td_retval[0] = td->td_ucred->cr_groups[0];
+	mtx_unlock_giant(s);
 	return (0);
 }
 
@@ -326,13 +328,13 @@
 	register struct getgroups_args *uap;
 {
 	struct ucred *cred;
-	struct proc *p = td->td_proc;
 	u_int ngrp;
 	int error;
+	int s;
 
-	mtx_lock(&Giant);
+	s = mtx_lock_giant(kern_giant_ucred);
 	error = 0;
-	cred = p->p_ucred;
+	cred = td->td_ucred;
 	if ((ngrp = uap->gidsetsize) == 0) {
 		td->td_retval[0] = cred->cr_ngroups;
 		goto done2;
@@ -347,7 +349,7 @@
 		goto done2;
 	td->td_retval[0] = ngrp;
 done2:
-	mtx_unlock(&Giant);
+	mtx_unlock_giant(s);
 	return (error);
 }
 
@@ -1057,11 +1059,11 @@
 	struct getresuid_args *uap;
 {
 	struct ucred *cred;
-	struct proc *p = td->td_proc;
 	int error1 = 0, error2 = 0, error3 = 0;
+	int s;
 
-	mtx_lock(&Giant);
-	cred = p->p_ucred;
+	s = mtx_lock_giant(kern_giant_ucred);
+	cred = td->td_ucred;
 	if (uap->ruid)
 		error1 = copyout((caddr_t)&cred->cr_ruid,
 		    (caddr_t)uap->ruid, sizeof(cred->cr_ruid));
@@ -1071,7 +1073,7 @@
 	if (uap->suid)
 		error3 = copyout((caddr_t)&cred->cr_svuid,
 		    (caddr_t)uap->suid, sizeof(cred->cr_svuid));
-	mtx_unlock(&Giant);
+	mtx_unlock_giant(s);
 	return (error1 ? error1 : error2 ? error2 : error3);
 }
 
@@ -1092,11 +1094,11 @@
 	struct getresgid_args *uap;
 {
 	struct ucred *cred;
-	struct proc *p = td->td_proc;
 	int error1 = 0, error2 = 0, error3 = 0;
+	int s;
 
-	mtx_lock(&Giant);
-	cred = p->p_ucred;
+	s = mtx_lock_giant(kern_giant_ucred);
+	cred = td->td_ucred;
 	if (uap->rgid)
 		error1 = copyout((caddr_t)&cred->cr_rgid,
 		    (caddr_t)uap->rgid, sizeof(cred->cr_rgid));
@@ -1106,7 +1108,7 @@
 	if (uap->sgid)
 		error3 = copyout((caddr_t)&cred->cr_svgid,
 		    (caddr_t)uap->sgid, sizeof(cred->cr_svgid));
-	mtx_unlock(&Giant);
+	mtx_unlock_giant(s);
 	return (error1 ? error1 : error2 ? error2 : error3);
 }
 
@@ -1172,6 +1174,8 @@
 
 /*
  * Check if gid is a member of the group set.
+ *
+ * MPSAFE (cred must be held)
  */
 int
 groupmember(gid, cred)
@@ -1228,6 +1232,8 @@
 
 /*
  * wrapper to use if you have the thread on hand but not the proc.
+ *
+ * MPSAFE (cred must be held)
  */
 int
 suser_xxx_td(cred, td, flag)
@@ -1269,6 +1275,8 @@
  * existing securelevel checks that occurred without a process/credential
  * context.  In the future this will be disallowed, so a kernel message
  * is displayed.
+ *
+ * MPSAFE
  */
 int
 securelevel_gt(struct ucred *cr, int level)
Index: kern/subr_trap.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/subr_trap.c,v
retrieving revision 1.208
diff -u -r1.208 subr_trap.c
--- kern/subr_trap.c	17 Feb 2002 01:09:55 -0000	1.208
+++ kern/subr_trap.c	18 Feb 2002 19:46:51 -0000
@@ -72,12 +72,18 @@
 	struct ksegrp *kg = td->td_ksegrp;
 	int sig;
 
-	mtx_lock(&Giant);
 	PROC_LOCK(p);
-	while ((sig = CURSIG(p)) != 0)
-		postsig(sig);
-	PROC_UNLOCK(p);
-	mtx_unlock(&Giant);
+	if (!SIGISEMPTY(p->p_siglist)) {
+	    PROC_UNLOCK(p);
+	    mtx_lock(&Giant);
+	    PROC_LOCK(p);
+	    while ((sig = CURSIG(p)) != 0)
+		    postsig(sig);
+	    PROC_UNLOCK(p);
+	    mtx_unlock(&Giant);
+	} else {
+	    PROC_UNLOCK(p);
+	}
 
 	mtx_lock_spin(&sched_lock);
 	td->td_priority = kg->kg_user_pri;
@@ -131,7 +137,9 @@
 #endif
 
 	KASSERT(TRAPF_USERMODE(framep), ("ast in kernel mode"));
+#if 0
 	KASSERT(td->td_ucred == NULL, ("leaked ucred"));
+#endif
 #ifdef WITNESS
 	if (witness_list(td))
 		panic("Returning to user mode with mutex(s) held");
@@ -187,7 +195,7 @@
 		}
 
 		userret(td, framep, sticks);
-#ifdef	INVARIANTS
+#if 0
 		mtx_lock(&Giant);
 		crfree(td->td_ucred);
 		mtx_unlock(&Giant);

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




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