From 7c5b01014617411dc7e802169e95e467438a06cc Mon Sep 17 00:00:00 2001 From: guenther Date: Mon, 19 Jul 2010 23:00:15 +0000 Subject: [PATCH] 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@ --- sys/kern/kern_descrip.c | 8 +------- sys/kern/kern_exit.c | 6 +----- sys/kern/kern_fork.c | 4 +--- sys/kern/kern_proc.c | 5 +---- sys/kern/kern_sysctl.c | 31 +++++-------------------------- sys/sys/file.h | 3 +-- sys/sys/proc.h | 3 +-- 7 files changed, 11 insertions(+), 49 deletions(-) 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 #include #include -#include #include #include #include @@ -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. */ -- 2.20.1