Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 25 Nov 2008 21:14:00 +0000 (UTC)
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r185319 - in head/sys/cddl: compat/opensolaris/kern compat/opensolaris/sys contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <200811252114.mAPLE0A9035277@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: pjd
Date: Tue Nov 25 21:14:00 2008
New Revision: 185319
URL: http://svn.freebsd.org/changeset/base/185319

Log:
  Fix locking (file descriptor table and Giant around VFS).
  
  Most submitted by:	kib
  Reviewed by:		kib

Modified:
  head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c
  head/sys/cddl/compat/opensolaris/sys/vnode.h
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_config.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c

Modified: head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c
==============================================================================
--- head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c	Tue Nov 25 20:28:33 2008	(r185318)
+++ head/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c	Tue Nov 25 21:14:00 2008	(r185319)
@@ -67,17 +67,25 @@ static void *
 kobj_open_file_vnode(const char *file)
 {
 	struct thread *td = curthread;
+	struct filedesc *fd;
 	struct nameidata nd;
 	int error, flags;
 
-	if (td->td_proc->p_fd->fd_rdir == NULL)
-		td->td_proc->p_fd->fd_rdir = rootvnode;
-	if (td->td_proc->p_fd->fd_cdir == NULL)
-		td->td_proc->p_fd->fd_cdir = rootvnode;
+	fd = td->td_proc->p_fd;
+	FILEDESC_XLOCK(fd);
+	if (fd->fd_rdir == NULL) {
+		fd->fd_rdir = rootvnode;
+		vref(fd->fd_rdir);
+	}
+	if (fd->fd_cdir == NULL) {
+		fd->fd_cdir = rootvnode;
+		vref(fd->fd_cdir);
+	}
+	FILEDESC_XUNLOCK(fd);
 
 	flags = FREAD;
-	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, file, td);
-	error = vn_open_cred(&nd, &flags, 0, curthread->td_ucred, NULL);
+	NDINIT(&nd, LOOKUP, MPSAFE, UIO_SYSSPACE, file, td);
+	error = vn_open_cred(&nd, &flags, O_NOFOLLOW, curthread->td_ucred, NULL);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	if (error != 0)
 		return (NULL);
