From: guenther <guenther@openbsd.org>
Date: Mon, 19 Jul 2010 23:00:15 +0000 (+0000)
Subject: Rollback the allproclk and fileheadlk addition.  When grabbing an
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=7c5b01014617411dc7e802169e95e467438a06cc;p=openbsd

Rollback the allproclk and fileheadlk addition.  When grabbing an
rwlock, the thread will release biglock if it sleeps, means that
atomicity from before the rw_enter() to after it is not guaranteed.
The change didn't address those, so pulling it until it does.

"go for it" tedu@
---

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 8efcc923f98..12c44f0294d 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.83 2010/03/24 23:18:17 tedu Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.84 2010/07/19 23:00:15 guenther Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -68,7 +68,6 @@
  * Descriptor management.
  */
 struct filelist filehead;	/* head of list of open files */
-struct rwlock fileheadlk;
 int nfiles;			/* actual number of open files */
 
 static __inline void fd_used(struct filedesc *, int);
@@ -88,7 +87,6 @@ filedesc_init(void)
 	pool_init(&fdesc_pool, sizeof(struct filedesc0), 0, 0, 0, "fdescpl",
 		&pool_allocator_nointr);
 	LIST_INIT(&filehead);
-	rw_init(&fileheadlk, "filehead");
 }
 
 static __inline int
@@ -827,13 +825,11 @@ restart:
 	nfiles++;
 	fp = pool_get(&file_pool, PR_WAITOK|PR_ZERO);
 	fp->f_iflags = FIF_LARVAL;
-	rw_enter_write(&fileheadlk);
 	if ((fq = p->p_fd->fd_ofiles[0]) != NULL) {
 		LIST_INSERT_AFTER(fq, fp, f_list);
 	} else {
 		LIST_INSERT_HEAD(&filehead, fp, f_list);
 	}
-	rw_exit_write(&fileheadlk);
 	p->p_fd->fd_ofiles[i] = fp;
 	fp->f_count = 1;
 	fp->f_cred = p->p_ucred;
@@ -1092,9 +1088,7 @@ closef(struct file *fp, struct proc *p)
 		error = 0;
 
 	/* Free fp */
-	rw_enter_write(&fileheadlk);
 	LIST_REMOVE(fp, f_list);
-	rw_exit_write(&fileheadlk);
 	crfree(fp->f_cred);
 #ifdef DIAGNOSTIC
 	if (fp->f_count != 0 || fp->f_usecount != 1)
diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index d8fc8a2e97e..bf36116bd53 100644
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_exit.c,v 1.94 2010/06/29 20:25:57 guenther Exp $	*/
+/*	$OpenBSD: kern_exit.c,v 1.95 2010/07/19 23:00:15 guenther Exp $	*/
 /*	$NetBSD: kern_exit.c,v 1.39 1996/04/22 01:38:25 christos Exp $	*/
 
 /*
@@ -242,7 +242,6 @@ exit1(struct proc *p, int rv, int flags)
          * deadproc list later (using the p_hash member), and
          * wake up the reaper when we do.
          */
-	rw_enter_write(&allproclk);
 	/*
 	 * NOTE: WE ARE NO LONGER ALLOWED TO SLEEP!
 	 */
@@ -251,7 +250,6 @@ exit1(struct proc *p, int rv, int flags)
 	LIST_REMOVE(p, p_hash);
 	LIST_REMOVE(p, p_list);
 	LIST_INSERT_HEAD(&zombproc, p, p_list);
-	rw_exit_write(&allproclk);
 
 	/*
 	 * Give orphaned children to init(8).
@@ -568,9 +566,7 @@ proc_zap(struct proc *p)
 	 * Unlink it from its process group and free it.
 	 */
 	leavepgrp(p);
-	rw_enter_write(&allproclk);
 	LIST_REMOVE(p, p_list);	/* off zombproc */
-	rw_exit_write(&allproclk);
 	LIST_REMOVE(p, p_sibling);
 
 	/*
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 8e4674cb6bb..8d30efdc09c 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_fork.c,v 1.119 2010/07/02 01:25:05 art Exp $	*/
+/*	$OpenBSD: kern_fork.c,v 1.120 2010/07/19 23:00:15 guenther Exp $	*/
 /*	$NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $	*/
 
 /*
@@ -388,7 +388,6 @@ fork1(struct proc *p1, int exitsig, int flags, void *stack, size_t stacksize,
 #endif
 
 	/* Find an unused pid satisfying 1 <= lastpid <= PID_MAX */
