From owner-freebsd-current@FreeBSD.ORG Tue Aug 14 10:47:37 2007 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 714E816A41B for ; Tue, 14 Aug 2007 10:47:37 +0000 (UTC) (envelope-from lulf@stud.ntnu.no) Received: from merke.itea.ntnu.no (merke.itea.ntnu.no [129.241.7.61]) by mx1.freebsd.org (Postfix) with ESMTP id C8DCB13C465 for ; Tue, 14 Aug 2007 10:47:36 +0000 (UTC) (envelope-from lulf@stud.ntnu.no) Received: from localhost (localhost [127.0.0.1]) by merke.itea.ntnu.no (Postfix) with ESMTP id 3B9E713D03C; Tue, 14 Aug 2007 12:47:34 +0200 (CEST) Received: from caracal.stud.ntnu.no (caracal.stud.ntnu.no [129.241.56.185]) by merke.itea.ntnu.no (Postfix) with ESMTP; Tue, 14 Aug 2007 12:47:25 +0200 (CEST) Received: by caracal.stud.ntnu.no (Postfix, from userid 2312) id 10BFE6240E2; Tue, 14 Aug 2007 12:47:25 +0200 (CEST) Date: Tue, 14 Aug 2007 12:47:25 +0200 From: Ulf Lilleengen To: freebsd-current@freebsd.org Message-ID: <20070814104724.GA13068@stud.ntnu.no> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="fUYQa+Pmc3FrFX/N" Content-Disposition: inline User-Agent: Mutt/1.5.9i X-Content-Scanned: with sophos and spamassassin at mailgw.ntnu.no. Cc: kib@FreeBSD.org Subject: [PATCH] Make fdescfs MPSAFE X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Aug 2007 10:47:37 -0000 --fUYQa+Pmc3FrFX/N Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi, To be able to better understand VFS and locking in general, I started making fdescfs MPSAFE. I'm not experienced with any of these things, so there might be some errors, although I've looked through much VFS code and code for other FS like nullfs. I've tested it by running two pthreads on the same fd, and that seamt to work, but there might be other cases where it will fail. Patch is attached. -- Ulf Lilleengen --fUYQa+Pmc3FrFX/N Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="fdescfs_locking.diff" Index: sys/fs/fdescfs/fdesc.h =================================================================== RCS file: /home/ncvs/src/sys/fs/fdescfs/fdesc.h,v retrieving revision 1.20 diff -u -r1.20 fdesc.h --- sys/fs/fdescfs/fdesc.h 10 Feb 2005 12:09:15 -0000 1.20 +++ sys/fs/fdescfs/fdesc.h 13 Aug 2007 23:29:21 -0000 @@ -31,7 +31,7 @@ * * @(#)fdesc.h 8.5 (Berkeley) 1/21/94 * - * $FreeBSD: src/sys/fs/fdescfs/fdesc.h,v 1.20 2005/02/10 12:09:15 phk Exp $ + * $FreeBSD$ */ #ifdef _KERNEL @@ -59,6 +59,7 @@ #define VTOFDESC(vp) ((struct fdescnode *)(vp)->v_data) extern vfs_init_t fdesc_init; +extern vfs_uninit_t fdesc_uninit; extern int fdesc_allocvp(fdntype, int, struct mount *, struct vnode **, struct thread *); #endif /* _KERNEL */ Index: sys/fs/fdescfs/fdesc_vfsops.c =================================================================== RCS file: /home/ncvs/src/sys/fs/fdescfs/fdesc_vfsops.c,v retrieving revision 1.56 diff -u -r1.56 fdesc_vfsops.c --- sys/fs/fdescfs/fdesc_vfsops.c 4 Apr 2007 09:11:32 -0000 1.56 +++ sys/fs/fdescfs/fdesc_vfsops.c 13 Aug 2007 23:29:21 -0000 @@ -31,7 +31,7 @@ * * @(#)fdesc_vfsops.c 8.4 (Berkeley) 1/21/94 * - * $FreeBSD: src/sys/fs/fdescfs/fdesc_vfsops.c,v 1.56 2007/04/04 09:11:32 rwatson Exp $ + * $FreeBSD$ */ /* @@ -85,17 +85,22 @@ if (mp->mnt_flag & (MNT_UPDATE | MNT_ROOTFS)) return (EOPNOTSUPP); - error = fdesc_allocvp(Froot, FD_ROOT, mp, &rvp, td); - if (error) - return (error); - MALLOC(fmp, struct fdescmount *, sizeof(struct fdescmount), M_FDESCMNT, M_WAITOK); /* XXX */ + error = fdesc_allocvp(Froot, FD_ROOT, mp, &rvp, td); + if (error) { + free(fmp, M_FDESCMNT); + return (error); + } rvp->v_type = VDIR; rvp->v_vflag |= VV_ROOT; fmp->f_root = rvp; + VOP_UNLOCK(rvp, 0, td); /* XXX -- don't mark as local to work around fts() problems */ /*mp->mnt_flag |= MNT_LOCAL;*/ + MNT_ILOCK(mp); + mp->mnt_kern_flag |= MNTK_MPSAFE; + MNT_IUNLOCK(mp); mp->mnt_data = (qaddr_t) fmp; vfs_getnewfsid(mp); @@ -148,8 +153,8 @@ * Return locked reference to root. */ vp = VFSTOFDESC(mp)->f_root; - VREF(vp); vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td); + VREF(vp); *vpp = vp; return (0); } @@ -209,6 +214,7 @@ .vfs_root = fdesc_root, .vfs_statfs = fdesc_statfs, .vfs_unmount = fdesc_unmount, + .vfs_uninit = fdesc_uninit, }; VFS_SET(fdesc_vfsops, fdescfs, VFCF_SYNTHETIC); Index: sys/fs/fdescfs/fdesc_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/fs/fdescfs/fdesc_vnops.c,v retrieving revision 1.104 diff -u -r1.104 fdesc_vnops.c --- sys/fs/fdescfs/fdesc_vnops.c 4 Apr 2007 09:11:32 -0000 1.104 +++ sys/fs/fdescfs/fdesc_vnops.c 13 Aug 2007 23:29:21 -0000 @@ -31,7 +31,7 @@ * * @(#)fdesc_vnops.c 8.9 (Berkeley) 1/21/94 * - * $FreeBSD: src/sys/fs/fdescfs/fdesc_vnops.c,v 1.104 2007/04/04 09:11:32 rwatson Exp $ + * $FreeBSD$ */ /* @@ -58,7 +58,7 @@ #define FDL_WANT 0x01 #define FDL_LOCKED 0x02 -static int fdcache_lock; +static struct mtx fdesc_hashmtx; #define NFDCACHE 4 #define FD_NHASH(ix) \ @@ -97,9 +97,32 @@ { fdhashtbl = hashinit(NFDCACHE, M_CACHE, &fdhash); + mtx_init(&fdesc_hashmtx, "fdeschs", NULL, MTX_DEF); return (0); } +/* Cleanup. */ +int +fdesc_uninit(vfsp) + struct vfsconf *vfsp; +{ + + mtx_destroy(&fdesc_hashmtx); + return (0); +} + +static void +fdesc_insmntque_dtr(struct vnode *vp, void *arg) +{ + struct fdescnode *fd; + + fd = (struct fdescnode *)arg; + free(fd, M_TEMP); + vp->v_data = NULL; + vgone(vp); + vput(vp); +} + int fdesc_allocvp(ftype, ix, mp, vpp, td) fdntype ftype; @@ -114,25 +137,20 @@ fc = FD_NHASH(ix); loop: + mtx_lock(&fdesc_hashmtx); LIST_FOREACH(fd, fc, fd_hash) { if (fd->fd_ix == ix && fd->fd_vnode->v_mount == mp) { - if (vget(fd->fd_vnode, 0, td)) + VI_LOCK(fd->fd_vnode); + mtx_unlock(&fdesc_hashmtx); + if (vget(fd->fd_vnode, LK_EXCLUSIVE | LK_INTERLOCK, td)) { + VI_UNLOCK(fd->fd_vnode); goto loop; + } *vpp = fd->fd_vnode; return (error); } } - - /* - * otherwise lock the array while we call getnewvnode - * since that can block. - */ - if (fdcache_lock & FDL_LOCKED) { - fdcache_lock |= FDL_WANT; - (void) tsleep( &fdcache_lock, PINOD, "fdalvp", 0); - goto loop; - } - fdcache_lock |= FDL_LOCKED; + mtx_unlock(&fdesc_hashmtx); /* * Do the MALLOC before the getnewvnode since doing so afterward @@ -151,23 +169,14 @@ fd->fd_type = ftype; fd->fd_fd = -1; fd->fd_ix = ix; - /* XXX: vnode should be locked here */ - error = insmntque(*vpp, mp); /* XXX: Too early for mpsafe fs */ - if (error != 0) { - free(fd, M_TEMP); - *vpp = NULLVP; + vn_lock(*vpp, LK_EXCLUSIVE | LK_RETRY, td); + error = insmntque1(*vpp, mp, fdesc_insmntque_dtr, fd); + if (error != 0) goto out; - } + mtx_lock(&fdesc_hashmtx); LIST_INSERT_HEAD(fc, fd, fd_hash); - + mtx_unlock(&fdesc_hashmtx); out: - fdcache_lock &= ~FDL_LOCKED; - - if (fdcache_lock & FDL_WANT) { - fdcache_lock &= ~FDL_WANT; - wakeup( &fdcache_lock); - } - return (error); } @@ -233,8 +242,7 @@ if (error) goto bad; VTOFDESC(fvp)->fd_fd = fd; - if (fvp != dvp) - vn_lock(fvp, LK_EXCLUSIVE | LK_RETRY, td); + *vpp = fvp; return (0); @@ -528,7 +536,10 @@ struct vnode *vp = ap->a_vp; struct fdescnode *fd = VTOFDESC(vp); + mtx_lock(&fdesc_hashmtx); LIST_REMOVE(fd, fd_hash); + mtx_unlock(&fdesc_hashmtx); + FREE(vp->v_data, M_TEMP); vp->v_data = 0; --fUYQa+Pmc3FrFX/N--