vfs: leave handling of VOP_LOCK and VOP_UNLOCK to each filesystem

kernel-src/fs/devfs.c

@@ -21,6 +21,9 @@ static uintmax_t currentinode;
 static vfsops_t vfsops;
 static vops_t vnops;
 
+#define INTERNAL_LOCK(v) MUTEX_ACQUIRE(&(v)->lock, false)
+#define INTERNAL_UNLOCK(v) MUTEX_RELEASE(&(v)->lock);
+
 static int devfs_mount(vfs_t **vfs, vnode_t *mountpoint, vnode_t *backing, void *data) {
 	vfs_t *vfsp = alloc(sizeof(vfs_t));
 	if (vfs == NULL)
@@ -96,18 +99,18 @@ int devfs_lookup(vnode_t *node, char *name, vnode_t **result, cred_t *cred) {
 		return ENOTDIR;
 
 	void *v;
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	MUTEX_ACQUIRE(&tablelock, false);
 	int error = hashtable_get(&devnode->children, &v, name, strlen(name));
 	MUTEX_RELEASE(&tablelock);
 	if (error) {
-		VOP_UNLOCK(node);
+		INTERNAL_UNLOCK(node);
 		return error;
 	}
 
 	vnode_t *rnode = v;
 	VOP_HOLD(rnode);
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	*result = rnode;
 
 	return 0;
@@ -117,16 +120,16 @@ int devfs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) {
 	devnode_t *devnode = (devnode_t *)node;
 
 	if (devnode->physical && devnode->physical != node) {
-		VOP_LOCK(devnode->physical);
+		INTERNAL_LOCK(devnode->physical);
 		int err = VOP_GETATTR(devnode->physical, attr, cred);
-		VOP_UNLOCK(devnode->physical);
+		INTERNAL_UNLOCK(devnode->physical);
 		return err;
 	}
 
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	*attr = devnode->attr;
 	attr->type = node->type;
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 
 	return 0;
 }
@@ -139,7 +142,7 @@ int devfs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) {
 		return err;
 	}
 
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	if (which & V_ATTR_GID)
 		devnode->attr.gid = attr->gid;
 	if (which & V_ATTR_UID)
@@ -152,7 +155,7 @@ int devfs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) {
 		devnode->attr.mtime = attr->mtime;
 	if (which & V_ATTR_CTIME)
 		devnode->attr.ctime = attr->ctime;
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	return 0;
 }
 
@@ -218,7 +221,7 @@ int devfs_maxseek(vnode_t *node, size_t *max) {
 }
 
 int devfs_inactive(vnode_t *node) {
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	devnode_t *devnode = (devnode_t *)node;
 	vnode_t *master = (vnode_t *)devnode->master;
 	if (devnode->physical && devnode->physical != node)
@@ -242,15 +245,15 @@ int devfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vnode_t *
 
 	size_t namelen = strlen(name);
 	void *v;
-	VOP_LOCK(parent);
+	INTERNAL_LOCK(parent);
 	if (hashtable_get(&parentdevnode->children, &v, name, namelen) == 0) {
-		VOP_UNLOCK(parent);
+		INTERNAL_UNLOCK(parent);
 		return EEXIST;
 	}
 
 	devnode_t *node = slab_allocate(nodecache);
 	if (node == NULL) {
-		VOP_UNLOCK(parent);
+		INTERNAL_UNLOCK(parent);
 		return ENOMEM;
 	}
 
@@ -272,13 +275,13 @@ int devfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vnode_t *
 		int error = hashtable_init(&node->children, 100);
 		if (error) {
 			slab_free(nodecache, node);
-			VOP_UNLOCK(parent);
+			INTERNAL_UNLOCK(parent);
 			return error;
 		}
 		if (hashtable_set(&node->children, node, ".", 1, true) || hashtable_set(&node->children, parent, "..", 2, true)) {
 			hashtable_destroy(&node->children);
 			slab_free(nodecache, node);
-			VOP_UNLOCK(parent);
+			INTERNAL_UNLOCK(parent);
 			return error;
 		}
 	}
