From fef149ca556144c1c21f1ae68e4394dfcd0b902d Mon Sep 17 00:00:00 2001 From: Matheus Date: Thu, 15 Jun 2023 19:57:56 -0300 Subject: [PATCH] vfs: redo vnode locking --- kernel-src/fs/devfs.c | 24 +++++++++++--- kernel-src/fs/tmpfs.c | 60 ++++++++++++++++++++++++++++------ kernel-src/fs/vfs.c | 20 ------------ kernel-src/mm/vmm.c | 4 --- kernel-src/sys/syscalls/open.c | 2 -- kernel-src/sys/syscalls/seek.c | 2 -- kernel-src/sys/syscalls/stat.c | 2 -- 7 files changed, 69 insertions(+), 45 deletions(-) diff --git a/kernel-src/fs/devfs.c b/kernel-src/fs/devfs.c index 112187e5..00fb6423 100644 --- a/kernel-src/fs/devfs.c +++ b/kernel-src/fs/devfs.c @@ -85,12 +85,16 @@ int devfs_lookup(vnode_t *node, char *name, vnode_t **result, cred_t *cred) { return EINVAL; void *v; + VOP_LOCK(node); int error = hashtable_get(&nametable, &v, name, strlen(name)); - if (error) + if (error) { + VOP_UNLOCK(node); return error; + } vnode_t *rnode = v; VOP_HOLD(rnode); + VOP_UNLOCK(node); *result = rnode; return 0; @@ -106,8 +110,10 @@ int devfs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) { return err; } + VOP_LOCK(node); *attr = devnode->attr; attr->type = node->type; + VOP_UNLOCK(node); return 0; } @@ -116,18 +122,18 @@ int devfs_setattr(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); int err = VOP_GETATTR(devnode->physical, attr, cred); - VOP_UNLOCK(devnode->physical); return err; } + VOP_LOCK(node); devnode->attr.gid = attr->gid; devnode->attr.uid = attr->uid; devnode->attr.mode = attr->mode; devnode->attr.atime = attr->atime; devnode->attr.mtime = attr->mtime; devnode->attr.ctime = attr->ctime; + VOP_UNLOCK(node); return 0; } @@ -170,6 +176,7 @@ int devfs_access(vnode_t *node, mode_t mode, cred_t *cred) { } int devfs_inactive(vnode_t *node) { + VOP_LOCK(node); devnode_t *devnode = (devnode_t *)node; vnode_t *master = (vnode_t *)devnode->master; if (devnode->physical && devnode->physical != node) @@ -186,12 +193,17 @@ int devfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vnode_t * size_t namelen = strlen(name); void *v; - if (hashtable_get(&nametable, &v, name, namelen) == 0) + VOP_LOCK(parent); + if (hashtable_get(&nametable, &v, name, namelen) == 0) { + VOP_UNLOCK(parent); return EEXIST; + } devnode_t *node = slab_allocate(nodecache); - if (node == NULL) + if (node == NULL) { + VOP_UNLOCK(parent); return ENOMEM; + } timespec_t time = timekeeper_time(); vattr_t tmpattr = *attr; @@ -208,11 +220,13 @@ int devfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vnode_t * int error = hashtable_set(&nametable, node, name, namelen, true); if (error) { + VOP_UNLOCK(parent); slab_free(nodecache, node); return error; } VOP_HOLD(&node->vnode); + VOP_UNLOCK(parent); *result = &node->vnode; return 0; diff --git a/kernel-src/fs/tmpfs.c b/kernel-src/fs/tmpfs.c index ce6b9280..da135fef 100644 --- a/kernel-src/fs/tmpfs.c +++ b/kernel-src/fs/tmpfs.c @@ -42,6 +42,7 @@ static int tmpfs_poll(vnode_t *node, int event) { #define DATAMMUFLAGS (ARCH_MMU_FLAGS_WRITE | ARCH_MMU_FLAGS_READ | ARCH_MMU_FLAGS_NOEXEC) +// expects to be called with node locked static int tmpfs_resize(tmpfsnode_t *node, size_t size) { if (size == node->attr.size || node->vnode.type != V_TYPE_REGULAR) return 0; @@ -91,22 +92,28 @@ static int tmpfs_resize(tmpfsnode_t *node, size_t size) { } 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; + VOP_UNLOCK(node); return 0; } static int tmpfs_setattr(vnode_t *node, vattr_t *attr, cred_t *cred) { + VOP_LOCK(node); tmpfsnode_t *tmpnode = (tmpfsnode_t *)node; int err = tmpfs_resize(tmpnode, attr->size); - if (err) + if (err) { + VOP_UNLOCK(node); return err; + } tmpnode->attr.gid = attr->gid; tmpnode->attr.uid = attr->uid; tmpnode->attr.mode = attr->mode; tmpnode->attr.atime = attr->atime; tmpnode->attr.mtime = attr->mtime; tmpnode->attr.ctime = attr->ctime; + VOP_UNLOCK(node); return 0; } @@ -116,13 +123,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) + if (err) { + VOP_UNLOCK(parent); return err; + } child = r; VOP_HOLD(child); + VOP_UNLOCK(parent); *result = child; return 0; } @@ -135,13 +146,18 @@ static int tmpfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vn size_t namelen = strlen(name); void *v; - if (hashtable_get(&parenttmpnode->children, &v, name, namelen) == 0) + VOP_LOCK(parent); + if (hashtable_get(&parenttmpnode->children, &v, name, namelen) == 0) { + VOP_UNLOCK(parent); return EEXIST; + } tmpfsnode_t *tmpnode = newnode(parent->vfs, type); vnode_t *node = (vnode_t *)tmpnode; - if (tmpnode == NULL) + if (tmpnode == NULL) { + VOP_UNLOCK(parent); return ENOMEM; + } timespec_t time = timekeeper_time(); @@ -154,6 +170,7 @@ 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; } @@ -162,12 +179,14 @@ static int tmpfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vn 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); @@ -195,10 +214,8 @@ static int tmpfs_open(vnode_t **nodep, int flag, cred_t *cred) { if (err) return err; - VOP_UNLOCK(node); VOP_RELEASE(node); VOP_HOLD(devnode); - VOP_LOCK(devnode); *nodep = devnode; } @@ -210,6 +227,7 @@ static int tmpfs_close(vnode_t *node, int flag, cred_t *cred) { } static int tmpfs_root(vfs_t *vfs, vnode_t **vnode) { + // FIXME lock vfs if (vfs->root) { *vnode = vfs->root; return 0; @@ -232,9 +250,11 @@ static int tmpfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset if (node->type != V_TYPE_REGULAR) return EINVAL; + VOP_LOCK(node); // offset past end of file if (offset >= tmpnode->attr.size) { *readc = 0; + VOP_UNLOCK(node); return 0; } @@ -245,6 +265,7 @@ static int tmpfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset *readc = size; memcpy(buffer, (void *)((uintptr_t)tmpnode->data + offset), size); + VOP_UNLOCK(node); return 0; } @@ -255,15 +276,19 @@ static int tmpfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offse size_t top = offset + size; + VOP_LOCK(node); if (top > tmpnode->attr.size) { int err = tmpfs_resize(tmpnode, top); - if (err) + if (err) { + VOP_UNLOCK(node); return err; + } } *writec = size; memcpy((void *)((uintptr_t)tmpnode->data + offset), buffer, size); + VOP_UNLOCK(node); return 0; } @@ -279,14 +304,19 @@ 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) + if (err) { + VOP_UNLOCK(node); return err; + } vnode_t *unlinknode = r; tmpfsnode_t *unlinktmpnode = r; err = hashtable_remove(&tmpnode->children, name, namelen); + VOP_UNLOCK(node); if (err) return err; @@ -305,10 +335,14 @@ 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; - if (hashtable_get(&tmpdir->children, &v, name, namelen) == 0) + VOP_LOCK(node); + 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; @@ -335,8 +369,10 @@ 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; + VOP_UNLOCK(linknode); VOP_RELEASE(linknode); return 0; @@ -348,13 +384,17 @@ 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) + if (ret == NULL) { + VOP_UNLOCK(node); return ENOMEM; + } strcpy(ret, tmpnode->link); *link = ret; + VOP_UNLOCK(node); return 0; } diff --git a/kernel-src/fs/vfs.c b/kernel-src/fs/vfs.c index a113a718..573da63d 100644 --- a/kernel-src/fs/vfs.c +++ b/kernel-src/fs/vfs.c @@ -84,9 +84,7 @@ int vfs_open(vnode_t *ref, char *path, int flags, vnode_t **res) { if (err) return err; - VOP_LOCK(*res); err = VOP_OPEN(res, flags, getcred()); - VOP_UNLOCK(*res); if (err) VOP_RELEASE(*res); @@ -94,9 +92,7 @@ int vfs_open(vnode_t *ref, char *path, int flags, vnode_t **res) { } int vfs_close(vnode_t *node, int flags) { - VOP_LOCK(node); int err = VOP_CLOSE(node, flags, getcred()); - VOP_UNLOCK(node); return err; } @@ -109,9 +105,7 @@ int vfs_create(vnode_t *ref, char *path, vattr_t *attr, int type, vnode_t **node goto cleanup; vnode_t *ret; - VOP_LOCK(parent); err = VOP_CREATE(parent, component, attr, type, &ret, getcred()); - VOP_UNLOCK(parent); VOP_RELEASE(parent); if (err) goto cleanup; @@ -127,16 +121,12 @@ int vfs_create(vnode_t *ref, char *path, vattr_t *attr, int type, vnode_t **node } int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t *written, int flags) { - VOP_LOCK(node); int err = VOP_WRITE(node, buffer, size, offset, flags, written, getcred()); - VOP_UNLOCK(node); return err; } int vfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t *bytesread, int flags) { - VOP_LOCK(node); int err = VOP_READ(node, buffer, size, offset, flags, bytesread, getcred()); - VOP_UNLOCK(node); return err; } @@ -160,14 +150,10 @@ int vfs_link(vnode_t *destref, char *destpath, vnode_t *linkref, char *linkpath, VOP_RELEASE(parent); goto cleanup; } - VOP_LOCK(parent); err = VOP_LINK(targetnode, parent, component, getcred()); - VOP_UNLOCK(parent); VOP_RELEASE(targetnode); } else { - VOP_LOCK(parent); err = VOP_SYMLINK(parent, component, attr, linkpath, getcred()); - VOP_UNLOCK(parent); } VOP_RELEASE(parent); @@ -184,9 +170,7 @@ int vfs_unlink(vnode_t *ref, char *path) { if (err) goto cleanup; - VOP_LOCK(parent); VOP_UNLINK(parent, component, getcred()); - VOP_UNLOCK(parent); VOP_RELEASE(parent); cleanup: @@ -303,9 +287,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int } } - VOP_LOCK(current); error = VOP_LOOKUP(current, component, &next, getcred()); - VOP_UNLOCK(current); if (error) break; @@ -326,9 +308,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int if (next->type == V_TYPE_LINK && (islast == false || (islast == true && (flags & VFS_LOOKUP_NOLINK) == 0))) { // get path char *linkderef; - VOP_LOCK(next); error = VOP_READLINK(next, &linkderef, getcred()); - VOP_UNLOCK(next); if (error) { VOP_RELEASE(next); break; diff --git a/kernel-src/mm/vmm.c b/kernel-src/mm/vmm.c index 78903faf..7f087637 100644 --- a/kernel-src/mm/vmm.c +++ b/kernel-src/mm/vmm.c @@ -207,9 +207,7 @@ static void destroyrange(vmmcontext_t *context, vmmrange_t *range, uintmax_t _of continue; if ((range->flags & VMM_FLAGS_FILE) && (range->flags & VMM_FLAGS_SHARED) && (flags & VMM_DESTROY_FLAGS_NOSYNC) == 0) { - VOP_LOCK(range->vnode); __assert(VOP_MUNMAP(range->vnode, vaddr, range->offset + offset, mmuflagstovnodeflags(range->mmuflags), NULL) == 0); // XXX pass cred struct - VOP_UNLOCK(range->vnode); } else { pmm_free(physical, 1); arch_mmu_unmap(context->pagetable, vaddr); @@ -316,9 +314,7 @@ bool vmm_pagefault(void *addr, bool user, int actions) { if (range->flags & VMM_FLAGS_FILE) { uintmax_t mapoffset = (uintptr_t)addr - (uintptr_t)range->start; - VOP_LOCK(range->vnode); __assert(VOP_MMAP(range->vnode, addr, range->offset + mapoffset, mmuflagstovnodeflags(range->mmuflags), NULL) == 0); // XXX pass cred struct - VOP_UNLOCK(range->vnode); status = true; } else { void *paddr = pmm_alloc(1, PMM_SECTION_DEFAULT); diff --git a/kernel-src/sys/syscalls/open.c b/kernel-src/sys/syscalls/open.c index 53a3ef62..796dfbf3 100644 --- a/kernel-src/sys/syscalls/open.c +++ b/kernel-src/sys/syscalls/open.c @@ -54,9 +54,7 @@ syscallret_t syscall_openat(context_t *context, int dirfd, const char *path, int goto cleanup; vattr_t attr; - VOP_LOCK(vnode); ret.errno = VOP_GETATTR(vnode, &attr, &_cpu()->thread->proc->cred); - VOP_UNLOCK(vnode); if (ret.errno) goto cleanup; diff --git a/kernel-src/sys/syscalls/seek.c b/kernel-src/sys/syscalls/seek.c index ec5d2cd5..2549f9ab 100644 --- a/kernel-src/sys/syscalls/seek.c +++ b/kernel-src/sys/syscalls/seek.c @@ -49,9 +49,7 @@ syscallret_t syscall_seek(context_t *context, int fd, off_t offset, int whence) } case SEEK_END: { vattr_t attr; - VOP_LOCK(file->vnode); ret.errno = VOP_GETATTR(file->vnode, &attr, &_cpu()->thread->proc->cred); - VOP_UNLOCK(file->vnode); if (ret.errno) goto cleanup; file->offset = attr.size; diff --git a/kernel-src/sys/syscalls/stat.c b/kernel-src/sys/syscalls/stat.c index 31afce03..1a63ef58 100644 --- a/kernel-src/sys/syscalls/stat.c +++ b/kernel-src/sys/syscalls/stat.c @@ -8,9 +8,7 @@ static int dostat(vnode_t *vnode, stat_t *stat) { vattr_t attr; - VOP_LOCK(vnode); int e = VOP_GETATTR(vnode, &attr, &_cpu()->thread->proc->cred); - VOP_UNLOCK(vnode); if (e) return e;