@@ -120,13 +128,15 @@ kobj_get_filesize_vnode(struct _buf *fil
 {
 	struct vnode *vp = file->ptr;
 	struct vattr va;
-	int error;
+	int error, vfslocked;
 
+	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	vn_lock(vp, LK_SHARED | LK_RETRY);
 	error = VOP_GETATTR(vp, &va, curthread->td_ucred);
 	VOP_UNLOCK(vp, 0);
 	if (error == 0)
 		*size = (uint64_t)va.va_size;
+	VFS_UNLOCK_GIANT(vfslocked);
 	return (error);
 }
 
@@ -159,7 +169,7 @@ kobj_read_file_vnode(struct _buf *file, 
 	struct thread *td = curthread;
 	struct uio auio;
 	struct iovec aiov;
-	int error;
+	int error, vfslocked;
 
 	bzero(&aiov, sizeof(aiov));
 	bzero(&auio, sizeof(auio));
@@ -175,9 +185,11 @@ kobj_read_file_vnode(struct _buf *file, 
 	auio.uio_resid = size;
 	auio.uio_td = td;
 
+	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	vn_lock(vp, LK_SHARED | LK_RETRY);
 	error = VOP_READ(vp, &auio, IO_UNIT | IO_SYNC, td->td_ucred);
 	VOP_UNLOCK(vp, 0);
+	VFS_UNLOCK_GIANT(vfslocked);
 	return (error != 0 ? -1 : size - auio.uio_resid);
 }
 
@@ -211,9 +223,11 @@ kobj_close_file(struct _buf *file)
 	if (file->mounted) {
 		struct vnode *vp = file->ptr;
 		struct thread *td = curthread;
-		int flags = FREAD;
+		int vfslocked;
 
-		vn_close(vp, flags, td->td_ucred, td);
+		vfslocked = VFS_LOCK_GIANT(vp->v_mount);
+		vn_close(vp, FREAD, td->td_ucred, td);
+		VFS_UNLOCK_GIANT(vfslocked);
 	}
 	kmem_free(file, sizeof(*file));
 }

Modified: head/sys/cddl/compat/opensolaris/sys/vnode.h
==============================================================================
--- head/sys/cddl/compat/opensolaris/sys/vnode.h	Tue Nov 25 20:28:33 2008	(r185318)
+++ head/sys/cddl/compat/opensolaris/sys/vnode.h	Tue Nov 25 21:14:00 2008	(r185319)
@@ -149,6 +149,7 @@ vn_openat(char *pnamep, enum uio_seg seg
     int fd)
 {
 	struct thread *td = curthread;
+	struct filedesc *fdc;
 	struct nameidata nd;
 	int error, operation;
 
@@ -164,20 +165,26 @@ vn_openat(char *pnamep, enum uio_seg seg
 	}
 	ASSERT(umask == 0);
 
-	if (td->td_proc->p_fd->fd_rdir == NULL)
-		td->td_proc->p_fd->fd_rdir = rootvnode;
-	if (td->td_proc->p_fd->fd_cdir == NULL)
-		td->td_proc->p_fd->fd_cdir = rootvnode;
+	fdc = td->td_proc->p_fd;
+	FILEDESC_XLOCK(fdc);
+	if (fdc->fd_rdir == NULL) {
+		fdc->fd_rdir = rootvnode;
+		vref(fdc->fd_rdir);
+	}
+	if (fdc->fd_cdir == NULL) {
+		fdc->fd_cdir = rootvnode;
+		vref(fdc->fd_rdir);
+	}
+	FILEDESC_XUNLOCK(fdc);
 
 	if (startvp != NULL)
 		vref(startvp);
-	NDINIT_ATVP(&nd, operation, NOFOLLOW | MPSAFE, UIO_SYSSPACE, pnamep,
-	    startvp, td);
+	NDINIT_ATVP(&nd, operation, MPSAFE, UIO_SYSSPACE, pnamep, startvp, td);
+	filemode |= O_NOFOLLOW;
 	error = vn_open_cred(&nd, &filemode, createmode, td->td_ucred, NULL);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	if (error == 0) {
 		/* We just unlock so we hold a reference. */
-		VN_HOLD(nd.ni_vp);
 		VOP_UNLOCK(nd.ni_vp, 0);
 		*vpp = nd.ni_vp;
 	}
@@ -248,12 +255,16 @@ drop:
 static __inline int
 zfs_vop_close(vnode_t *vp, int flag, int count, offset_t offset, cred_t *cr)
 {
+	int error, vfslocked;
 
 	ASSERT(flag == (FWRITE | FCREAT | FTRUNC | FOFFMAX));
 	ASSERT(count == 1);
 	ASSERT(offset == 0);
 
-	return (vn_close(vp, flag, cr, curthread));
+	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
+	error = vn_close(vp, flag, cr, curthread);
+	VFS_UNLOCK_GIANT(vfslocked);
+	return (error);
 }
 #define	VOP_CLOSE(vp, oflags, count, offset, cr, ct)			\
 	zfs_vop_close((vp), (oflags), (count), (offset), (cr))

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_config.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_config.c	Tue Nov 25 20:28:33 2008	(r185318)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_config.c	Tue Nov 25 21:14:00 2008	(r185319)
@@ -154,11 +154,10 @@ out:
 static void
 spa_config_write(spa_config_dirent_t *dp, nvlist_t *nvl)
 {
+	int oflags = FWRITE | FTRUNC | FCREAT | FOFFMAX;
+	char *buf, *temp;
 	size_t buflen;
-	char *buf;
 	vnode_t *vp;
-	int oflags = FWRITE | FTRUNC | FCREAT | FOFFMAX;
-	char *temp;
 
 	/*
 	 * If the nvlist is empty (NULL), then remove the old cachefile.
@@ -193,7 +192,6 @@ spa_config_write(spa_config_dirent_t *dp
 			(void) vn_rename(temp, dp->scd_path, UIO_SYSSPACE);
 		}
 		(void) VOP_CLOSE(vp, oflags, 1, 0, kcred, NULL);
-		VN_RELE(vp);
 	}
 
 	(void) vn_remove(temp, UIO_SYSSPACE, RMFILE);

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c	Tue Nov 25 20:28:33 2008	(r185318)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c	Tue Nov 25 21:14:00 2008	(r185319)
@@ -41,7 +41,7 @@ vdev_file_open(vdev_t *vd, uint64_t *psi
 	vdev_file_t *vf;
 	vnode_t *vp;
 	vattr_t vattr;
-	int error;
+	int error, vfslocked;
 
 	/*
 	 * We must have a pathname, and it must be absolute.
@@ -75,6 +75,7 @@ vdev_file_open(vdev_t *vd, uint64_t *psi
 	 * Make sure it's a regular file.
 	 */
 	if (vp->v_type != VREG) {
+		(void) VOP_CLOSE(vp, spa_mode, 1, 0, kcred, NULL);
 		vd->vdev_stat.vs_aux = VDEV_AUX_OPEN_FAILED;
 		return (ENODEV);
 	}
@@ -83,10 +84,13 @@ vdev_file_open(vdev_t *vd, uint64_t *psi
 	 * Determine the physical size of the file.
 	 */
 	vattr.va_mask = AT_SIZE;
+	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	vn_lock(vp, LK_SHARED | LK_RETRY);
 	error = VOP_GETATTR(vp, &vattr, kcred);
 	VOP_UNLOCK(vp, 0);
+	VFS_UNLOCK_GIANT(vfslocked);
 	if (error) {
+		(void) VOP_CLOSE(vp, spa_mode, 1, 0, kcred, NULL);
 		vd->vdev_stat.vs_aux = VDEV_AUX_OPEN_FAILED;
 		return (error);
 	}
@@ -101,18 +105,12 @@ static void
 vdev_file_close(vdev_t *vd)
 {
 	vdev_file_t *vf = vd->vdev_tsd;
-	int vfslocked;
 
 	if (vf == NULL)
 		return;
 
-	if (vf->vf_vnode != NULL) {
-		vfslocked = VFS_LOCK_GIANT(vf->vf_vnode->v_mount);
-		(void)vn_close(vf->vf_vnode, spa_mode, kcred, curthread);
-		VN_RELE(vf->vf_vnode);
-		VFS_UNLOCK_GIANT(vfslocked);
-	}
-
+	if (vf->vf_vnode != NULL)
+		(void) VOP_CLOSE(vf->vf_vnode, spa_mode, 1, 0, kcred, NULL);
 	kmem_free(vf, sizeof (vdev_file_t));
 	vd->vdev_tsd = NULL;
 }
@@ -122,6 +120,7 @@ vdev_file_io_start(zio_t *zio)
 {
 	vdev_t *vd = zio->io_vd;
 	vdev_file_t *vf = vd->vdev_tsd;
+	vnode_t *vp = vf->vf_vnode;
 	ssize_t resid;
 
 	if (zio->io_type == ZIO_TYPE_IOCTL) {
@@ -133,7 +132,7 @@ vdev_file_io_start(zio_t *zio)
 
 		switch (zio->io_cmd) {
 		case DKIOCFLUSHWRITECACHE:
-			zio->io_error = VOP_FSYNC(vf->vf_vnode, FSYNC | FDSYNC,
+			zio->io_error = VOP_FSYNC(vp, FSYNC | FDSYNC,
 			    kcred, NULL);
 			break;
 		default:
@@ -144,9 +143,8 @@ vdev_file_io_start(zio_t *zio)
 	}
 
 	zio->io_error = vn_rdwr(zio->io_type == ZIO_TYPE_READ ?
-	    UIO_READ : UIO_WRITE, vf->vf_vnode, zio->io_data,
-	    zio->io_size, zio->io_offset, UIO_SYSSPACE,
-	    0, RLIM64_INFINITY, kcred, &resid);
+	    UIO_READ : UIO_WRITE, vp, zio->io_data, zio->io_size,
+	    zio->io_offset, UIO_SYSSPACE, 0, RLIM64_INFINITY, kcred, &resid);
 
 	if (resid != 0 && zio->io_error == 0)
 		zio->io_error = ENOSPC;

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