@@ -287,7 +290,7 @@ int devfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vnode_t *
 	if (error) {
 		if (type == V_TYPE_DIR)
 			hashtable_destroy(&node->children);
-		VOP_UNLOCK(parent);
+		INTERNAL_UNLOCK(parent);
 		slab_free(nodecache, node);
 		return error;
 	}
@@ -297,7 +300,7 @@ int devfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vnode_t *
 		VOP_HOLD(parent);
 
 	VOP_HOLD(&node->vnode);
-	VOP_UNLOCK(parent);
+	INTERNAL_UNLOCK(parent);
 	*result = &node->vnode;
 
 	return 0;
@@ -307,7 +310,7 @@ static int devfs_getdents(vnode_t *node, dent_t *buffer, size_t count, uintmax_t
 	if (node->type != V_TYPE_DIR)
 		return ENOTDIR;
 
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 
 	devnode_t *devnode = (devnode_t *)node;
 
@@ -336,7 +339,7 @@ static int devfs_getdents(vnode_t *node, dent_t *buffer, size_t count, uintmax_t
 		*readcount += 1;
 	}
 
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	return 0;
 }
 
@@ -365,6 +368,21 @@ static int devfs_sync(vnode_t *vnode) {
 	return vmmcache_sync(vnode);
 }
 
+// most locking is handled by the devices.
+// we only care about the tree side of things, and each function
+// will have their own handling of the vnode mutex with the
+// INTERNAL_LOCK and INTERNAL_UNLOCK macros.
+//
+// the current handling does mean that it is not atomic between operations.
+// a future locking implementation could lock only specific types, such as directories.
+static int devfs_lock(vnode_t *vnode) {
+	return 0;
+}
+
+static int devfs_unlock(vnode_t *vnode) {
+	return 0;
+}
+
 static int devfs_enodev() {
 	return ENODEV;
 }
