Eliminate the f_usecount ref count in struct file; instead of sleeping

at the top of closef() until all in-progress calls finish, just do the
advisory locking bits required of close() by POSIX and let whichever
thread has the last reference do the call to the file's fo_close()
method and the final cleanup.

lots of discussion with deraadt@ and others; worked out with and ok krw@

3 files changed, 34 insertions, 46 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 546503b49c1..1f4934e795b 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.93 2012/04/22 05:43:14 guenther Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.94 2012/05/01 03:43:23 guenther Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -1053,33 +1053,15 @@ int
 closef(struct file *fp, struct proc *p)
 {
 	struct filedesc *fdp;
-	int references_left;
-	int error;
 
 	if (fp == NULL)
 		return (0);
 
-	/*
-	 * Some files passed to this function could be accessed
-	 * without a FILE_IS_USABLE check (and in some cases it's perfectly
-	 * legal), we must beware of files where someone already won the
-	 * race to FIF_WANTCLOSE.
-	 */
-	if ((fp->f_iflags & FIF_WANTCLOSE) != 0 ||
-	    --fp->f_count > 0) {
-		references_left = 1;
-	} else {
-		references_left = 0;
 #ifdef DIAGNOSTIC
-		if (fp->f_count < 0)
-			panic("closef: count < 0");
+	if (fp->f_count < 2)
+		panic("closef: count (%d) < 2", fp->f_count);
 #endif
-
-		/* Wait for the last usecount to drain. */
-		fp->f_iflags |= FIF_WANTCLOSE;
-		while (fp->f_usecount > 1)
-			tsleep(&fp->f_usecount, PRIBIO, "closef", 0);
-	}
+	fp->f_count--;
 
 	/*
 	 * POSIX record locking dictates that any close releases ALL
@@ -1103,10 +1085,18 @@ closef(struct file *fp, struct proc *p)
 		(void) VOP_ADVLOCK(vp, fdp, F_UNLCK, &lf, F_POSIX);
 	}
 
-	if (references_left) {
-		FRELE(fp, p);
-		return (0);
-	}
+	return (FRELE(fp, p));
+}
+
+int
+fdrop(struct file *fp, struct proc *p)
+{
+	int error;
+
+#ifdef DIAGNOSTIC
+	if (fp->f_count != 0)
+		panic("fdrop: count (%d) != 0", fp->f_count);
+#endif
 
 	if (fp->f_ops)
 		error = (*fp->f_ops->fo_close)(fp, p);
@@ -1116,10 +1106,6 @@ closef(struct file *fp, struct proc *p)
 	/* Free fp */
 	LIST_REMOVE(fp, f_list);
 	crfree(fp->f_cred);
-#ifdef DIAGNOSTIC
-	if (fp->f_count != 0 || fp->f_usecount != 1)
-		panic("closef: count: %d/%d", fp->f_count, fp->f_usecount);
-#endif
 	nfiles--;
 	pool_put(&file_pool, fp);
 
diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
index c7b1e5331e3..907035779df 100644
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_sysctl.c,v 1.221 2012/04/17 23:17:53 pirofti Exp $	*/
+/*	$OpenBSD: kern_sysctl.c,v 1.222 2012/05/01 03:43:23 guenther Exp $	*/
 /*	$NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $	*/
 
 /*-
@@ -1088,7 +1088,7 @@ fill_file2(struct kinfo_file2 *kf, struct file *fp, struct filedesc *fdp,
 		kf->f_gid = fp->f_cred->cr_gid;
 		kf->f_ops = PTRTOINT64(fp->f_ops);
 		kf->f_data = PTRTOINT64(fp->f_data);
-		kf->f_usecount = fp->f_usecount;
+		kf->f_usecount = 0;
 
 		if (suser(p, 0) == 0 || p->p_ucred->cr_uid == fp->f_cred->cr_uid) {
 			kf->f_offset = fp->f_offset;
@@ -1288,8 +1288,11 @@ sysctl_file2(int *name, u_int namelen, char *where, size_t *sizep,
 			break;
 		}
 		LIST_FOREACH(pp, &allproc, p_list) {
-			/* skip system, exiting, embryonic and undead processes */
-			if ((pp->p_flag & P_SYSTEM) || (pp->p_flag & P_WEXIT)
+			/*
+			 * skip system, exiting, embryonic and undead
+			 * processes, as well as threads
+			 */
+			if ((pp->p_flag & (P_SYSTEM | P_WEXIT | P_THREAD))
 			    || (pp->p_p->ps_flags & PS_EXITING)
 			    || pp->p_stat == SIDL || pp->p_stat == SZOMB)
 				continue;
@@ -1317,8 +1320,11 @@ sysctl_file2(int *name, u_int namelen, char *where, size_t *sizep,
 		break;
 	case KERN_FILE_BYUID:
 		LIST_FOREACH(pp, &allproc, p_list) {
-			/* skip system, exiting, embryonic and undead processes */
-			if ((pp->p_flag & P_SYSTEM) || (pp->p_flag & P_WEXIT)
+			/*
+			 * skip system, exiting, embryonic and undead
+			 * processes, as well as threads
+			 */
+			if ((pp->p_flag & (P_SYSTEM | P_WEXIT | P_THREAD))
 			    || (pp->p_p->ps_flags & PS_EXITING)
 			    || pp->p_stat == SIDL || pp->p_stat == SZOMB)
 				continue;
diff --git a/sys/sys/file.h b/sys/sys/file.h
index c36470bd21e..9c294f0f835 100644
--- a/sys/sys/file.h
+++ b/sys/sys/file.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: file.h,v 1.29 2012/04/22 05:43:14 guenther Exp $	*/
+/*	$OpenBSD: file.h,v 1.30 2012/05/01 03:43:23 guenther Exp $	*/
 /*	$NetBSD: file.h,v 1.11 1995/03/26 20:24:13 jtc Exp $	*/
 
 /*
@@ -78,7 +78,6 @@ struct file {
 	off_t	f_offset;
 	void 	*f_data;	/* private data */
 	int	f_iflags;	/* internal flags */
-	int	f_usecount;	/* number of users (temporary references). */
 	u_int64_t f_rxfer;	/* total number of read transfers */
 	u_int64_t f_wxfer;	/* total number of write transfers */
 	u_int64_t f_seek;	/* total independent seek operations */
@@ -86,26 +85,23 @@ struct file {
 	u_int64_t f_wbytes;	/* total bytes written */
 };
 
-#define FIF_WANTCLOSE		0x01	/* a close is waiting for usecount */
 #define FIF_LARVAL		0x02	/* not fully constructed, don't use */
 #define FIF_MARK		0x04	/* mark during gc() */
 #define FIF_DEFER		0x08	/* defer for next gc() pass */
 
 #define FILE_IS_USABLE(fp) \
-	(((fp)->f_iflags & (FIF_WANTCLOSE|FIF_LARVAL)) == 0)
+	(((fp)->f_iflags & FIF_LARVAL) == 0)
 
-#define FREF(fp) do { (fp)->f_usecount++; } while (0)
-#define FRELE(fp,p) do {					\
-	--(fp)->f_usecount;					\
-	if (((fp)->f_iflags & FIF_WANTCLOSE) != 0)		\
-		wakeup(&(fp)->f_usecount);			\
-} while (0)
+#define FREF(fp)	do { (fp)->f_count++; } while (0)
+#define FRELE(fp,p)	(--(fp)->f_count == 0 ? fdrop(fp, p) : 0)
 
 #define FILE_SET_MATURE(fp,p) do {				\
 	(fp)->f_iflags &= ~FIF_LARVAL;				\
 	FRELE(fp, p);						\
 } while (0)
 
+int	fdrop(struct file *, struct proc *);
+
 LIST_HEAD(filelist, file);
 extern struct filelist filehead;	/* head of list of open files */
 extern int maxfiles;			/* kernel limit on number of open files */