Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Aug 2007 12:47:25 +0200
From:      Ulf Lilleengen <lulf@stud.ntnu.no>
To:        freebsd-current@freebsd.org
Cc:        kib@FreeBSD.org
Subject:   [PATCH] Make fdescfs MPSAFE
Message-ID:  <20070814104724.GA13068@stud.ntnu.no>

next in thread | raw e-mail | index | archive | help

--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--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070814104724.GA13068>