@@ -400,7 +418,9 @@ static vops_t vnops = {
 	.rename = devfs_enodev,
 	.putpage = devfs_putpage,
 	.getpage = devfs_getpage,
-	.sync = devfs_sync
+	.sync = devfs_sync,
+	.lock = devfs_lock,
+	.unlock = devfs_unlock
 };
 
 static void ctor(scache_t *cache, void *obj) {
@@ -530,7 +550,7 @@ void devfs_remove(char *name, int major, int minor) {
 	__assert(vfs_lookup(&parent, (vnode_t *)devfsroot, name, namebuff, VFS_LOOKUP_PARENT) == 0);
 	devnode_t *parentdevnode = (devnode_t *)parent;
 
-	VOP_LOCK(parent);
+	INTERNAL_LOCK(parent);
 
 	// get device vnode
 	void *tmp;
@@ -541,7 +561,7 @@ void devfs_remove(char *name, int major, int minor) {
 
 	__assert(hashtable_remove(&parentdevnode->children, namebuff, strlen(namebuff)) == 0);
 
-	VOP_UNLOCK(parent);
+	INTERNAL_UNLOCK(parent);
 
 	// release all the references to the removed node:
 	VOP_RELEASE(vnode);

kernel-src/fs/ext2.c

@@ -1511,6 +1511,7 @@ static int ext2_symlink(vnode_t *vnode, char *name, vattr_t *attr, char *path, c
 		return err;
 	}
 
+	// TODO no lock needed; (probably) not gonna be accessed by anyone else as the direcotry lock is held
 	VOP_LOCK(newvnode);
 	ext2node_t *newnode = (ext2node_t *)newvnode;
 
@@ -1731,6 +1732,16 @@ static int ext2_sync(vnode_t *vnode) {
 	return e ? e : e2;
 }
 
+static int ext2_lock(vnode_t *vnode) {
+	MUTEX_ACQUIRE(&vnode->lock, false);
+	return 0;
+}
+
+static int ext2_unlock(vnode_t *vnode) {
+	MUTEX_RELEASE(&vnode->lock);
+	return 0;
+}
+
 static int ext2_inactive(vnode_t *vnode) {
 	ext2node_t *node = (ext2node_t *)vnode;
 	ext2fs_t *fs = (ext2fs_t *)vnode->vfs;
@@ -1915,7 +1926,9 @@ static vops_t vnops = {
 	.rename = ext2_rename,
 	.getpage = ext2_getpage,
 	.putpage = ext2_putpage,
-	.sync = ext2_sync
+	.sync = ext2_sync,
+	.lock = ext2_lock,
+	.unlock = ext2_unlock
 };
 
 void ext2_init() {

kernel-src/fs/pipefs.c

@@ -17,8 +17,11 @@ static uintmax_t currentinode;
 
 static vops_t vnops;
 
+#define INTERNAL_LOCK(v) MUTEX_ACQUIRE(&(v)->lock, false)
+#define INTERNAL_UNLOCK(v) MUTEX_RELEASE(&(v)->lock)
+
 int pipefs_close(vnode_t *node, int flags, cred_t *cred) {
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	pipenode_t *pipenode = (pipenode_t *)node;
 
 	if (flags & V_FFLAGS_READ) {
@@ -35,7 +38,7 @@ int pipefs_close(vnode_t *node, int flags, cred_t *cred) {
 	if (pipenode->writers == 0)
 		poll_event(&pipenode->pollheader, POLLHUP);
 
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	return 0;
 }
 
@@ -43,7 +46,7 @@ int pipefs_open(vnode_t **node, int flags, cred_t *cred) {
 	if ((flags & (V_FFLAGS_WRITE | V_FFLAGS_READ)) == 0)
 		return EINVAL;
 
-	VOP_LOCK(*node);
+	INTERNAL_LOCK(*node);
 	pipenode_t *pipenode = (pipenode_t *)(*node);
 
 	if (flags & V_FFLAGS_READ) {
@@ -77,22 +80,22 @@ int pipefs_open(vnode_t **node, int flags, cred_t *cred) {
 	else
 		EVENT_ATTACH(&listener, &pipenode->readopenevent);
 
-	VOP_UNLOCK(*node);
+	INTERNAL_UNLOCK(*node);
 	error = EVENT_WAIT(&listener, 0);
-	VOP_LOCK(*node);
+	INTERNAL_LOCK(*node);
 
 	EVENT_DETACHALL(&listener);
 
 	leave:
 	if (error) {
-		VOP_UNLOCK(*node);
+		INTERNAL_UNLOCK(*node);
 		pipefs_close(*node, flags, cred);
 	} else {
 		if (flags & V_FFLAGS_READ)
 			EVENT_SIGNAL(&pipenode->readopenevent);
 		else
 			EVENT_SIGNAL(&pipenode->writeopenevent);
-		VOP_UNLOCK(*node);
+		INTERNAL_UNLOCK(*node);
 	}
 
 	return error;
@@ -129,7 +132,7 @@ static int internalpoll(vnode_t *node, polldata_t *data, int events) {
 
 int pipefs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, int flags, size_t *readc, cred_t *cred) {
 	pipenode_t *pipenode = (pipenode_t *)node;
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 
 	int error = 0;
 	*readc = 0;
@@ -154,7 +157,7 @@ int pipefs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, int
 
 		revents = internalpoll(node, &desc.data[0], POLLIN);
 
-		VOP_UNLOCK(node);
+		INTERNAL_UNLOCK(node);
 
 		error = poll_dowait(&desc, 0);
 
@@ -164,7 +167,7 @@ int pipefs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, int
 		if (error)
 			return error;
 
-		VOP_LOCK(node);
+		INTERNAL_LOCK(node);
 	}
 
 	*readc = ringbuffer_read(&pipenode->data, buffer, size);
@@ -174,7 +177,7 @@ int pipefs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, int
 		poll_event(&pipenode->pollheader, POLLOUT);
 
 	leave:
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	return error;
 }
 
@@ -195,7 +198,7 @@ static int writetopipe(pipenode_t *pipenode, void *buffer, size_t size, size_t *
 
 int pipefs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, int flags, size_t *writec, cred_t *cred) {
 	pipenode_t *pipenode = (pipenode_t *)node;
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 
 	int error = 0;
 	*writec = 0;
@@ -269,7 +272,7 @@ int pipefs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, int
 
 		revents = internalpoll(node, &desc.data[0], POLLOUT);
 
-		VOP_UNLOCK(node);
+		INTERNAL_UNLOCK(node);
 
 		error = poll_dowait(&desc, 0);
 
@@ -279,33 +282,33 @@ int pipefs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, int
 		if (error)
 			return error;
 
-		VOP_LOCK(node);
+		INTERNAL_LOCK(node);
 	}
 
 	error = writetopipe(pipenode, buffer, size, writec);
 
 	leave:
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	return error;
 }
 
 int pipefs_poll(vnode_t *node, polldata_t *data, int events) {
 	int revents = 0;
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 
 	revents = internalpoll(node, data, events);
 
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	return revents;
 }
 
 int pipefs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) {
 	pipenode_t *pipenode = (pipenode_t *)node;
 
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	*attr = pipenode->attr;
 	attr->type = node->type;
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 
 	return 0;
 }
@@ -313,7 +316,7 @@ int pipefs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) {
 int pipefs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) {
 	pipenode_t *pipenode = (pipenode_t *)node;
 
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	if (which & V_ATTR_GID)
 		pipenode->attr.gid = attr->gid;
 	if (which & V_ATTR_UID)
@@ -326,18 +329,28 @@ int pipefs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) {
 		pipenode->attr.mtime = attr->mtime;
 	if (which & V_ATTR_CTIME)
 		pipenode->attr.ctime = attr->ctime;
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	return 0;
 }
 
 int pipefs_inactive(vnode_t *node) {
 	pipenode_t *pipenode = (pipenode_t *)node;
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	ringbuffer_destroy(&pipenode->data);
 	slab_free(nodecache, node);
 	return 0;
 }
 
+// the mutex vnode is handled internally
+// (as we can sleep when waiting for data/space)
+// this does mean that operations are not atomic between a VOP_LOCK and VOP_UNLOCK
+static int pipefs_lock(vnode_t *vnode) {
+	return 0;
+}
+
+static int pipefs_unlock(vnode_t *vnode) {
+	return 0;
+}
 
 static int pipefs_enodev() {
 	return ENODEV;
@@ -366,7 +379,9 @@ static vops_t vnops = {
 	.rename = pipefs_enodev,
 	.putpage = pipefs_enodev,
 	.getpage = pipefs_enodev,
-	.sync = pipefs_enodev
+	.sync = pipefs_enodev,
+	.lock = pipefs_lock,
+	.unlock = pipefs_unlock
 };
 
 static void ctor(scache_t *cache, void *obj) {

kernel-src/fs/sockfs.c

@@ -12,6 +12,9 @@
 static scache_t *nodecache;
 static uintmax_t currentinode;
 
+#define INTERNAL_LOCK(v) MUTEX_ACQUIRE(&(v)->lock, false)
+#define INTERNAL_UNLOCK(v) MUTEX_RELEASE(&(v)->lock)
+
 int sockfs_open(vnode_t **node, int flags, cred_t *cred) {
 	return ENOSYS; // not needed
 }
@@ -38,17 +41,17 @@ int sockfs_poll(vnode_t *node, polldata_t *data, int events) {
 int sockfs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) {
 	socketnode_t *socketnode = (socketnode_t *)node;
 
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	*attr = socketnode->attr;
 	attr->type = node->type;
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 
 	return 0;
 }
 
 int sockfs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) {
 	socketnode_t *socketnode = (socketnode_t *)node;
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 
 	if (which & V_ATTR_GID)
 		socketnode->attr.gid = attr->gid;
@@ -63,7 +66,7 @@ int sockfs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) {
 	if (which & V_ATTR_CTIME)
 		socketnode->attr.ctime = attr->ctime;
 
-	VOP_UNLOCK(node);
+	INTERNAL_UNLOCK(node);
 	return 0;
 }
 
@@ -167,12 +170,23 @@ int sockfs_ioctl(vnode_t *node, unsigned long request, void *arg, int *result) {
 
 int sockfs_inactive(vnode_t *node) {
 	socket_t *socket = SOCKFS_SOCKET_FROM_NODE(node);
-	VOP_LOCK(node);
+	INTERNAL_LOCK(node);
 	socket->ops->destroy(socket);
 	slab_free(nodecache, node);
 	return 0;
 }
 
+// the mutex vnode is handled internally
+// (as we can sleep when waiting for data/space, and the handling happens in the socket layer)
+// this does mean that operations are not atomic between a INTERNAL_LOCK and INTERNAL_UNLOCK
+static int sockfs_lock(vnode_t *) {
+	return 0;
+}
+
+static int sockfs_unlock(vnode_t *) {
+	return 0;
+}
+
 static int sockfs_enodev() {
 	return ENODEV;
 }
@@ -201,7 +215,9 @@ static vops_t vnops = {
 	.ioctl = sockfs_ioctl,
 	.putpage = sockfs_enodev,
 	.getpage = sockfs_enodev,
-	.sync = sockfs_enodev
+	.sync = sockfs_enodev,
+	.lock = sockfs_lock,
+	.unlock = sockfs_unlock
 };
 
 static void ctor(scache_t *cache, void *obj) {

kernel-src/fs/tmpfs.c

@@ -520,6 +520,16 @@ static int tmpfs_sync(vnode_t *node) {
 	return 0;
 }
 
+static int tmpfs_lock(vnode_t *node) {
+	MUTEX_ACQUIRE(&node->lock, false);
+	return 0;
+}
+
+static int tmpfs_unlock(vnode_t *node) {
+	MUTEX_RELEASE(&node->lock);
+	return 0;
+}
+
 static vfsops_t vfsops = {
 	.mount = tmpfs_mount,
 	.root = tmpfs_root
@@ -548,7 +558,9 @@ static vops_t vnops = {
 	.rename = tmpfs_rename,
 	.getpage = tmpfs_getpage,
 	.putpage = tmpfs_putpage,
-	.sync = tmpfs_sync
+	.sync = tmpfs_sync,
+	.lock = tmpfs_lock,
+	.unlock = tmpfs_unlock
 };
 
 static tmpfsnode_t *newnode(vfs_t *vfs, int type) {

kernel-src/include/kernel/vfs.h

@@ -142,6 +142,8 @@ typedef struct vops_t {
 	int (*getpage)(vnode_t *node, uintmax_t offset, struct page_t *page);
 	int (*putpage)(vnode_t *node, uintmax_t offset, struct page_t *page);
 	int (*sync)(vnode_t *node);
+	int (*lock)(vnode_t *node);
+	int (*unlock)(vnode_t *node);
 } vops_t;
 
 #define VFS_INIT(v, o, f) \
@@ -166,8 +168,8 @@ typedef struct vops_t {
 	(vn)->vfs = v; \
 	(vn)->vfsmounted = NULL; 
 
-#define VOP_LOCK(v) MUTEX_ACQUIRE(&(v)->lock, false)
+#define VOP_LOCK(v) (v)->ops->lock(v)
-#define VOP_UNLOCK(v) MUTEX_RELEASE(&(v)->lock)
+#define VOP_UNLOCK(v) (v)->ops->unlock(v)
 
 #define VOP_OPEN(v, f, c) (*v)->ops->open(v, f, c)
 #define VOP_CLOSE(v, f, c) (v)->ops->close(v, f, c)