From owner-p4-projects@FreeBSD.ORG Mon Mar 27 19:30:05 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 0B56016A42A; Mon, 27 Mar 2006 19:30:05 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DDB3716A428 for ; Mon, 27 Mar 2006 19:30:04 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9089343D49 for ; Mon, 27 Mar 2006 19:30:04 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.1/8.13.1) with ESMTP id k2RJU4nr041939 for ; Mon, 27 Mar 2006 19:30:04 GMT (envelope-from jhb@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.1/8.13.1/Submit) id k2RJU4WT041935 for perforce@freebsd.org; Mon, 27 Mar 2006 19:30:04 GMT (envelope-from jhb@freebsd.org) Date: Mon, 27 Mar 2006 19:30:04 GMT Message-Id: <200603271930.k2RJU4WT041935@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jhb@freebsd.org using -f From: John Baldwin To: Perforce Change Reviews Cc: Subject: PERFORCE change 94124 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Mar 2006 19:30:05 -0000 http://perforce.freebsd.org/chv.cgi?CH=94124 Change 94124 by jhb@jhb_slimer on 2006/03/27 19:29:53 - Conditionalize Giant for VFS operations. - Use the right msleep wait channel to avoid possible panics due to races with kldunload when tearing down the kthread and use the queue mutex to protect writes to the flags member of the softc once a md device is created. Affected files ... .. //depot/projects/smpng/sys/dev/md/md.c#75 edit Differences ... ==== //depot/projects/smpng/sys/dev/md/md.c#75 (text+ko) ==== @@ -88,7 +88,8 @@ #define MD_MODVER 1 -#define MD_SHUTDOWN 0x10000 /* Tell worker thread to terminate. */ +#define MD_SHUTDOWN 0x10000 /* Tell worker thread to terminate. */ +#define MD_EXITING 0x20000 /* Worker thread is exiting. */ #ifndef MD_NSECT #define MD_NSECT (10000 * 2) @@ -485,12 +486,11 @@ static int mdstart_vnode(struct md_s *sc, struct bio *bp) { - int error; + int error, vfslocked; struct uio auio; struct iovec aiov; struct mount *mp; - mtx_assert(&Giant, MA_OWNED); /* * VNODE I/O * @@ -519,6 +519,7 @@ * When reading set IO_DIRECT to try to avoid double-caching * the data. When writing IO_DIRECT is not optimal. */ + vfslocked = VFS_LOCK_GIANT(sc->vnode->v_mount); if (bp->bio_cmd == BIO_READ) { vn_lock(sc->vnode, LK_EXCLUSIVE | LK_RETRY, curthread); error = VOP_READ(sc->vnode, &auio, IO_DIRECT, sc->cred); @@ -531,6 +532,7 @@ VOP_UNLOCK(sc->vnode, 0, curthread); vn_finished_write(mp); } + VFS_UNLOCK_GIANT(vfslocked); bp->bio_resid = auio.uio_resid; return (error); } @@ -641,35 +643,20 @@ { struct md_s *sc; struct bio *bp; - int error, hasgiant; + int error; sc = arg; mtx_lock_spin(&sched_lock); sched_prio(curthread, PRIBIO); mtx_unlock_spin(&sched_lock); - switch (sc->type) { - case MD_VNODE: - mtx_lock(&Giant); - hasgiant = 1; - break; - case MD_MALLOC: - case MD_PRELOAD: - case MD_SWAP: - default: - hasgiant = 0; - break; - } - for (;;) { + mtx_lock(&sc->queue_mtx); if (sc->flags & MD_SHUTDOWN) { - sc->procp = NULL; - wakeup(&sc->procp); - if (hasgiant) - mtx_unlock(&Giant); + sc->flags |= MD_EXITING; + mtx_unlock_spin(&sc->queue_mtx); kthread_exit(0); } - mtx_lock(&sc->queue_mtx); bp = bioq_takefirst(&sc->bio_queue); if (!bp) { msleep(sc, &sc->queue_mtx, PRIBIO | PDROP, "mdwait", 0); @@ -867,7 +854,7 @@ { struct vattr vattr; struct nameidata nd; - int error, flags; + int error, flags, vfslocked; error = copyinstr(mdio->md_file, sc->file, sizeof(sc->file), NULL); if (error != 0) @@ -879,15 +866,17 @@ */ if ((mdio->md_options & MD_READONLY) != 0) flags &= ~FWRITE; - NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, sc->file, td); + NDINIT(&nd, LOOKUP, FOLLOW | MPSAFE, UIO_SYSSPACE, sc->file, td); error = vn_open(&nd, &flags, 0, -1); if (error != 0) return (error); + vfslocked = NDHASGIANT(&nd); NDFREE(&nd, NDF_ONLY_PNBUF); if (nd.ni_vp->v_type != VREG || (error = VOP_GETATTR(nd.ni_vp, &vattr, td->td_ucred, td))) { VOP_UNLOCK(nd.ni_vp, 0, td); (void)vn_close(nd.ni_vp, flags, td->td_ucred, td); + VFS_UNLOCK_GIANT(vfslocked); return (error ? error : EINVAL); } VOP_UNLOCK(nd.ni_vp, 0, td); @@ -904,15 +893,17 @@ error = mdsetcred(sc, td->td_ucred); if (error != 0) { (void)vn_close(nd.ni_vp, flags, td->td_ucred, td); + VFS_UNLOCK_GIANT(vfslocked); return (error); } + VFS_UNLOCK_GIANT(vfslocked); return (0); } static int mddestroy(struct md_s *sc, struct thread *td) { - + int vfslocked; if (sc->gp) { sc->gp->softc = NULL; @@ -922,16 +913,18 @@ sc->gp = NULL; sc->pp = NULL; } + mtx_lock(&sc->queue_mtx); sc->flags |= MD_SHUTDOWN; wakeup(sc); - while (sc->procp != NULL) - tsleep(&sc->procp, PRIBIO, "mddestroy", hz / 10); + while (!(sc->flags & MD_EXITING)) + msleep(sc->procp, &sc->queue_mtx, PRIBIO, "mddestroy", hz / 10); + mtx_unlock(&sc->queue_mtx); mtx_destroy(&sc->queue_mtx); if (sc->vnode != NULL) { - mtx_lock(&Giant); + vfslocked = VFS_LOCK_GIANT(sc->vnode->v_mount); (void)vn_close(sc->vnode, sc->flags & MD_READONLY ? FREAD : (FREAD|FWRITE), sc->cred, td); - mtx_unlock(&Giant); + VFS_UNLOCK_GIANT(vfslocked); } if (sc->cred != NULL) crfree(sc->cred);