Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys Implement mutexes for file descriptor and current workin...



details:   https://anonhg.NetBSD.org/src/rev/4836a98fc1ce
branches:  trunk
changeset: 567102:4836a98fc1ce
user:      pk <pk%NetBSD.org@localhost>
date:      Mon May 31 15:30:55 2004 +0000

description:
Implement mutexes for file descriptor and current working directory access.
Fix a potential race condition when reallocating storage for file descriptors
(even for non-SMP kernels).
Add missing locks for `struct file' ref count updates.

diffstat:

 sys/kern/kern_descrip.c |  242 ++++++++++++++++++++++++++++++++---------------
 sys/sys/filedesc.h      |   10 +-
 2 files changed, 173 insertions(+), 79 deletions(-)

diffs (truncated from 560 to 300 lines):

diff -r 49579771a5bb -r 4836a98fc1ce sys/kern/kern_descrip.c
--- a/sys/kern/kern_descrip.c   Mon May 31 14:04:59 2004 +0000
+++ b/sys/kern/kern_descrip.c   Mon May 31 15:30:55 2004 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_descrip.c,v 1.125 2004/04/25 16:42:41 simonb Exp $        */
+/*     $NetBSD: kern_descrip.c,v 1.126 2004/05/31 15:30:55 pk Exp $    */
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1991, 1993
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_descrip.c,v 1.125 2004/04/25 16:42:41 simonb Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_descrip.c,v 1.126 2004/05/31 15:30:55 pk Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -158,6 +158,7 @@
 {
        u_int off = fd >> NDENTRYSHIFT;
 
+       LOCK_ASSERT(simple_lock_held(&fdp->fd_slock));
        KDASSERT((fdp->fd_lomap[off] & (1 << (fd & NDENTRYMASK))) == 0);
 
        fdp->fd_lomap[off] |= 1 << (fd & NDENTRYMASK);
@@ -176,6 +177,7 @@
 {
        u_int off = fd >> NDENTRYSHIFT;
 
+       LOCK_ASSERT(simple_lock_held(&fdp->fd_slock));
        if (fd < fdp->fd_freefile)
                fdp->fd_freefile = fd;
 
@@ -315,11 +317,14 @@
                }
                if (new != i)
                        panic("dup2: fdalloc");
-       } else if (fdp->fd_ofiles[new] == NULL) {
+       } else {
+               simple_lock(&fdp->fd_slock);
                /*
                 * Mark `new' slot "used" only if it was empty.
                 */
-               fd_used(fdp, new);
+               if (fdp->fd_ofiles[new] == NULL)
+                       fd_used(fdp, new);
+               simple_unlock(&fdp->fd_slock);
        }
 
        /*
@@ -557,15 +562,19 @@
         * Note: `old' is already used for us.
         * Note: Caller already marked `new' slot "used".
         */
+       simple_lock(&fdp->fd_slock);
        delfp = fdp->fd_ofiles[new];
 
        fp = fdp->fd_ofiles[old];
        KDASSERT(fp != NULL);
        fdp->fd_ofiles[new] = fp;
        fdp->fd_ofileflags[new] = fdp->fd_ofileflags[old] &~ UF_EXCLOSE;
-       fp->f_count++;
+       simple_unlock(&fdp->fd_slock);
+
        *retval = new;