-	rw_enter_write(&allproclk);
 	do {
 		lastpid = 1 + (randompid ? arc4random() : lastpid) % PID_MAX;
 	} while (pidtaken(lastpid));
@@ -396,7 +395,6 @@ fork1(struct proc *p1, int exitsig, int flags, void *stack, size_t stacksize,
 
 	LIST_INSERT_HEAD(&allproc, p2, p_list);
 	LIST_INSERT_HEAD(PIDHASH(p2->p_pid), p2, p_hash);
-	rw_exit_write(&allproclk);
 	LIST_INSERT_HEAD(&p1->p_children, p2, p_sibling);
 	LIST_INSERT_AFTER(p1, p2, p_pglist);
 	if (p2->p_flag & P_TRACED) {
diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index 20884921077..96276b58303 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_proc.c,v 1.43 2010/07/10 21:29:37 guenther Exp $	*/
+/*	$OpenBSD: kern_proc.c,v 1.44 2010/07/19 23:00:15 guenther Exp $	*/
 /*	$NetBSD: kern_proc.c,v 1.14 1996/02/09 18:59:41 christos Exp $	*/
 
 /*
@@ -40,7 +40,6 @@
 #include <sys/acct.h>
 #include <sys/wait.h>
 #include <sys/file.h>
-#include <sys/rwlock.h>
 #include <ufs/ufs/quota.h>
 #include <sys/uio.h>
 #include <sys/malloc.h>
@@ -62,7 +61,6 @@ u_long pidhash;
 struct pgrphashhead *pgrphashtbl;
 u_long pgrphash;
 struct proclist allproc;
-struct rwlock allproclk;
 struct proclist zombproc;
 
 struct pool proc_pool;
@@ -85,7 +83,6 @@ void
 procinit(void)
 {
 	LIST_INIT(&allproc);
-	rw_init(&allproclk, "allproc");
 	LIST_INIT(&zombproc);
 
 
diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
index 9fc2b8e78f2..2e78801663c 100644
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_sysctl.c,v 1.189 2010/07/10 21:29:37 guenther Exp $	*/
+/*	$OpenBSD: kern_sysctl.c,v 1.190 2010/07/19 23:00:15 guenther Exp $	*/
 /*	$NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $	*/
 
 /*-
@@ -163,13 +163,8 @@ sys___sysctl(struct proc *p, void *v, register_t *retval)
 	switch (name[0]) {
 	case CTL_KERN:
 		fn = kern_sysctl;
-		switch (name[1]) {	/* XXX */
-		case KERN_VNODE:
-		case KERN_FILE:
-		case KERN_FILE2:
+		if (name[1] == KERN_VNODE)	/* XXX */
 			dolock = 0;
-			break;
-		}
 		break;
 	case CTL_HW:
 		fn = hw_sysctl;
@@ -1004,12 +999,10 @@ sysctl_file(char *where, size_t *sizep, struct proc *p)
 	/*
 	 * followed by an array of file structures
 	 */
-	rw_enter_read(&fileheadlk);
 	LIST_FOREACH(fp, &filehead, f_list) {
 		if (buflen < sizeof(struct file)) {
 			*sizep = where - start;
-			error = ENOMEM;
-			goto out;
+			return (ENOMEM);
 		}
 
 		/* Only let the superuser or the owner see some information */
@@ -1023,14 +1016,12 @@ sysctl_file(char *where, size_t *sizep, struct proc *p)
 		}
 		error = copyout(&cfile, where, sizeof (struct file));
 		if (error)
-			goto out;
+			return (error);
 		buflen -= sizeof(struct file);
 		where += sizeof(struct file);
 	}
 	*sizep = where - start;
-out:
-	rw_exit_read(&fileheadlk);
-	return (error);
+	return (0);
 }
 
 #ifndef SMALL_KERNEL
@@ -1234,13 +1225,11 @@ sysctl_file2(int *name, u_int namelen, char *where, size_t *sizep,
 			error = EINVAL;
 			break;
 		}
-		rw_enter_read(&fileheadlk);
 		LIST_FOREACH(fp, &filehead, f_list) {
 			if (fp->f_count == 0)
 				continue;
 			FILLIT(fp, NULL, 0, NULL, NULL);
 		}
-		rw_exit_read(&fileheadlk);
 		break;
 	case KERN_FILE_BYPID:
 		/* A arg of -1 indicates all processes */
