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 [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to