-       FILE_UNUSE(fp, p);
+       simple_lock(&fp->f_slock);
+       fp->f_count++;
+       FILE_UNUSE_HAVELOCK(fp, p);
 
        if (delfp != NULL) {
                simple_lock(&delfp->f_slock);
@@ -581,8 +590,10 @@
 fdremove(struct filedesc *fdp, int fd)
 {
 
+       simple_lock(&fdp->fd_slock);
        fdp->fd_ofiles[fd] = NULL;
        fd_unused(fdp, fd);
+       simple_unlock(&fdp->fd_slock);
 }
 
 int
@@ -592,25 +603,33 @@
        struct file     **fpp, *fp;
 
        fdp = p->p_fd;
+       simple_lock(&fdp->fd_slock);
+       if ((u_int) fd > fdp->fd_lastfile)
+               goto badf;
        fpp = &fdp->fd_ofiles[fd];
        fp = *fpp;
        if (fp == NULL)
-               return (EBADF);
+               goto badf;
 
        simple_lock(&fp->f_slock);
        if (!FILE_IS_USABLE(fp)) {
                simple_unlock(&fp->f_slock);
-               return (EBADF);
+               goto badf;
        }
 
        FILE_USE(fp);
 
        *fpp = NULL;
        fdp->fd_ofileflags[fd] = 0;
+       fd_unused(fdp, fd);
+       simple_unlock(&fdp->fd_slock);
        if (fd < fdp->fd_knlistsize)
                knote_fdclose(p, fd);
-       fd_unused(fdp, fd);
        return (closef(fp, p));
+
+badf:
+       simple_unlock(&fdp->fd_slock);
+       return (EBADF);
 }
 
 /*
@@ -631,8 +650,6 @@
        fd = SCARG(uap, fd);
        fdp = p->p_fd;
 
-       if ((u_int) fd >= fdp->fd_nfiles)
-               return (EBADF);
 #if 0
        if (fd_getfile(fdp, fd) == NULL)
                return (EBADF);
@@ -741,10 +758,11 @@
 fdalloc(struct proc *p, int want, int *result)
 {
        struct filedesc *fdp;
-       int i, lim, last;
+       int i, lim, last, error;
        u_int off, new;
 
        fdp = p->p_fd;
+       simple_lock(&fdp->fd_slock);
 
        /*
         * Search for a free descriptor starting at the higher
@@ -777,35 +795,58 @@
                                if (want <= fdp->fd_freefile)
                                        fdp->fd_freefile = i;
                                *result = i;
-                               return (0);
+                               error = 0;
+                               goto out;
                        }
                }
        }
 
-       /* No space in current array.  Expand? */
-       if (fdp->fd_nfiles >= lim)
-               return (EMFILE);
+       /* No space in current array.  Expand or let the caller do it. */
+       error = (fdp->fd_nfiles >= lim) ? EMFILE : ENOSPC;
 
