From f55b51a06df1cbb7f1336d83629eeb9e51ef0a1c Mon Sep 17 00:00:00 2001 From: Matheus Date: Tue, 27 Aug 2024 21:02:12 -0300 Subject: [PATCH] vfs: finish new locking scheme --- kernel-src/fs/ext2.c | 43 +++++---- kernel-src/fs/tmpfs.c | 23 +++-- kernel-src/fs/vfs.c | 147 ++++++++++++++++++++++------- kernel-src/include/kernel/vfs.h | 6 +- kernel-src/mm/vmmcache.c | 16 +++- kernel-src/sys/syscalls/open.c | 5 +- kernel-src/sys/syscalls/renameat.c | 39 +------- kernel-src/sys/syscalls/unlinkat.c | 17 +--- 8 files changed, 168 insertions(+), 128 deletions(-) diff --git a/kernel-src/fs/ext2.c b/kernel-src/fs/ext2.c index 4c593920..cf7b6517 100644 --- a/kernel-src/fs/ext2.c +++ b/kernel-src/fs/ext2.c @@ -1174,6 +1174,11 @@ static int ext2_lookup(vnode_t *vnode, char *name, vnode_t **result, cred_t *cre MUTEX_RELEASE(&fs->inodetablelock); + // VOP_LOOKUP is required to unlock the parent vnode + // if the name is ".." + if (strcmp(name, "..") == 0) + VOP_UNLOCK(vnode); + // VOP_LOOKUP returns the vnode locked *result = &newnode->vnode; if (newnode != node) @@ -1529,12 +1534,13 @@ static int ext2_munmap(vnode_t *node, void *addr, uintmax_t offset, int flags, c __assert(!"unreachable"); } -static int handleinodeunlink(ext2fs_t *fs, ext2node_t *node, int inode) { +static int handleinodeunlink(ext2fs_t *fs, ext2node_t *node, int inode, ext2node_t *known) { bool isdir = false; void *v; MUTEX_ACQUIRE(&fs->inodetablelock, false); int err = hashtable_get(&fs->inodetable, &v, &inode, sizeof(inode)); if (err == ENOENT) { + __assert(known == NULL); inode_t buff; err = readinode(fs, &buff, inode); ASSERT_UNCLEAN(fs, err == 0); @@ -1560,7 +1566,13 @@ static int handleinodeunlink(ext2fs_t *fs, ext2node_t *node, int inode) { ext2node_t *unlinknode = v; MUTEX_RELEASE(&fs->inodetablelock); - VOP_LOCK(&unlinknode->vnode); + if (known) { + __assert(unlinknode == known); + } + + if (known == NULL) + VOP_LOCK(&unlinknode->vnode); + if (INODE_TYPEPERM_TYPE(unlinknode->inode.typeperm) == INODE_TYPE_DIR) { // account for . entry isdir = 1; unlinknode->inode.links -= 1; @@ -1569,7 +1581,9 @@ static int handleinodeunlink(ext2fs_t *fs, ext2node_t *node, int inode) { int nlinks = --unlinknode->inode.links; err = writeinode(fs, &unlinknode->inode, unlinknode->id); ASSERT_UNCLEAN(fs, err == 0); - VOP_UNLOCK(&unlinknode->vnode); + if (known == NULL) + VOP_UNLOCK(&unlinknode->vnode); + if (nlinks == 0) { vnode_t *unlinkvnode = v; // do an operation to remove from memory and inode table if theres no more refcounts nor links. @@ -1594,7 +1608,7 @@ static int ext2_unlink(vnode_t *vnode, vnode_t *child, char *name, cred_t *cred) if (err) return err; - err = handleinodeunlink(fs, node, inode); + err = handleinodeunlink(fs, node, inode, (ext2node_t *)child); return err; } @@ -1634,8 +1648,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) { +static int ext2_rename(vnode_t *sourcedir, vnode_t *source, char *oldname, vnode_t *targetdir, vnode_t *new, char *newname, int flags) { if (sourcedir->vfs != targetdir->vfs) return EXDEV; @@ -1645,33 +1658,24 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch // the only guarantee mandated by posix is that target will always point to a valid file // and it *may* have both source and target pointing to the same file - // get the source vnode - vnode_t *source = NULL; - int err = VOP_LOOKUP(sourcedir, oldname, &source, NULL); - 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 int oldinode; - err = findindir(fs, ext2targetnode, newname, &oldinode, ext2sourcenode); + int err = findindir(fs, ext2targetnode, newname, &oldinode, ext2sourcenode); if (err == ENOENT) { // the dirent needs to be created err = linkinternal(fs, ext2targetnode, ext2sourcenode, newname); } else if (err == 0){ // the dirent was switched, clean up the inode - err = handleinodeunlink(fs, ext2targetnode, oldinode); + err = handleinodeunlink(fs, ext2targetnode, oldinode, (ext2node_t *)new); ext2sourcenode->inode.links += 1; ASSERT_UNCLEAN(fs, writeinode(fs, &ext2sourcenode->inode, ext2sourcenode->id) == 0); } if (err) { - VOP_RELEASE(source); return err; } @@ -1687,15 +1691,14 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch } cleanup: - VOP_RELEASE(source); return err; } static int ext2_sync(vnode_t *vnode) { - int e = vmmcache_sync(vnode); + int e = vmmcache_syncvnode(vnode, 0, UINT64_MAX); ext2fs_t *fs = (ext2fs_t *)vnode->vfs; // TODO don't sync the entire disk but rather only the inodes and blocks - int e2 = vmmcache_sync(fs->backing); + int e2 = vmmcache_syncvnode(fs->backing, 0, UINT64_MAX); // only the first errors are reported return e ? e : e2; } diff --git a/kernel-src/fs/tmpfs.c b/kernel-src/fs/tmpfs.c index c613e8d1..da0c2347 100644 --- a/kernel-src/fs/tmpfs.c +++ b/kernel-src/fs/tmpfs.c @@ -77,6 +77,12 @@ static int tmpfs_lookup(vnode_t *parent, char *name, vnode_t **result, cred_t *c child = r; VOP_HOLD(child); + + // VOP_LOOKUP is required to unlock the parent vnode + // if the name is ".." + if (strcmp(name, "..") == 0) + VOP_UNLOCK(parent); + // VOP_LOOKUP is expected to return child locked if (child != parent) VOP_LOCK(child); @@ -248,7 +254,6 @@ static int tmpfs_unlink(vnode_t *node, vnode_t *child, char *name, cred_t *cred) int err = hashtable_get(&tmpnode->children, &r, name, namelen); if (err) { - VOP_UNLOCK(node); return err; } @@ -260,9 +265,7 @@ static int tmpfs_unlink(vnode_t *node, vnode_t *child, char *name, cred_t *cred) if (err) return err; - VOP_LOCK(unlinknode); --unlinktmpnode->attr.nlinks; - VOP_UNLOCK(unlinknode); VOP_RELEASE(unlinknode); return 0; @@ -281,10 +284,8 @@ 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_UNLOCK(node); + if (hashtable_get(&tmpdir->children, &v, name, namelen) == 0) return EEXIST; - } int error = hashtable_set(&tmpdir->children, node, name, namelen, true); if (error) @@ -297,7 +298,7 @@ static int tmpfs_link(vnode_t *node, vnode_t *dir, char *name, cred_t *cred) { return 0; } -static int tmpfs_rename(vnode_t *source, char *oldname, vnode_t *target, char *newname, int flags) { +static int tmpfs_rename(vnode_t *source, vnode_t *sourcefile, char *oldname, vnode_t *target, vnode_t *targetfile, char *newname, int flags) { if (source->vfs != target->vfs) return EXDEV; @@ -317,6 +318,8 @@ static int tmpfs_rename(vnode_t *source, char *oldname, vnode_t *target, char *n goto cleanup; vnode_t *node = v; + __assert(sourcefile == node); + vnode_t *oldnode = NULL; // get old node in target @@ -326,6 +329,8 @@ static int tmpfs_rename(vnode_t *source, char *oldname, vnode_t *target, char *n else if (error == 0) // found oldnode = v; + __assert(oldnode == targetfile); + if (node->vfsmounted || (oldnode && oldnode->vfsmounted)) { error = EBUSY; goto cleanup; @@ -383,10 +388,8 @@ static int tmpfs_readlink(vnode_t *node, char **link, cred_t *cred) { tmpfsnode_t *tmpnode = (tmpfsnode_t *)node; char *ret = alloc(strlen(tmpnode->link) + 1); - if (ret == NULL) { - VOP_UNLOCK(node); + if (ret == NULL) return ENOMEM; - } strcpy(ret, tmpnode->link); *link = ret; diff --git a/kernel-src/fs/vfs.c b/kernel-src/fs/vfs.c index 10d6adf0..6703f930 100644 --- a/kernel-src/fs/vfs.c +++ b/kernel-src/fs/vfs.c @@ -469,15 +469,21 @@ int vfs_link(vnode_t *destref, char *destpath, vnode_t *linkref, char *linkpath, int vfs_unlink(vnode_t *ref, char *path) { char *component = alloc(strlen(path) + 1); + if (component == NULL) + return ENOMEM; + vnode_t *parent = NULL; - int err = vfs_lookup(&parent, ref, path, component, VFS_LOOKUP_PARENT); + int err = vfs_lookup(&parent, ref, path, component, VFS_LOOKUP_NOLINK | VFS_LOOKUP_PARENT); if (err) goto cleanup; + bool isdotdot = strcmp(path, "..") == 0; + vnode_t *child = NULL; err = VOP_LOOKUP(parent, component, &child, getcred()); if (err) { VOP_UNLOCK(parent); + VOP_RELEASE(parent); goto cleanup; } @@ -485,16 +491,22 @@ int vfs_unlink(vnode_t *ref, char *path) { if (0) { // locked by VOP_LOOKUP VOP_UNLOCK(child); - // locked by vfs_lookup - VOP_UNLOCK(parent); + // locked by vfs_lookup, unlocked by VOP_LOOKUP if component is ".." + if (!isdotdot) + VOP_UNLOCK(parent); + VOP_RELEASE(child); + VOP_RELEASE(parent); return err; } err = VOP_UNLINK(parent, child, component, getcred()); // locked by VOP_LOOKUP VOP_UNLOCK(child); - // locked by vfs_lookup - VOP_UNLOCK(parent); + // locked by vfs_lookup, unlocked by VOP_LOOKUP if component is ".." + if (!isdotdot) + VOP_UNLOCK(parent); + + VOP_RELEASE(child); VOP_RELEASE(parent); cleanup: @@ -502,6 +514,95 @@ int vfs_unlink(vnode_t *ref, char *path) { return err; } +int vfs_rename(vnode_t *srcref, char *srcpath, vnode_t *dstref, char *dstpath, int flags) { + void *srccomp = alloc(strlen(srcpath) + 1); + if (srccomp == NULL) + return ENOMEM; + + void *dstcomp = alloc(strlen(dstpath) + 1); + if (dstcomp == NULL) { + free(srccomp); + return ENOMEM; + } + + vnode_t *srcdir = NULL; + vnode_t *dstdir = NULL; + int err = vfs_lookup(&srcdir, srcref, srcpath, srccomp, VFS_LOOKUP_NOLINK | VFS_LOOKUP_PARENT); + if (err) + goto cleanup_comp; + + VOP_UNLOCK(srcdir); // locked by vfs_lookup, unlocked here to allow the other vfs_lookup to not deadlock + + err = vfs_lookup(&dstdir, dstref, dstpath, dstcomp, VFS_LOOKUP_NOLINK | VFS_LOOKUP_PARENT); + if (err) { + VOP_RELEASE(srcdir); + goto cleanup_comp; + } + + if (srcdir != dstdir) + VOP_LOCK(srcdir); // lock it again for the other ops if they arent the same + + bool srcdotdot = strcmp(srccomp, "..") == 0; + bool dstdotdot = strcmp(dstcomp, "..") == 0; + vnode_t *src = NULL; + vnode_t *dst = NULL; + + err = VOP_LOOKUP(srcdir, srccomp, &src, getcred()); + if (err) + goto cleanup_locks; + + // srcdir is unlocked by VOP_LOOKUP if srccomp is ".." + if (srcdotdot) + VOP_LOCK(srcdir); + + VOP_UNLOCK(src); // locked by VOP_LOOKUP, unlocked here to allow the other VOP_LOOKUP to not deadlock + + err = VOP_LOOKUP(dstdir, dstcomp, &dst, getcred()); + if (err != 0 && err != ENOENT) { + VOP_RELEASE(src); + goto cleanup_locks; + } + + // dstdir is unlocked by VOP_LOOKUP if dstcomp is ".." + if (dstdotdot) + VOP_LOCK(dstdir); + + if (src != dst) + VOP_LOCK(src); // lock it again for the rename if they arent the same + + // TODO auth check + if (0) + goto cleanup_child; + + err = VOP_RENAME(srcdir, src, srccomp, dstdir, dst, dstcomp, flags); + + cleanup_child: + + if (src != dst) + VOP_UNLOCK(src); + + if (dst) + VOP_UNLOCK(dst); + + VOP_RELEASE(src); + + if (dst) + VOP_RELEASE(dst); + + cleanup_locks: + if (srcdir != dstdir) + VOP_UNLOCK(srcdir); + + VOP_UNLOCK(dstdir); // locked by vfs_lookup + VOP_RELEASE(srcdir); + VOP_RELEASE(dstdir); + + cleanup_comp: + free(srccomp); + free(dstcomp); + return err; +} + // returns the highest node in a mount point static int highestnodeinmp(vnode_t *node, vnode_t **ret) { int e = 0; @@ -523,7 +624,6 @@ 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 @@ -545,12 +645,6 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int if (e) return e; - if ((flags & VFS_LOOKUP_INPUTLOCKED) == 0) { - arch_e9_puts("lock1\n"); - VOP_LOCK(r); - arch_e9_puts("lock1ok\n"); - } - VOP_HOLD(r); *result = r; return 0; @@ -578,11 +672,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int vnode_t *next; error = 0; VOP_HOLD(current); - - arch_e9_puts("lock2\n"); - if ((flags & VFS_LOOKUP_INPUTLOCKED) == 0 || current != start) - VOP_LOCK(current); - arch_e9_puts("lock2ok\n"); + VOP_LOCK(current); for (int i = 0; i < pathlen; ++i) { if (compbuffer[i] == '\0') @@ -604,17 +694,14 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int islast = j == pathlen; } - arch_e9_puts("component: "); - arch_e9_puts(component); - if (islast && (flags & VFS_LOOKUP_PARENT)) { __assert(lastcomp); strcpy(lastcomp, component); break; } - - if (strcmp(component, "..") == 0) { + bool isdotdot = strcmp(component, "..") == 0; + if (isdotdot) { // if the tree root, skip to next component vnode_t *root = NULL; __assert(highestnodeinmp(vfsroot, &root) == 0); @@ -626,7 +713,6 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int // if the root of a mounted fs, go to if (current->flags & V_FLAGS_ROOT) { vnode_t *n = lowestnodeinmp(current); - arch_e9_puts("lock3\n"); if (n != current) { VOP_HOLD(n); VOP_UNLOCK(current); @@ -634,7 +720,6 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int current = n; VOP_LOCK(current); } - arch_e9_puts("lock3ok\n"); } } @@ -643,13 +728,12 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int if (error) break; - arch_e9_puts("lookup\n"); error = VOP_LOOKUP(current, component, &next, getcred()); - arch_e9_puts("lookupok\n"); if (error) break; - if ((current != start || (flags & VFS_LOOKUP_INPUTLOCKED) == 0) && current != next) + // if the current node is not the next one and VOP_LOOKUP didn't already by looking up "..", unlock it + if (current != next && isdotdot == false) VOP_UNLOCK(current); vnode_t *r = next; @@ -663,13 +747,11 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int } if (r != next) { - arch_e9_puts("lock4\n"); VOP_HOLD(r); VOP_UNLOCK(next); VOP_RELEASE(next); next = r; VOP_LOCK(next); - arch_e9_puts("lock4ok\n"); } // dereference symlink @@ -703,9 +785,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int } // TODO possible deadlock when input vnode is locked and the vfs_lookup would lock it - arch_e9_puts("it1\n"); error = vfs_lookup(&derefresult, derefstart, linkderef, NULL, pass); - arch_e9_puts("it1ok\n"); if (*linkderef == '/') VOP_RELEASE(derefstart); @@ -726,10 +806,7 @@ 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_UNLOCK(current); VOP_RELEASE(current); } else { // result is already locked here diff --git a/kernel-src/include/kernel/vfs.h b/kernel-src/include/kernel/vfs.h index 017394d5..94316e97 100644 --- a/kernel-src/include/kernel/vfs.h +++ b/kernel-src/include/kernel/vfs.h @@ -138,7 +138,7 @@ typedef struct vops_t { int (*ioctl)(vnode_t *node, unsigned long request, void *arg, int *result); int (*maxseek)(vnode_t *node, size_t *max); int (*resize)(vnode_t *node, size_t newsize, cred_t *cred); - int (*rename)(vnode_t *source, char *oldname, vnode_t *target, char *newname, int flags); + int (*rename)(vnode_t *sourcedir, vnode_t *source, char *oldname, vnode_t *targetdir, vnode_t *target, char *newname, int flags); 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); @@ -192,7 +192,7 @@ typedef struct vops_t { #define VOP_IOCTL(v, r, a, rp) ((v)->ops->ioctl ? (v)->ops->ioctl(v, r, a, rp) : ENOTTY) #define VOP_MAXSEEK(v, rp) ((v)->ops->maxseek ? (v)->ops->maxseek(v, rp) : ENOTTY) #define VOP_RESIZE(v, s, c) (v)->ops->resize(v, s, c) -#define VOP_RENAME(s, o, t, n, f) (s)->ops->rename(s, o, t, n, f) +#define VOP_RENAME(sd, s, o, td, t, n, f) (s)->ops->rename(sd, s, o, td, t, n, f) #define VOP_GETPAGE(v, o, p) (v)->ops->getpage(v, o, p) #define VOP_PUTPAGE(v, o, p) (v)->ops->putpage(v, o, p) #define VOP_SYNC(v) (v)->ops->sync(v) @@ -216,11 +216,11 @@ int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t int vfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t *bytesread, int flags); int vfs_create(vnode_t *ref, char *path, vattr_t *attr, int type, vnode_t **node); int vfs_link(vnode_t *destref, char *destpath, vnode_t *linkref, char *linkpath, int type, vattr_t *attr); +int vfs_rename(vnode_t *srcref, char *srcpath, vnode_t *dstref, char *dstpath, int flags); int vfs_unlink(vnode_t *ref, char *path); int vfs_pollstub(vnode_t *node, struct polldata *, int events); void vfs_inactive(vnode_t *node); -#define VFS_LOOKUP_INPUTLOCKED 0x10000000 #define VFS_LOOKUP_PARENT 0x20000000 #define VFS_LOOKUP_NOLINK 0x40000000 #define VFS_LOOKUP_INTERNAL 0x80000000 diff --git a/kernel-src/mm/vmmcache.c b/kernel-src/mm/vmmcache.c index 5afb76be..c32978cf 100644 --- a/kernel-src/mm/vmmcache.c +++ b/kernel-src/mm/vmmcache.c @@ -294,13 +294,19 @@ int vmmcache_truncate(vnode_t *vnode, uintmax_t offset) { // called with lock held // returns with lock released // expects backing lock to be held -static int syncpage(page_t *page) { +static int syncpage(page_t *page, bool backinglock) { __assert(page->flags & PAGE_FLAGS_DIRTY); page->flags &= ~PAGE_FLAGS_DIRTY; RELEASE_LOCK(); int e = 0; if ((page->flags & PAGE_FLAGS_TRUNCATED) == 0) { + if (backinglock) + VOP_LOCK(page->backing); + e = VOP_PUTPAGE(page->backing, page->offset, (page_t *)page); + + if (backinglock) + VOP_UNLOCK(page->backing); VOP_RELEASE(page->backing); } else { // page got truncated from the file while waiting to be written to disk @@ -355,7 +361,9 @@ int vmmcache_syncvnode(vnode_t *vnode, uintmax_t offset, size_t size) { page_t *page = vnodedirtylist; vnodedirtylist = vnodedirtylist->writenext; page->writenext = NULL; - int error = syncpage(page); + + int error = syncpage(page, false); + if (e == 0) e = error; // syncpage returns with lock released @@ -441,9 +449,7 @@ static void writer() { page->writenext = NULL; // TODO notify error on vmmcache_syncvnode - VOP_LOCK(page->backing); - syncpage((page_t *)page); - VOP_UNLOCK(page->backing); + syncpage((page_t *)page, true); } } diff --git a/kernel-src/sys/syscalls/open.c b/kernel-src/sys/syscalls/open.c index 11949deb..a8c61028 100644 --- a/kernel-src/sys/syscalls/open.c +++ b/kernel-src/sys/syscalls/open.c @@ -99,7 +99,10 @@ syscallret_t syscall_openat(context_t *context, int dirfd, char *path, int flags ret.ret = newfd; cleanup: - VOP_UNLOCK(vnode); + + if (vnode) + VOP_UNLOCK(vnode); + cleanup_nounlock: if (vnode && ret.errno) { vfs_close(vnode, fileflagstovnodeflags(flags)); diff --git a/kernel-src/sys/syscalls/renameat.c b/kernel-src/sys/syscalls/renameat.c index 8339d6c2..1d613c5e 100644 --- a/kernel-src/sys/syscalls/renameat.c +++ b/kernel-src/sys/syscalls/renameat.c @@ -18,8 +18,6 @@ syscallret_t syscall_renameat(context_t *context, int olddirfd, char *uoldpath, return ret; } - char *oldcomponent = NULL; - char *newcomponent = NULL; char *oldpath = alloc(oldpathlen + 1); if (oldpath == NULL) { ret.errno = ENOMEM; @@ -42,54 +40,19 @@ syscallret_t syscall_renameat(context_t *context, int olddirfd, char *uoldpath, file_t *oldfile = NULL; vnode_t *newdirnode = NULL; file_t *newfile = NULL; - vnode_t *targetdirnode = NULL; - vnode_t *sourcedirnode = NULL; ret.errno = dirfd_enter(oldpath, olddirfd, &oldfile, &olddirnode); - if (ret.errno) goto cleanup; ret.errno = dirfd_enter(newpath, newdirfd, &newfile, &newdirnode); - if (ret.errno) goto cleanup; - oldcomponent = alloc(strlen(oldpath) + 1); - newcomponent = alloc(strlen(newpath) + 1); - - if (oldcomponent == NULL && newcomponent == NULL) - goto cleanup; - - // TODO deadlocks. - ret.errno = vfs_lookup(&sourcedirnode, olddirnode, oldpath, oldcomponent, VFS_LOOKUP_NOLINK | VFS_LOOKUP_PARENT); - if (ret.errno) - goto cleanup; - - ret.errno = vfs_lookup(&targetdirnode, newdirnode, newpath, newcomponent, VFS_LOOKUP_NOLINK | VFS_LOOKUP_PARENT); - if (ret.errno) { - VOP_UNLOCK(sourcedirnode); - goto cleanup; - } - - ret.errno = VOP_RENAME(sourcedirnode, oldcomponent, targetdirnode, newcomponent, flags); + ret.errno = vfs_rename(olddirnode, oldpath, newdirnode, newpath, flags); ret.ret = ret.errno ? -1 : 0; - VOP_UNLOCK(targetdirnode); - VOP_UNLOCK(sourcedirnode); cleanup: - if (targetdirnode) - VOP_RELEASE(targetdirnode); - - if (sourcedirnode) - VOP_RELEASE(sourcedirnode); - - if (oldcomponent) - free(oldcomponent); - - if (newcomponent) - free(newcomponent); - if (olddirnode) dirfd_leave(olddirnode, oldfile); diff --git a/kernel-src/sys/syscalls/unlinkat.c b/kernel-src/sys/syscalls/unlinkat.c index b39ddfdd..f18124dd 100644 --- a/kernel-src/sys/syscalls/unlinkat.c +++ b/kernel-src/sys/syscalls/unlinkat.c @@ -40,25 +40,10 @@ syscallret_t syscall_unlinkat(context_t *, int dirfd, char *upath, int flags) { if (ret.errno) goto cleanup; - // TODO rmdir flag - - vnode_t *node = NULL; - ret.errno = vfs_lookup(&node, dirnode, path, component, VFS_LOOKUP_NOLINK | VFS_LOOKUP_PARENT); - if (ret.errno) - goto cleanup; - - ret.errno = VOP_UNLINK(node, component, &_cpu()->thread->proc->cred); + ret.errno = vfs_unlink(dirnode, path); ret.ret = ret.errno ? -1 : 0; - // locked by vfs_lookup - VOP_UNLOCK(node); cleanup: - if (component) - free(component); - - if (node) - VOP_RELEASE(node); - if (dirnode) dirfd_leave(dirnode, file);