Revert "vfs/zfs: remove per-file lock enforcement for ZFS read/write …

…ops" This reverts commit b5eadc3. It introduces a deadlock as outlined in issue #504. Signed-off-by: Pekka Enberg <[email protected]>
cloudius-systems · Oct 1, 2014 · 697943f · 697943f
1 parent 1545b50
commit 697943f
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 16 deletions.
diff --git a/bsd/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c b/bsd/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
@@ -570,16 +570,15 @@ zfs_read(vnode_t *vp, struct file* fp, uio_t *uio, int ioflag)
 	xuio_t		*xuio = NULL;
 #endif
 
+	// Return EISDIR when reading from a directory, as Linux does.
+	if (vp->v_type == VDIR) {
+	    return EISDIR;
+	}
+
 	ZFS_ENTER(zfsvfs);
 	ZFS_VERIFY_ZP(zp);
 	os = zfsvfs->z_os;
 
-	// Return EISDIR when reading from a directory, as Linux does.
-	if (S_ISDIR(zp->z_mode)) {
-		ZFS_EXIT(zfsvfs);
-		return EISDIR;
-	}
-
 	if (zp->z_pflags & ZFS_AV_QUARANTINED) {
 		ZFS_EXIT(zfsvfs);
 		return (EACCES);
@@ -974,11 +973,8 @@ zfs_write(vnode_t *vp, uio_t *uio, int ioflag)
 		 */
 		nbytes = MIN(n, max_blksz - P2PHASE(woff, max_blksz));
 
-		if (woff + nbytes > zp->z_size) {
-			vn_lock(vp);
+		if (woff + nbytes > zp->z_size)
 			vnode_pager_setsize(vp, woff + nbytes);
-			vn_unlock(vp);
-		}
 
 		if (abuf == NULL) {
 			tx_bytes = uio->uio_resid;

diff --git a/core/pagecache.cc b/core/pagecache.cc
@@ -204,12 +204,17 @@ class cached_page_write : public cached_page {
     }
     int writeback()
     {
+        int error;
         struct iovec iov {_page, mmu::page_size};
         struct uio uio {&iov, 1, _key.offset, mmu::page_size, UIO_WRITE};
 
         _dirty = false;
 
-        return VOP_WRITE(_vp, &uio, 0);
+        vn_lock(_vp);
+        error = VOP_WRITE(_vp, &uio, 0);
+        vn_unlock(_vp);
+
+        return error;
     }
     void* release() { // called to demote a page from cache page to anonymous
         assert(boost::get<std::nullptr_t>(_ptes) == nullptr);

diff --git a/fs/devfs/devfs_vnops.cc b/fs/devfs/devfs_vnops.cc
@@ -104,14 +104,12 @@ devfs_close(struct vnode *vp, struct file *fp)
 static int
 devfs_read(struct vnode *vp, struct file *fp, struct uio *uio, int ioflags)
 {
-	SCOPE_LOCK(vp->v_lock);
 	return device_read((device*)vp->v_data, uio, ioflags);
 }
 
 static int
 devfs_write(struct vnode *vp, struct uio *uio, int ioflags)
 {
-	SCOPE_LOCK(vp->v_lock);
 	return device_write((device*)vp->v_data, uio, ioflags);
 }
 

diff --git a/fs/procfs/procfs_vnops.cc b/fs/procfs/procfs_vnops.cc
@@ -147,7 +147,6 @@ static int
 procfs_read(vnode* vp, file *fp, uio* uio, int ioflags)
 {
     auto* data = static_cast<string*>(fp->f_data);
-    SCOPE_LOCK(vp->v_lock);
 
     if (vp->v_type == VDIR)
         return EISDIR;

diff --git a/fs/ramfs/ramfs_vnops.cc b/fs/ramfs/ramfs_vnops.cc
@@ -299,7 +299,6 @@ ramfs_read(struct vnode *vp, struct file *fp, struct uio *uio, int ioflag)
 {
 	struct ramfs_node *np = (ramfs_node*)vp->v_data;
 	size_t len;
-	SCOPE_LOCK(vp->v_lock);
 
 	if (vp->v_type == VDIR)
 		return EISDIR;
@@ -326,7 +325,6 @@ static int
 ramfs_write(struct vnode *vp, struct uio *uio, int ioflag)
 {
 	struct ramfs_node *np = (ramfs_node*)vp->v_data;
-	SCOPE_LOCK(vp->v_lock);
 
 	if (vp->v_type == VDIR)
 		return EISDIR;

diff --git a/fs/vfs/vfs_fops.cc b/fs/vfs/vfs_fops.cc
@@ -47,6 +47,7 @@ int vfs_file::read(struct uio *uio, int flags)
 
 	bytes = uio->uio_resid;
 
+	vn_lock(vp);
 	if ((flags & FOF_OFFSET) == 0)
 		uio->uio_offset = fp->f_offset;
 
@@ -56,6 +57,7 @@ int vfs_file::read(struct uio *uio, int flags)
 		if ((flags & FOF_OFFSET) == 0)
 			fp->f_offset += count;
 	}
+	vn_unlock(vp);
 
 	return error;
 }
@@ -72,6 +74,8 @@ int vfs_file::write(struct uio *uio, int flags)
 
 	bytes = uio->uio_resid;
 
+	vn_lock(vp);
+
 	if (fp->f_flags & O_APPEND)
 		ioflags |= IO_APPEND;
 	if (fp->f_flags & (O_DSYNC|O_SYNC))
@@ -87,6 +91,7 @@ int vfs_file::write(struct uio *uio, int flags)
 			fp->f_offset += count;
 	}
 
+	vn_unlock(vp);
 	return error;
 }
 
@@ -166,7 +171,9 @@ int vfs_file::get_arcbuf(void* key, off_t offset)
     data.uio_resid = mmu::page_size;
     data.uio_rw = UIO_READ;
 
+    vn_lock(vp);
     assert(VOP_CACHE(vp, this, &data) == 0);
+    vn_unlock(vp);
 
     return (data.uio_resid != 0) ? -1 : 0;
 }