-       /* Let the caller do it. */
-       return (ENOSPC);
+out:
+       simple_unlock(&fdp->fd_slock);
+       return (error);
 }
 
 void
 fdexpand(struct proc *p)
 {
        struct filedesc *fdp;
-       int             i, nfiles;
+       int             i, nfiles, oldnfiles;
        struct file     **newofile;
        char            *newofileflags;
-       uint32_t        *newhimap, *newlomap;
+       uint32_t        *newhimap = NULL, *newlomap = NULL;
 
        fdp = p->p_fd;
 
-       if (fdp->fd_nfiles < NDEXTENT)
+restart:
+       oldnfiles = fdp->fd_nfiles;
+
+       if (oldnfiles < NDEXTENT)
                nfiles = NDEXTENT;
        else
-               nfiles = 2 * fdp->fd_nfiles;
+               nfiles = 2 * oldnfiles;
+
        newofile = malloc(nfiles * OFILESIZE, M_FILEDESC, M_WAITOK);
+       if (NDHISLOTS(nfiles) > NDHISLOTS(oldnfiles)) {
+               newhimap = malloc(NDHISLOTS(nfiles) * sizeof(uint32_t),
+                   M_FILEDESC, M_WAITOK);
+               newlomap = malloc(NDLOSLOTS(nfiles) * sizeof(uint32_t),
+                   M_FILEDESC, M_WAITOK);
+       }
+
+       simple_lock(&fdp->fd_slock);
+       /* lock fdp */
+       if (fdp->fd_nfiles != oldnfiles) {
+               /* fdp changed; retry */
+               simple_unlock(&fdp->fd_slock);
+               free(newofile, M_FILEDESC);
+               if (newhimap != NULL) free(newhimap, M_FILEDESC);
+               if (newlomap != NULL) free(newlomap, M_FILEDESC);
+               goto restart;
+       }
+
        newofileflags = (char *) &newofile[nfiles];
        /*
         * Copy the existing ofile and ofileflags arrays
@@ -818,26 +859,21 @@
        memcpy(newofileflags, fdp->fd_ofileflags,
            (i = sizeof(char) * fdp->fd_nfiles));
        memset(newofileflags + i, 0, nfiles * sizeof(char) - i);
-       if (fdp->fd_nfiles > NDFILE)
+       if (oldnfiles > NDFILE)
                free(fdp->fd_ofiles, M_FILEDESC);
 
-       if (NDHISLOTS(nfiles) > NDHISLOTS(fdp->fd_nfiles)) {
-               newhimap = malloc(NDHISLOTS(nfiles) * sizeof(uint32_t),
-                   M_FILEDESC, M_WAITOK);
-               newlomap = malloc(NDLOSLOTS(nfiles) * sizeof(uint32_t),
-                   M_FILEDESC, M_WAITOK);
-
+       if (NDHISLOTS(nfiles) > NDHISLOTS(oldnfiles)) {
                memcpy(newhimap, fdp->fd_himap,
-                   (i = NDHISLOTS(fdp->fd_nfiles) * sizeof(uint32_t)));
+                   (i = NDHISLOTS(oldnfiles) * sizeof(uint32_t)));
                memset((char *)newhimap + i, 0,
                    NDHISLOTS(nfiles) * sizeof(uint32_t) - i);
 
                memcpy(newlomap, fdp->fd_lomap,
-                   (i = NDLOSLOTS(fdp->fd_nfiles) * sizeof(uint32_t)));
+                   (i = NDLOSLOTS(oldnfiles) * sizeof(uint32_t)));
                memset((char *)newlomap + i, 0,
                    NDLOSLOTS(nfiles) * sizeof(uint32_t) - i);
 
-               if (NDHISLOTS(fdp->fd_nfiles) > NDHISLOTS(NDFILE)) {
+               if (NDHISLOTS(oldnfiles) > NDHISLOTS(NDFILE)) {
                        free(fdp->fd_himap, M_FILEDESC);
                        free(fdp->fd_lomap, M_FILEDESC);
                }
@@ -848,6 +884,9 @@
        fdp->fd_ofiles = newofile;
        fdp->fd_ofileflags = newofileflags;
        fdp->fd_nfiles = nfiles;
+
+       simple_unlock(&fdp->fd_slock);
+
        fdexpanded++;
 }
 
@@ -965,6 +1004,7 @@
 
        cwdi = pool_get(&cwdi_pool, PR_WAITOK);
 
+       simple_lock_init(&cwdi->cwdi_slock);
        cwdi->cwdi_cdir = p->p_cwdi->cwdi_cdir;
        if (cwdi->cwdi_cdir)
                VREF(cwdi->cwdi_cdir);
@@ -983,9 +1023,12 @@
 void
 cwdshare(struct proc *p1, struct proc *p2)
 {
+       struct cwdinfo *cwdi = p1->p_cwdi;
 
-       p2->p_cwdi = p1->p_cwdi;
-       p1->p_cwdi->cwdi_refcnt++;
+       simple_lock(&cwdi->cwdi_slock);
+       cwdi->cwdi_refcnt++;
+       simple_unlock(&cwdi->cwdi_slock);
+       p2->p_cwdi = cwdi;
 }
 
 /*
@@ -995,30 +1038,31 @@
 void
 cwdunshare(struct proc *p)
 {
-       struct cwdinfo *newcwdi;
+       struct cwdinfo *oldcwdi, *newcwdi;
 
        if (p->p_cwdi->cwdi_refcnt == 1)
                return;
 
        newcwdi = cwdinit(p);
-       cwdfree(p);
+       oldcwdi = p->p_cwdi;
        p->p_cwdi = newcwdi;
+       cwdfree(oldcwdi);
 }
 
 /*
  * Release a cwdinfo structure.
  */
 void
-cwdfree(struct proc *p)
+cwdfree(struct cwdinfo *cwdi)



Home | Main Index | Thread Index | Old Index