vfs: redo locking

kernel-src/fs/devfs.c

@@ -120,9 +120,7 @@ int devfs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) {
 	devnode_t *devnode = (devnode_t *)node;
 
 	if (devnode->physical && devnode->physical != node) {
-		INTERNAL_LOCK(devnode->physical);
 		int err = VOP_GETATTR(devnode->physical, attr, cred);
-		INTERNAL_UNLOCK(devnode->physical);
 		return err;
 	}
 
@@ -451,6 +449,7 @@ int devfs_getbyname(char *name, vnode_t **ret) {
 	if (error)
 		goto cleanup;
 
+	VOP_UNLOCK(*ret);
 	cleanup:
 	return error;
 }
@@ -480,7 +479,6 @@ int devfs_register(devops_t *devops, char *name, int type, int major, int minor,
 	attr.uid = cred == NULL ? 0 : cred->euid;
 	attr.gid = cred == NULL ? 0 : cred->egid;
 
-
 	vnode_t *dir = NULL;
 	char lastcomp[strlen(name) + 1];
 	error = vfs_lookup(&dir, (vnode_t *)devfsroot, name, lastcomp, VFS_LOOKUP_PARENT);
@@ -493,6 +491,8 @@ int devfs_register(devops_t *devops, char *name, int type, int major, int minor,
 	// the devfs node will have a reference from both the devtable and from the fs link
 	error = VOP_CREATE(dir, lastcomp, &attr, type, &newvnode, NULL);
 
+	// locked by vfs_lookup
+	VOP_UNLOCK(dir);
 	VOP_RELEASE(dir);
 
 	if (error) {
@@ -507,6 +507,8 @@ int devfs_register(devops_t *devops, char *name, int type, int major, int minor,
 	master->attr = newdevnode->attr;
 	master->vnode.type = type;
 	newdevnode->master = master;
+	// locked by VOP_CREATE
+	VOP_UNLOCK(newvnode);
 
 	return 0;
 }

kernel-src/fs/ext2.c

@@ -1071,7 +1071,10 @@ static int ext2_open(vnode_t **vnodep, int flags, cred_t *cred) {
 		if (error)
 			return error;
 
+		VOP_LOCK(fifo);
 		error = VOP_OPEN(&fifo, flags, cred);
+		VOP_UNLOCK(fifo);
+
 		if (error == 0)
 			*vnodep = fifo;
 	}
@@ -1085,8 +1088,6 @@ static int ext2_close(vnode_t *vnode, int flags, cred_t *cred) {
 
 static int ext2_getattr(vnode_t *vnode, vattr_t *attr, cred_t *cred) {
 	ext2node_t *node = (ext2node_t *)vnode;
-	VOP_LOCK(vnode);
-
 	attr->rdevmajor = 0;
 	attr->rdevminor = 0;
 	attr->devmajor = ((ext2fs_t *)vnode->vfs)->backingmajor;
@@ -1106,15 +1107,11 @@ static int ext2_getattr(vnode_t *vnode, vattr_t *attr, cred_t *cred) {
 	attr->mtime.s = node->inode.mtime;
 	attr->mtime.ns = 0;
 	attr->blocksused = ROUND_UP(attr->size, attr->fsblocksize) / attr->fsblocksize;
-
-	VOP_UNLOCK(vnode);
 	return 0;
 }
 
 static int ext2_setattr(vnode_t *vnode, vattr_t *attr, int which, cred_t *cred) {
 	ext2node_t *node = (ext2node_t *)vnode;
-	VOP_LOCK(vnode);
-
 	if (which & V_ATTR_MODE)
 		INODE_TYPEPERM_SETPERM(node->inode.typeperm, attr->mode);
 	if (which & V_ATTR_GID)
@@ -1128,8 +1125,6 @@ static int ext2_setattr(vnode_t *vnode, vattr_t *attr, int which, cred_t *cred)
 	if (which & V_ATTR_ATIME)
 		node->inode.atime = attr->atime.s;
 	int e = writeinode((ext2fs_t *)vnode->vfs, &node->inode, node->id);
-	VOP_UNLOCK(vnode);
-
 	return e;
 }
 
@@ -1137,8 +1132,6 @@ static int ext2_lookup(vnode_t *vnode, char *name, vnode_t **result, cred_t *cre
 	ext2node_t *node = (ext2node_t *)vnode;
 	ext2fs_t *fs = (ext2fs_t *)node->vnode.vfs;
 	ext2node_t *newnode = NULL;
-	VOP_LOCK(vnode);
-
 	int inode = 0;
 	int err = findindir(fs, node, name, &inode, NULL);
 	if (err)
@@ -1181,7 +1174,9 @@ static int ext2_lookup(vnode_t *vnode, char *name, vnode_t **result, cred_t *cre
 
 	MUTEX_RELEASE(&fs->inodetablelock);
 
+	// VOP_LOOKUP returns the vnode locked
 	*result = &newnode->vnode;
+	VOP_LOCK(*result);
 
 	cleanup:
 	if (err && newnode) {
@@ -1189,8 +1184,6 @@ static int ext2_lookup(vnode_t *vnode, char *name, vnode_t **result, cred_t *cre
 		VOP_RELEASE(tmp);
 		MUTEX_RELEASE(&fs->inodetablelock);
 	}
-
-	VOP_UNLOCK(vnode);
 	return err;
 }
 
@@ -1201,8 +1194,6 @@ static int ext2_getdents(vnode_t *vnode, dent_t *buffer, size_t count, uintmax_t
 	if (vnode->type != V_TYPE_DIR)
 		return ENOTDIR;
 
-	VOP_LOCK(vnode);
-
 	int err = 0;
 	// XXX loading the whole dir into memory at once isn't the best idea but works for now
 	void *dirbuffer = vmm_map(NULL, INODE_SIZE(&node->inode) + 1, VMM_FLAGS_ALLOCATE, BUFFER_MAP_FLAGS, NULL); // + 1 to make sure there is at least one page mapped
@@ -1242,7 +1233,6 @@ static int ext2_getdents(vnode_t *vnode, dent_t *buffer, size_t count, uintmax_t
 	if (dirbuffer)
 		vmm_unmap(dirbuffer, INODE_SIZE(&node->inode) + 1, 0);
 
-	VOP_UNLOCK(vnode);
 	return err;
 }
 
@@ -1256,7 +1246,6 @@ static int ext2_readlink(vnode_t *vnode, char **link, cred_t *cred) {
 	if (vnode->type != V_TYPE_LINK)
 		return EINVAL;
 
-	VOP_LOCK(vnode);
 	int err = 0;
 
 	size_t linksize = INODE_SIZE(&node->inode);
@@ -1271,7 +1260,6 @@ static int ext2_readlink(vnode_t *vnode, char **link, cred_t *cred) {
 
 	buf[linksize] = '\0';
 
-	VOP_UNLOCK(vnode);
 	if (err)
 		free(buf);
 	else
@@ -1291,7 +1279,6 @@ static int ext2_read(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset
 	ext2fs_t *fs = (ext2fs_t *)vnode->vfs;
 	uintmax_t endoffset = offset + size;
 	int err = 0;
-	VOP_LOCK(vnode);
 
 	size_t inodesize = INODE_SIZE(&node->inode);
 
@@ -1317,7 +1304,6 @@ static int ext2_read(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset
 	*readc = err ? -1 : size;
 
 	cleanup:
-	VOP_UNLOCK(vnode);
 	return err;
 }
 
@@ -1331,7 +1317,6 @@ int ext2_write(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset, int
 	ext2node_t *node = (ext2node_t *)vnode;
 	ext2fs_t *fs = (ext2fs_t *)vnode->vfs;
 	uintmax_t endoffset = offset + size;
-	VOP_LOCK(vnode);
 	int err = 0;
 	size_t inodesize = INODE_SIZE(&node->inode);
 
@@ -1357,7 +1342,6 @@ int ext2_write(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset, int
 	//ASSERT_UNCLEAN(fs, writeinode(fs, &node->inode, node->id) == 0);
 
 	cleanup:
-	VOP_UNLOCK(vnode);
 	return err;
 }
 
@@ -1371,7 +1355,6 @@ int ext2_resize(vnode_t *vnode, size_t newsize, cred_t *cred) {
 	__assert(vnode->type == V_TYPE_REGULAR);
 	ext2node_t *node = (ext2node_t *)vnode;
 	ext2fs_t *fs = (ext2fs_t *)vnode->vfs;
-	VOP_LOCK(vnode);
 	size_t size = INODE_SIZE(&node->inode);
 	int e = 0;
 	if (size != newsize) {
@@ -1379,7 +1362,6 @@ int ext2_resize(vnode_t *vnode, size_t newsize, cred_t *cred) {
 		if (size > newsize)
 			vmmcache_truncate(vnode, newsize);
 	}
-	VOP_UNLOCK(vnode);
 	return e;
 }
 
@@ -1414,11 +1396,7 @@ static int ext2_link(vnode_t *vnode, vnode_t *dirvnode, char *name, cred_t *cred
 	ext2node_t *node = (ext2node_t *)vnode;
 	ext2node_t *dirnode = (ext2node_t *)dirvnode;
 	ext2fs_t *fs = (ext2fs_t *)dirvnode->vfs;
-	VOP_LOCK(dirvnode);
-	VOP_LOCK(vnode);
 	int err = linkinternal(fs, dirnode, node, name);
-	VOP_UNLOCK(vnode);
-	VOP_UNLOCK(dirvnode);
 	return err;
 }
 
@@ -1482,6 +1460,9 @@ static int internalcreate(vnode_t *parent, char *name, vattr_t *attr, int type,
 		ASSERT_UNCLEAN(fs, changedircount(fs, INODE_GETGROUP(fs, newnode->id), 1) == 0);
 	}
 
+	// VOP_CREATE expects result to be locked
+	VOP_LOCK(*result);
+
 	cleanup:
 	if (err && id)
 		ASSERT_UNCLEAN(fs, freestructure(fs, id, true) == 0);
@@ -1493,9 +1474,7 @@ static int internalcreate(vnode_t *parent, char *name, vattr_t *attr, int type,
 }
 
 static int ext2_create(vnode_t *parent, char *name, vattr_t *attr, int type, vnode_t **result, cred_t *cred) {
-	VOP_LOCK(parent);
 	int err = internalcreate(parent, name, attr, type, result, cred);
-	VOP_UNLOCK(parent);
 	return err;
 }
 
@@ -1503,17 +1482,14 @@ static int ext2_symlink(vnode_t *vnode, char *name, vattr_t *attr, char *path, c
 	ext2fs_t *fs = (ext2fs_t *)vnode->vfs;
 	size_t linklen = strlen(path);
 
-	VOP_LOCK(vnode);
 	vnode_t *newvnode;
 	int err = internalcreate(vnode, name, attr, V_TYPE_LINK, &newvnode, cred);
-	if (err) {
-		VOP_UNLOCK(vnode);
+	if (err)
 		return err;
-	}
 
-	// TODO no lock needed; (probably) not gonna be accessed by anyone else as the direcotry lock is held
-	VOP_LOCK(newvnode);
+	// no lock needed, not gonna be accessed by anyone else as the direcotry lock is held
 	ext2node_t *newnode = (ext2node_t *)newvnode;
+	VOP_UNLOCK(newvnode);
 
 	if (linklen <= 60) {
 		// store it in the inode
@@ -1537,8 +1513,6 @@ static int ext2_symlink(vnode_t *vnode, char *name, vattr_t *attr, char *path, c
 	cleanup:
 	if (err)
 		ASSERT_UNCLEAN(fs, VOP_UNLINK(vnode, name, cred) == 0);
-	VOP_UNLOCK(newvnode);
-	VOP_UNLOCK(vnode);
 	return err;
 }
 
@@ -1618,14 +1592,13 @@ static int internalunlink(ext2fs_t *fs, ext2node_t *node, char *name) {
 	return err;
 }
 
+// TODO name -> child, name
 static int ext2_unlink(vnode_t *vnode, char *name, cred_t *cred) {
 	ext2node_t *node = (ext2node_t *)vnode;
 	ext2fs_t *fs = (ext2fs_t *)vnode->vfs;
-	VOP_LOCK(vnode);
 
 	int err = internalunlink(fs, node, name);
 
-	VOP_UNLOCK(vnode);
 	return err;
 }
 
@@ -1664,6 +1637,7 @@ static int ext2_putpage(vnode_t *node, uintmax_t offset, struct page_t *page) {
 	return error;
 }
 
+// TODO redo this (newname -> targetvnode + newname)
 static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, char *newname, int flags) {
 	if (sourcedir->vfs != targetdir->vfs)
 		return EXDEV;
@@ -1680,12 +1654,13 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch
 	if (err)
 		return err;
 
+	VOP_UNLOCK(source);
+
 	ext2fs_t *fs = (ext2fs_t *)targetdir->vfs;
 	ext2node_t *ext2targetnode = (ext2node_t *)targetdir;
 	ext2node_t *ext2sourcenode = (ext2node_t *)source;
 
 	// switch out or create the target dirent
-	VOP_LOCK(targetdir);
 	int oldinode;
 	err = findindir(fs, ext2targetnode, newname, &oldinode, ext2sourcenode);
 	if (err == ENOENT) {
@@ -1699,13 +1674,10 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch
 	}
 
 	if (err) {
-		VOP_UNLOCK(targetdir);
 		VOP_RELEASE(source);
 		return err;
 	}
 
-	VOP_UNLOCK(targetdir);
-	VOP_LOCK(sourcedir);
 	ext2node_t *ext2sourcedirnode = (ext2node_t *)sourcedir;
 	// unlink the original dirent
 	err = findindir(fs, ext2sourcedirnode, oldname, &oldinode, NULL);
@@ -1718,7 +1690,6 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch
 	}
 
 	cleanup:
-	VOP_UNLOCK(sourcedir);
 	VOP_RELEASE(source);
 	return err;
 }
@@ -1824,7 +1795,9 @@ static int ext2_mount(vfs_t **vfs, vnode_t *mountpoint, vnode_t *backing, void *
 	MUTEX_INIT(&fs->descriptorlock);
 
 	vattr_t vattr;
+	VOP_LOCK(backing);
 	int err = VOP_GETATTR(backing, &vattr, NULL);
+	VOP_UNLOCK(backing);
 	if (err)
 		goto cleanup;
 

kernel-src/fs/initrd.c

@@ -157,6 +157,7 @@ void initrd_unpack() {
 				if (err)
 					break;
 
+				VOP_UNLOCK(node);
 				err = vfs_write(node, datastart, entry.size, 0, &writecount, 0);
 				VOP_RELEASE(node);
 				break;

kernel-src/fs/tmpfs.c

@@ -37,18 +37,15 @@ static int tmpfs_mount(vfs_t **vfs, vnode_t *mountpoint, vnode_t *backing, void
 #define DATAMMUFLAGS (ARCH_MMU_FLAGS_WRITE | ARCH_MMU_FLAGS_READ | ARCH_MMU_FLAGS_NOEXEC)
 
 static int tmpfs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) {
-	VOP_LOCK(node);
 	tmpfsnode_t *tmpnode = (tmpfsnode_t *)node;
 	*attr = tmpnode->attr;
 	attr->blocksused = ROUND_UP(attr->size, PAGE_SIZE) / PAGE_SIZE;
 	attr->devmajor = 0;
 	attr->devminor = ((tmpfs_t *)node->vfs)->id;
-	VOP_UNLOCK(node);
 	return 0;
 }
 
 static int tmpfs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) {
-	VOP_LOCK(node);
 	tmpfsnode_t *tmpnode = (tmpfsnode_t *)node;
 
 	if (which & V_ATTR_GID)
@@ -64,7 +61,6 @@ static int tmpfs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred)
 	if (which & V_ATTR_CTIME)
 		tmpnode->attr.ctime = attr->ctime;
 
-	VOP_UNLOCK(node);
 	return 0;
 }
 
@@ -74,17 +70,17 @@ static int tmpfs_lookup(vnode_t *parent, char *name, vnode_t **result, cred_t *c
 	if (parent->type != V_TYPE_DIR)
 		return ENOTDIR;
 
-	VOP_LOCK(parent);
 	void *r;
 	int err = hashtable_get(&tmpparent->children, &r, name, strlen(name));
-	if (err) {
-		VOP_UNLOCK(parent);
+	if (err)
 		return err;
-	}
 
 	child = r;
 	VOP_HOLD(child);
-	VOP_UNLOCK(parent);
+	// VOP_LOOKUP is expected to return child locked
+	if (child != parent)
+		VOP_LOCK(child);
+
 	*result = child;
 	return 0;
 }
@@ -97,18 +93,13 @@ static int tmpfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vn
 	size_t namelen = strlen(name);
 	void *v;
 
-	VOP_LOCK(parent);
-	if (hashtable_get(&parenttmpnode->children, &v, name, namelen) == 0) {
-		VOP_UNLOCK(parent);
+	if (hashtable_get(&parenttmpnode->children, &v, name, namelen) == 0)
 		return EEXIST;
-	}
 
 	tmpfsnode_t *tmpnode = newnode(parent->vfs, type);
 	vnode_t *node = (vnode_t *)tmpnode;
-	if (tmpnode == NULL) {
-		VOP_UNLOCK(parent);
+	if (tmpnode == NULL)
 		return ENOMEM;
-	}
 
 	timespec_t time = timekeeper_time();
 
@@ -122,23 +113,20 @@ static int tmpfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vn
 	tmpnode->attr.nlinks = 1;
 
 	if (error) {
-		VOP_UNLOCK(parent);
 		VOP_RELEASE(node);
-		return error; 
+		return error;
 	}
 
 	// if a dir, create the .. entry (the . entry is already made in newnode() if it has the dir type)
 	if (type == V_TYPE_DIR) {
 		error = hashtable_set(&tmpnode->children, parenttmpnode, "..", 2, true);
 		if (error) {
-			VOP_UNLOCK(parent);
 			VOP_RELEASE(node);
 			return error;
 		}
 	}
 
 	error = hashtable_set(&parenttmpnode->children, tmpnode, name, namelen, true);
-	VOP_UNLOCK(parent);
 
 	if (error) {
 		VOP_RELEASE(node);
@@ -152,6 +140,8 @@ static int tmpfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vn
 	node->vfs = parent->vfs;
 	*result = node;
 	VOP_HOLD(node);
+	// VOP_CREATE is expected to return child locked
+	VOP_LOCK(node);
 
 	return error;
 }
@@ -171,22 +161,28 @@ static int tmpfs_open(vnode_t **nodep, int flags, cred_t *cred) {
 		if (error)
 			return error;
 
-		VOP_RELEASE(node);
-		VOP_HOLD(devnode);
-		*nodep = devnode; 
+		VOP_LOCK(devnode);
+		error = VOP_OPEN(&devnode, flags, cred);
+		VOP_UNLOCK(devnode);
+		if (error == 0) {
+			VOP_HOLD(devnode);
+			*nodep = devnode;
+		}
 	}
 
 	if (node->type == V_TYPE_FIFO) {
-		VOP_LOCK(*nodep);
 		vnode_t *fifo;
 		error = pipefs_getbinding(*nodep, &fifo);
-		VOP_UNLOCK(*nodep);
 		if (error)
 			return error;
 
+		VOP_LOCK(fifo);
 		error = VOP_OPEN(&fifo, flags, cred);
-		if (error == 0)
+		VOP_UNLOCK(fifo);
+		if (error == 0) {
 			*nodep = fifo;
+			VOP_HOLD(fifo);
+		}
 	}
 
 	return error;
@@ -249,7 +245,6 @@ static int tmpfs_unlink(vnode_t *node, char *name, cred_t *cred) {
 
 	size_t namelen = strlen(name);
 	void *r;
-	VOP_LOCK(node);
 
 	int err = hashtable_get(&tmpnode->children, &r, name, namelen);
 	if (err) {
@@ -261,7 +256,6 @@ static int tmpfs_unlink(vnode_t *node, char *name, cred_t *cred) {
 	tmpfsnode_t *unlinktmpnode = r;
 
 	err = hashtable_remove(&tmpnode->children, name, namelen);
-	VOP_UNLOCK(node);
 	if (err)
 		return err;
 
@@ -286,14 +280,12 @@ static int tmpfs_link(vnode_t *node, vnode_t *dir, char *name, cred_t *cred) {
 	tmpfsnode_t *tmpdir = (tmpfsnode_t *)dir;
 	size_t namelen = strlen(name);
 	void *v;
-	VOP_LOCK(dir);
 	if (hashtable_get(&tmpdir->children, &v, name, namelen) == 0) {
 		VOP_UNLOCK(node);
 		return EEXIST;
 	}
 
 	int error = hashtable_set(&tmpdir->children, node, name, namelen, true);
-	VOP_UNLOCK(dir);
 	if (error)
 		return error;
 
@@ -317,13 +309,6 @@ static int tmpfs_rename(vnode_t *source, char *oldname, vnode_t *target, char *n
 	size_t oldlen = strlen(oldname);
 	size_t newlen = strlen(newname);
 
-	if (target != source) {
-		VOP_LOCK(source);
-		VOP_LOCK(target);
-	} else {
-		VOP_LOCK(target);
-	}
-
 	// get original node
 	void *v;
 	int error = hashtable_get(&sourcedir->children, &v, oldname, oldlen);
@@ -361,13 +346,6 @@ static int tmpfs_rename(vnode_t *source, char *oldname, vnode_t *target, char *n
 		VOP_RELEASE(oldnode);
 
 	cleanup:
-	if (target != source) {
-		VOP_UNLOCK(target);
-		VOP_UNLOCK(source);
-	} else {
-		VOP_UNLOCK(target);
-	}
-
 	return error;
 }
 
@@ -388,9 +366,9 @@ static int tmpfs_symlink(vnode_t *node, char *name, vattr_t *attr, char *path, c
 		return err;
 	}
 
-	VOP_LOCK(linknode);
 	tmpfsnode_t *linktmpnode = (tmpfsnode_t *)linknode;
 	linktmpnode->link = pathbuf;
+	// locked by tmpfs_create
 	VOP_UNLOCK(linknode);
 	VOP_RELEASE(linknode);
 
@@ -403,7 +381,6 @@ static int tmpfs_readlink(vnode_t *node, char **link, cred_t *cred) {
 
 	tmpfsnode_t *tmpnode = (tmpfsnode_t *)node;
 
-	VOP_LOCK(node);
 	char *ret = alloc(strlen(tmpnode->link) + 1);
 	if (ret == NULL) {
 		VOP_UNLOCK(node);
@@ -413,7 +390,6 @@ static int tmpfs_readlink(vnode_t *node, char **link, cred_t *cred) {
 	strcpy(ret, tmpnode->link);
 	*link = ret;
 
-	VOP_UNLOCK(node);
 	return 0;
 }
 
@@ -432,8 +408,6 @@ static int tmpfs_getdents(vnode_t *node, dent_t *buffer, size_t count, uintmax_t
 	if (node->type != V_TYPE_DIR)
 		return ENOTDIR;
 
-	VOP_LOCK(node);
-
 	tmpfsnode_t *tmpnode = (tmpfsnode_t *)node;
 
 	*readcount = 0;
@@ -460,7 +434,6 @@ static int tmpfs_getdents(vnode_t *node, dent_t *buffer, size_t count, uintmax_t
 		*readcount += 1;
 	}
 
-	VOP_UNLOCK(node);
 	return 0;
 }
 
@@ -473,8 +446,6 @@ static int tmpfs_inactive(vnode_t *node) {
 }
 
 static int tmpfs_resize(vnode_t *node, size_t size, cred_t *) {
-	VOP_LOCK(node);
-
 	tmpfsnode_t *tmpfsnode = (tmpfsnode_t *)node;
 	if (size != tmpfsnode->attr.size && tmpfsnode->vnode.type == V_TYPE_REGULAR) {
 		if (tmpfsnode->attr.size > size)
@@ -482,20 +453,16 @@ static int tmpfs_resize(vnode_t *node, size_t size, cred_t *) {
 
 		tmpfsnode->attr.size = size;
 	}
-
-	VOP_UNLOCK(node);
 	return 0;
 }
 
 static int tmpfs_getpage(vnode_t *node, uintmax_t offset, struct page_t *page) {
 	int error = 0;
 
-	VOP_LOCK(node);
 	tmpfsnode_t *tmpfsnode = (tmpfsnode_t *)node;
 	if (offset >= tmpfsnode->attr.size)
 		error = ENXIO;
 
-	VOP_UNLOCK(node);
 	if (error)
 		return error;
 

kernel-src/fs/vfs.c

@@ -62,6 +62,7 @@ void vfs_inactive(vnode_t *vnode) {
 	vnode->ops->inactive(vnode);
 }
 
+// backing and pathref are expected to be unlocked
 int vfs_mount(vnode_t *backing, vnode_t *pathref, char *path, char *name, void *data) {
 	vfsops_t *ops;
 	int err = hashtable_get(&fstable, (void **)(&ops), name, strlen(name));
@@ -75,13 +76,16 @@ int vfs_mount(vnode_t *backing, vnode_t *pathref, char *path, char *name, void *
 		return err;
 
 	err = auth_filesystem_check(getcred(), AUTH_ACTIONS_FILESYSTEM_MOUNT, mounton);
-
 	if (err) {
+		// locked by vfs_lookup
+		VOP_UNLOCK(mounton);
 		VOP_RELEASE(mounton);
 		return err;
 	}
 
 	if (mounton->type != V_TYPE_DIR) {
+		// locked by vfs_lookup
+		VOP_UNLOCK(mounton);
 		VOP_RELEASE(mounton);
 		return ENOTDIR;
 	}
@@ -91,6 +95,8 @@ int vfs_mount(vnode_t *backing, vnode_t *pathref, char *path, char *name, void *
 	err = ops->mount(&vfs, mounton, backing, data);
 
 	if (err) {
+		// locked by vfs_lookup
+		VOP_UNLOCK(mounton);
 		VOP_RELEASE(mounton);
 		return err;
 	}
@@ -105,9 +111,13 @@ int vfs_mount(vnode_t *backing, vnode_t *pathref, char *path, char *name, void *
 	mounton->vfsmounted = vfs;
 	vfs->nodecovered = mounton;
 
+	// locked by vfs_lookup
+	VOP_UNLOCK(mounton);
+
 	return 0;
 }
 
+// ref is expected to be unlocked
 int vfs_open(vnode_t *ref, char *path, int flags, vnode_t **res) {
 	vnode_t *tmp = NULL;
 	int err = vfs_lookup(&tmp, ref, path, NULL, 0);
@@ -115,25 +125,37 @@ int vfs_open(vnode_t *ref, char *path, int flags, vnode_t **res) {
 		return err;
 
 	err = VOP_ACCESS(tmp, ((flags & V_FFLAGS_READ) ? V_ACCESS_READ : 0) | ((flags & V_FFLAGS_WRITE) ? V_ACCESS_WRITE : 0), getcred());
-	if (err)
+	if (err) {
+		// locked by vfs_lookup
+		VOP_UNLOCK(tmp);
 		return err;
+	}
 
-	err = VOP_OPEN(&tmp, flags, getcred());
+	vnode_t *new = ref;
+	err = VOP_OPEN(&new, flags, getcred());
 	if (err) {
 		VOP_RELEASE(tmp);
 	} else {
-		*res = tmp;
+		*res = new != tmp ? new : tmp;
 	}
 
+	// locked by vfs_lookup
+	VOP_UNLOCK(tmp);
+
 	return err;
 }
 
+// node is expected to be unlocked
 int vfs_close(vnode_t *node, int flags) {
+	VOP_LOCK(node);
 	int err = VOP_CLOSE(node, flags, getcred());
+	VOP_UNLOCK(node);
 	return err;
 }
 
 // if node is not NULL, then a reference is kept and the newnode is returned in node
+// ref is expected to be unlocked.
+// the new node is returned locked
 int vfs_create(vnode_t *ref, char *path, vattr_t *attr, int type, vnode_t **node) {
 	vnode_t *parent;
 	char *component = alloc(strlen(path) + 1);
@@ -143,21 +165,29 @@ int vfs_create(vnode_t *ref, char *path, vattr_t *attr, int type, vnode_t **node
 
 	err = VOP_ACCESS(parent, V_ACCESS_WRITE, getcred());
 	if (err) {
+		VOP_UNLOCK(parent);
 		VOP_RELEASE(parent);
-		goto cleanup;
+		goto cleanup_parent;
 	}
 
 	vnode_t *ret;
 	err = VOP_CREATE(parent, component, attr, type, &ret, getcred());
 	VOP_RELEASE(parent);
-	if (err)
+	if (err) {
+		VOP_UNLOCK(parent);
 		goto cleanup;
+	}
 
-	if (node)
+	if (node) {
 		*node = ret;
-	else
+	} else {
+		// returned locked by VOP_CREATE
+		VOP_UNLOCK(ret);
 		VOP_RELEASE(ret);
+	}
 
+	cleanup_parent:
+	VOP_UNLOCK(parent);
 	cleanup:
 	free(component);
 	return err;
@@ -188,12 +218,13 @@ static int writenocache(vnode_t *node, page_t *page, uintmax_t pageoffset) {
 }
 
 int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t *written, int flags) {
-	int err;
+	VOP_LOCK(node);
+	int err = 0;
 	if (node->type == V_TYPE_REGULAR || node->type == V_TYPE_BLKDEV) {
 		*written = 0;
 		// can't write a size 0 buffer
 		if (size == 0)
-			return 0;
+			goto leave;
 
 		// overflow
 		if (size + offset < offset) {
@@ -201,8 +232,6 @@ int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t
 			goto leave;
 		}
 
-		MUTEX_ACQUIRE(&node->sizelock, false);
-
 		vattr_t attr;
 		err = VOP_GETATTR(node, &attr, getcred());
 		if (err)
@@ -248,12 +277,12 @@ int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t
 			pageoffset += 1;
 			pagecount -= 1;
 
-			if (flags & V_FFLAGS_NOCACHE) {
+			if (flags & V_FFLAGS_NOCACHE)
 				err = writenocache(node, page, pageoffset * PAGE_SIZE);
-				if (err)
-					goto leave;
-			}
+
 			pmm_release(FROM_HHDM(address));
+			if (err)
+				goto leave;
 		}
 
 		for (uintmax_t offset = 0; offset < pagecount * PAGE_SIZE; offset += PAGE_SIZE) {
@@ -268,31 +297,32 @@ int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t
 			vmmcache_makedirty(page);
 			*written += writesize;
 
-			if (flags & V_FFLAGS_NOCACHE) {
+			if (flags & V_FFLAGS_NOCACHE)
 				err = writenocache(node, page, pageoffset * PAGE_SIZE + offset);
-				if (err)
-					goto leave;
-			}
-			pmm_release(FROM_HHDM(address));
-		}
 
-		leave:
-		MUTEX_RELEASE(&node->sizelock);
+			pmm_release(FROM_HHDM(address));
+
+			if (err)
+				goto leave;
+		}
 	} else {
 		// special file, just write as its not being cached
 		err = VOP_WRITE(node, buffer, size, offset, flags, written, getcred());
 	}
 
+	leave:
+	VOP_UNLOCK(node);
 	return err;
 }
 
 int vfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t *bytesread, int flags) {
-	int err;
+	VOP_LOCK(node);
+	int err = 0;
 	if (node->type == V_TYPE_REGULAR || node->type == V_TYPE_BLKDEV) {
 		*bytesread = 0;
 		// can't read 0 bytes from the cache
 		if (size == 0)
-			return 0;
+			goto leave;
 
 		// overflow
 		if (size + offset < offset) {
@@ -300,7 +330,6 @@ int vfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t
 			goto leave;
 		}
 
-		MUTEX_ACQUIRE(&node->sizelock, false);
 		size_t nodesize = 0;
 
 		if (node->type == V_TYPE_REGULAR) {
@@ -365,13 +394,12 @@ int vfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t
 			}
 			pmm_release(FROM_HHDM(address));
 		}
-
-		leave:
-		MUTEX_RELEASE(&node->sizelock);
 	} else {
 		// special file, just read as size doesn't matter
 		err = VOP_READ(node, buffer, size, offset, flags, bytesread, getcred());
 	}
+	leave:
+	VOP_UNLOCK(node);
 	return err;
 }
 
@@ -389,22 +417,31 @@ int vfs_link(vnode_t *destref, char *destpath, vnode_t *linkref, char *linkpath,
 		goto cleanup;
 
 	err = VOP_ACCESS(parent, V_ACCESS_WRITE, getcred());
-	if (err)
+	if (err) {
+		// locked by vfs_lookup
+		VOP_UNLOCK(parent);
 		goto cleanup;
+	}
 
+	// TODO rewrite this as it can deadlock as a directory is locked
 	if (type == V_TYPE_REGULAR) {
 		vnode_t *targetnode = NULL;
 		err = vfs_lookup(&targetnode, destref, destpath, NULL, 0);
 		if (err) {
-			VOP_RELEASE(parent);
+			// locked by vfs_lookup
+			VOP_UNLOCK(parent);
 			goto cleanup;
 		}
 		err = VOP_LINK(targetnode, parent, component, getcred());
+		// locked by vfs_lookup
+		VOP_UNLOCK(targetnode);
 		VOP_RELEASE(targetnode);
 	} else {
 		err = VOP_SYMLINK(parent, component, attr, destpath, getcred());
 	}
 
+	// locked by vfs_lookup
+	VOP_UNLOCK(parent);
 	cleanup:
 	VOP_RELEASE(parent);
 	free(component);
@@ -419,6 +456,8 @@ int vfs_unlink(vnode_t *ref, char *path) {
 		goto cleanup;
 
 	VOP_UNLINK(parent, component, getcred());
+	// locked by vfs_lookup
+	VOP_UNLOCK(parent);
 	VOP_RELEASE(parent);
 
 	cleanup:
@@ -446,6 +485,8 @@ static vnode_t *lowestnodeinmp(vnode_t *node) {
 }
 
 // looks up a pathname
+// result will always be returned locked.
+// if flags & VFS_LOOKUP_INPUTLOCKED, the input is already locked and will be locked on return
 // if flags & VFS_LOOKUP_PARENT, the parent of the resulting node will be put in
 // the result pointer and lastcomp will contain the last component name
 // if flags & VFS_LOOKUP_NOLINK, the last component will not be dereferenced if its a symbolic link
@@ -462,10 +503,15 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int
 			return ENOENT;
 		}
 
-		vnode_t *r = start; 
+		vnode_t *r = start;
 		int e = highestnodeinmp(start, &r);
 		if (e)
 			return e;
+
+		if ((flags & VFS_LOOKUP_INPUTLOCKED) == 0) {
+			VOP_LOCK(r);
+		}
+
 		VOP_HOLD(r);
 		*result = r;
 		return 0;
@@ -494,6 +540,9 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int
 	error = 0;
 	VOP_HOLD(current);
 
+	if ((flags & VFS_LOOKUP_INPUTLOCKED) == 0 || current != start)
+		VOP_LOCK(current);
+
 	for (int i = 0; i < pathlen; ++i) {
 		if (compbuffer[i] == '\0')
 			continue;
@@ -510,7 +559,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int
 		// check if its last with trailing '/' (or '\0's in this case, as '/'s were turned into '\0's in the buffer)
 		if (!islast) {
 			int j;
-			for (j = i + complen; j < pathlen && compbuffer[j] == '\0'; ++j);
+			for (j = i + complen; j < pathlen && compbuffer[j] == '\0'; ++j) asm("");
 			islast = j == pathlen;
 		}
 
@@ -528,17 +577,21 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int
 				i += complen;
 				continue;
 			}
+
 			// if the root of a mounted fs, go to
 			if (current->flags & V_FLAGS_ROOT) {
 				vnode_t *n = lowestnodeinmp(current);
 				if (n != current) {
 					VOP_HOLD(n);
+					VOP_UNLOCK(current);
 					VOP_RELEASE(current);
 					current = n;
+					VOP_LOCK(current);
 				}
 			}
 		}
 
+		// locked at the beggining of the function if nescessary or by a previous VOP_LOOKUP
 		error = VOP_ACCESS(current, V_ACCESS_SEARCH, getcred());
 		if (error)
 			break;
@@ -547,24 +600,35 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int
 		if (error)
 			break;
 
+		if ((current != start || (flags & VFS_LOOKUP_INPUTLOCKED) == 0) && current != next)
+			VOP_UNLOCK(current);
+
 		vnode_t *r = next;
 		error = highestnodeinmp(next, &r);
 		if (error) {
+			if (current != next)
+				VOP_UNLOCK(next);
+
 			VOP_RELEASE(next);
 			break;
 		}
 
 		if (r != next) {
 			VOP_HOLD(r);
+			VOP_UNLOCK(next);
 			VOP_RELEASE(next);
 			next = r;
+			VOP_LOCK(next);
 		}
 
 		// dereference symlink
 		if (next->type == V_TYPE_LINK && (islast == false || (islast == true && (flags & VFS_LOOKUP_NOLINK) == 0))) {
 			// get path
 			char *linkderef;
+
+			// next is already returned locked by VOP_LOOKUP
 			error = VOP_READLINK(next, &linkderef, getcred());
+			VOP_UNLOCK(next);
 			if (error) {
 				VOP_RELEASE(next);
 				break;
@@ -587,6 +651,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int
 				VOP_HOLD(derefstart);
 			}
 
+			// TODO possible deadlock when input vnode is locked and the vfs_lookup would lock it
 			error = vfs_lookup(&derefresult, derefstart, linkderef, NULL, pass);
 
 			if (*linkderef == '/')
@@ -595,8 +660,9 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int
 			free(linkderef);
 
 			VOP_RELEASE(next);
-			if (error)
+			if (error) {
 				break;
+			}
 
 			next = derefresult;
 		}
@@ -607,8 +673,13 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int
 	}
 
 	if (error) {
+		// current is locked here, unlock it if not the start or start wasnt locked
+		if (current != start || (flags & VFS_LOOKUP_INPUTLOCKED) == 0)
+			VOP_UNLOCK(current);
+
 		VOP_RELEASE(current);
 	} else {
+		// result is already locked here
 		*result = current;
 	}