@@ -1248,7 +1237,6 @@ sysctl_file2(int *name, u_int namelen, char *where, size_t *sizep,
 			error = EINVAL;
 			break;
 		}
-		rw_enter_read(&allproclk);
 		LIST_FOREACH(pp, &allproc, p_list) {
 			/* skip system, exiting, embryonic and undead processes */
 			if ((pp->p_flag & P_SYSTEM) || (pp->p_flag & P_WEXIT)
@@ -1259,7 +1247,6 @@ sysctl_file2(int *name, u_int namelen, char *where, size_t *sizep,
 				continue;
 			}
 			fdp = pp->p_fd;
-			fdplock(fdp);
 			if (pp->p_textvp)
 				FILLIT(NULL, NULL, KERN_FILE_TEXT, pp->p_textvp, pp);
 			if (fdp->fd_cdir)
@@ -1275,12 +1262,9 @@ sysctl_file2(int *name, u_int namelen, char *where, size_t *sizep,
 					continue;
 				FILLIT(fp, fdp, i, NULL, pp);
 			}
-			fdpunlock(fdp);
 		}
-		rw_exit_read(&allproclk);
 		break;
 	case KERN_FILE_BYUID:
-		rw_enter_read(&allproclk);
 		LIST_FOREACH(pp, &allproc, p_list) {
 			/* skip system, exiting, embryonic and undead processes */
 			if ((pp->p_flag & P_SYSTEM) || (pp->p_flag & P_WEXIT)
@@ -1291,7 +1275,6 @@ sysctl_file2(int *name, u_int namelen, char *where, size_t *sizep,
 				continue;
 			}
 			fdp = pp->p_fd;
-			fdplock(fdp);
 			if (fdp->fd_cdir)
 				FILLIT(NULL, NULL, KERN_FILE_CDIR, fdp->fd_cdir, pp);
 			if (fdp->fd_rdir)
@@ -1305,9 +1288,7 @@ sysctl_file2(int *name, u_int namelen, char *where, size_t *sizep,
 					continue;
 				FILLIT(fp, fdp, i, NULL, pp);
 			}
-			fdpunlock(fdp);
 		}
-		rw_exit_read(&allproclk);
 		break;
 	default:
 		error = EINVAL;
@@ -1362,7 +1343,6 @@ sysctl_doproc(int *name, u_int namelen, char *where, size_t *sizep)
 		elem_count = name[4];
 		kproc2 = malloc(sizeof(struct kinfo_proc2), M_TEMP, M_WAITOK);
 	}
-	rw_enter_read(&allproclk);
 	p = LIST_FIRST(&allproc);
 	doingzomb = 0;
 again:
@@ -1479,7 +1459,6 @@ again:
 		*sizep = needed;
 	}
 err:
-	rw_exit_read(&allproclk);
 	if (eproc)
 		free(eproc, M_TEMP);
 	if (kproc2)
diff --git a/sys/sys/file.h b/sys/sys/file.h
index 28474acf9f9..8b7f00eae9c 100644
--- a/sys/sys/file.h
+++ b/sys/sys/file.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: file.h,v 1.26 2010/03/24 23:18:17 tedu Exp $	*/
+/*	$OpenBSD: file.h,v 1.27 2010/07/19 23:00:15 guenther Exp $	*/
 /*	$NetBSD: file.h,v 1.11 1995/03/26 20:24:13 jtc Exp $	*/
 
 /*
@@ -106,7 +106,6 @@ struct file {
 
 LIST_HEAD(filelist, file);
 extern struct filelist filehead;	/* head of list of open files */
-extern struct rwlock fileheadlk;
 extern int maxfiles;			/* kernel limit on number of open files */
 extern int nfiles;			/* actual number of open files */
 extern struct fileops vnops;		/* vnode operations for files */
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 2cb299f285b..3e857da868e 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.h,v 1.130 2010/07/03 04:44:51 guenther Exp $	*/
+/*	$OpenBSD: proc.h,v 1.131 2010/07/19 23:00:15 guenther Exp $	*/
 /*	$NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $	*/
 
 /*-
@@ -405,7 +405,6 @@ extern int randompid;			/* fork() should create random pid's */
 
 LIST_HEAD(proclist, proc);
 extern struct proclist allproc;		/* List of all processes. */
-extern struct rwlock allproclk;		/* also used for zombie list */
 extern struct proclist zombproc;	/* List of zombie processes. */
 
 extern struct proc *initproc;		/* Process slots for init, pager. */