Date: Fri, 14 Feb 2003 02:38:08 -0800 From: Alfred Perlstein <bright@mu.org> To: current@freebsd.org Cc: smp@freebsd.org Subject: fix: lock order reversal proc/filedesc. Message-ID: <20030214103808.GE93252@elvis.mu.org>
next in thread | raw e-mail | index | archive | help
Thanks to Paul Saab's work on fixing twe(4) I was able to get a
crash dump from my box and figure out why my filedesc locking patch
was panic'ing. kevent_close was dereferencing the proc's filedesc
pointer during close, that doesn't work so well when you need to
do what I had to do. :)
The gist of the fix is that the mutex 'fdesc_mutex' is used as a
global barrier against losing hold of the proc->p_fd reference.
So, basically, if you are not the process that owns a filedesc you
must grab the mutex over _all_ usage of it, and you probably want
the allproc_lock as well to make sure you don't lose your process
as well. :) (see sysctl_kern_file() for an example.)
I've also fixed kqueue_close to use the stashed filedesc pointer
inside the f_data pointer rather than trying to get at it via
p->p_fd.
please review/test:
Index: kern/kern_descrip.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.185
diff -u -r1.185 kern_descrip.c
--- kern/kern_descrip.c 11 Feb 2003 07:20:52 -0000 1.185
+++ kern/kern_descrip.c 14 Feb 2003 10:11:43 -0000
@@ -1366,6 +1366,10 @@
return (newfdp);
}
+/* A mutex to protect the association between a proc and filedesc. */
+struct mtx fdesc_mtx;
+MTX_SYSINIT(fdesc, &fdesc_mtx, "fdesc", MTX_DEF);
+
/*
* Release a filedesc structure.
*/
@@ -1382,6 +1386,10 @@
if (fdp == NULL)
return;
+ mtx_lock(&fdesc_mtx);
+ td->td_proc->p_fd = NULL;
+ mtx_unlock(&fdesc_mtx);
+
FILEDESC_LOCK(fdp);
if (--fdp->fd_refcnt > 0) {
FILEDESC_UNLOCK(fdp);
@@ -1398,7 +1406,6 @@
if (*fpp)
(void) closef(*fpp, td);
}
- td->td_proc->p_fd = NULL;
if (fdp->fd_nfiles > NDFILE)
FREE(fdp->fd_ofiles, M_FILEDESC);
if (fdp->fd_cdir)
@@ -2105,7 +2112,9 @@
xf.xf_pid = p->p_pid;
xf.xf_uid = p->p_ucred->cr_uid;
PROC_UNLOCK(p);
+ mtx_lock(&fdesc_mtx);
if ((fdp = p->p_fd) == NULL) {
+ mtx_unlock(&fdesc_mtx);
continue;
}
FILEDESC_LOCK(fdp);
@@ -2125,6 +2134,7 @@
break;
}
FILEDESC_UNLOCK(fdp);
+ mtx_unlock(&fdesc_mtx);
if (error)
break;
}
Index: kern/kern_event.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_event.c,v
retrieving revision 1.54
diff -u -r1.54 kern_event.c
--- kern/kern_event.c 21 Jan 2003 08:55:53 -0000 1.54
+++ kern/kern_event.c 14 Feb 2003 09:59:11 -0000
@@ -839,7 +840,7 @@
kqueue_close(struct file *fp, struct thread *td)
{
struct kqueue *kq = fp->f_data;
- struct filedesc *fdp = td->td_proc->p_fd;
+ struct filedesc *fdp = kq->kq_fdp;
struct knote **knp, *kn, *kn0;
int i;
Index: kern/kern_exit.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.193
diff -u -r1.193 kern_exit.c
--- kern/kern_exit.c 8 Feb 2003 02:58:16 -0000 1.193
+++ kern/kern_exit.c 13 Feb 2003 09:15:30 -0000
@@ -263,7 +263,7 @@
* Close open files and release open-file table.
* This may block!
*/
- fdfree(td); /* XXXKSE *//* may not be the one in proc */
+ fdfree(td);
/*
* Remove ourself from our leader's peer list and wake our leader.
Index: kern/vfs_mount.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_mount.c,v
retrieving revision 1.97
diff -u -r1.97 vfs_mount.c
--- kern/vfs_mount.c 21 Jan 2003 08:55:55 -0000 1.97
+++ kern/vfs_mount.c 14 Feb 2003 10:09:16 -0000
@@ -1141,10 +1141,10 @@
return;
sx_slock(&allproc_lock);
LIST_FOREACH(p, &allproc, p_list) {
- PROC_LOCK(p);
+ mtx_lock(&fdesc_mtx);
fdp = p->p_fd;
if (fdp == NULL) {
- PROC_UNLOCK(p);
+ mtx_unlock(&fdesc_mtx);
continue;
}
nrele = 0;
@@ -1160,7 +1160,7 @@
nrele++;
}
FILEDESC_UNLOCK(fdp);
- PROC_UNLOCK(p);
+ mtx_unlock(&fdesc_mtx);
while (nrele--)
vrele(olddp);
}
Index: sys/filedesc.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/filedesc.h,v
retrieving revision 1.49
diff -u -r1.49 filedesc.h
--- sys/filedesc.h 1 Jan 2003 01:56:19 -0000 1.49
+++ sys/filedesc.h 14 Feb 2003 10:12:59 -0000
@@ -139,6 +139,8 @@
return ((u_int)fd >= (u_int)fdp->fd_nfiles ? NULL : fdp->fd_ofiles[fd]);
}
+extern struct mtx fdesc_mtx;
+
#endif /* _KERNEL */
#endif /* !_SYS_FILEDESC_H_ */
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030214103808.GE93252>