From da150523bad49aeb8c5f20804af4a1938ec8f7f3 Mon Sep 17 00:00:00 2001 From: Matheus Date: Mon, 26 Aug 2024 18:45:17 -0300 Subject: [PATCH] vfs: leave handling of VOP_LOCK and VOP_UNLOCK to each filesystem --- kernel-src/fs/devfs.c | 64 +++++++++++++++++++++------------ kernel-src/fs/ext2.c | 15 +++++++- kernel-src/fs/pipefs.c | 61 +++++++++++++++++++------------ kernel-src/fs/sockfs.c | 28 +++++++++++---- kernel-src/fs/tmpfs.c | 14 +++++++- kernel-src/include/kernel/vfs.h | 6 ++-- 6 files changed, 133 insertions(+), 55 deletions(-) diff --git a/kernel-src/fs/devfs.c b/kernel-src/fs/devfs.c index 860336a8..e7364650 100644 --- a/kernel-src/fs/devfs.c +++ b/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); diff --git a/kernel-src/fs/ext2.c b/kernel-src/fs/ext2.c index e3b1d681..c4a7e97c 100644 --- a/kernel-src/fs/ext2.c +++ b/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() { diff --git a/kernel-src/fs/pipefs.c b/kernel-src/fs/pipefs.c index 7014e8de..e1974833 100644 --- a/kernel-src/fs/pipefs.c +++ b/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) { diff --git a/kernel-src/fs/sockfs.c b/kernel-src/fs/sockfs.c index d5b21c9f..3b3cb082 100644 --- a/kernel-src/fs/sockfs.c +++ b/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) { diff --git a/kernel-src/fs/tmpfs.c b/kernel-src/fs/tmpfs.c index 85fd0dc8..b26c3382 100644 --- a/kernel-src/fs/tmpfs.c +++ b/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) { diff --git a/kernel-src/include/kernel/vfs.h b/kernel-src/include/kernel/vfs.h index 8f11eb96..b93aad9c 100644 --- a/kernel-src/include/kernel/vfs.h +++ b/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_UNLOCK(v) MUTEX_RELEASE(&(v)->lock) +#define VOP_LOCK(v) (v)->ops->lock(v) +#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)