From 032560bda0296ca6901503e6cc72a50d41903233 Mon Sep 17 00:00:00 2001 From: Matheus Date: Mon, 3 Jul 2023 22:51:53 -0300 Subject: [PATCH] file: redo locking --- kernel-src/fs/file.c | 6 --- kernel-src/include/kernel/file.h | 3 ++ kernel-src/sys/syscalls/execve.c | 2 + kernel-src/sys/syscalls/fcntl.c | 5 ++- kernel-src/sys/syscalls/getdent.c | 5 ++- kernel-src/sys/syscalls/open.c | 12 ++--- kernel-src/sys/syscalls/pipe2.c | 73 +++++++++++++++++++++++++++++++ kernel-src/sys/syscalls/read.c | 5 ++- kernel-src/sys/syscalls/seek.c | 16 ++++--- kernel-src/sys/syscalls/waitpid.c | 2 + kernel-src/sys/syscalls/write.c | 5 ++- 11 files changed, 107 insertions(+), 27 deletions(-) create mode 100644 kernel-src/sys/syscalls/pipe2.c diff --git a/kernel-src/fs/file.c b/kernel-src/fs/file.c index f6035441..83c11510 100644 --- a/kernel-src/fs/file.c +++ b/kernel-src/fs/file.c @@ -81,14 +81,10 @@ file_t *fd_get(int fd) { spinlock_release(&proc->fdlock); - if (file) - MUTEX_ACQUIRE(&file->mutex, false); - return file; } void fd_release(file_t *file) { - MUTEX_RELEASE(&file->mutex); FILE_RELEASE(file); } @@ -117,8 +113,6 @@ int fd_new(int flags, file_t **rfile, int *rfd) { proc->fdfirst = fd + 1; proc->fd[fd].file = file; proc->fd[fd].flags = flags; - MUTEX_ACQUIRE(&file->mutex, false); - FILE_HOLD(file); spinlock_release(&proc->fdlock); diff --git a/kernel-src/include/kernel/file.h b/kernel-src/include/kernel/file.h index 58480976..d34f4547 100644 --- a/kernel-src/include/kernel/file.h +++ b/kernel-src/include/kernel/file.h @@ -27,6 +27,9 @@ int fd_close(int fd); int fd_dup(int oldfd, int newfd, bool exact, int flags, int *ret); int fd_clone(proc_t *proc); +#define FILE_LOCK(x) MUTEX_ACQUIRE(&(x)->mutex, false) +#define FILE_UNLOCK(x) MUTEX_RELEASE(&(x)->mutex) + #define FILE_READ 1 #define FILE_WRITE 2 #define FILE_KNOWN_FLAGS 3 diff --git a/kernel-src/sys/syscalls/execve.c b/kernel-src/sys/syscalls/execve.c index b8222088..da7df458 100644 --- a/kernel-src/sys/syscalls/execve.c +++ b/kernel-src/sys/syscalls/execve.c @@ -112,6 +112,8 @@ syscallret_t syscall_execve(context_t *context, char *upath, char *uargv[], char goto error; } + ret.ret = 0; + // TODO kill other threads and close CLOEXEC fds error: diff --git a/kernel-src/sys/syscalls/fcntl.c b/kernel-src/sys/syscalls/fcntl.c index 373d30b7..c2c2e36e 100644 --- a/kernel-src/sys/syscalls/fcntl.c +++ b/kernel-src/sys/syscalls/fcntl.c @@ -33,10 +33,12 @@ syscallret_t syscall_fcntl(context_t *, int fd, int cmd, uint64_t arg) { ret.ret = file->flags; break; case F_SETFL: + FILE_LOCK(file); arg &= ~(O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC); file->flags &= ~(O_APPEND | O_ASYNC | O_NOATIME | O_NONBLOCK); file->flags |= arg; ret.ret = 0; + FILE_UNLOCK(file); break; case F_SETFD: case F_GETFD: // TODO implement this @@ -47,8 +49,7 @@ syscallret_t syscall_fcntl(context_t *, int fd, int cmd, uint64_t arg) { break; } - if (file) - fd_release(file); + fd_release(file); ret.errno = 0; return ret; diff --git a/kernel-src/sys/syscalls/getdent.c b/kernel-src/sys/syscalls/getdent.c index bf9c7506..eb8cb167 100644 --- a/kernel-src/sys/syscalls/getdent.c +++ b/kernel-src/sys/syscalls/getdent.c @@ -39,12 +39,13 @@ syscallret_t syscall_getdents(context_t *, int dirfd, void *ubuffer, size_t read goto cleanup; } - ret.errno = VOP_GETDENTS(fd->vnode, buffer, count, fd->offset, &ret.ret); + size_t offset = fd->offset; + ret.errno = VOP_GETDENTS(fd->vnode, buffer, count, offset, &ret.ret); if (ret.errno) goto cleanup; - fd->offset += ret.ret; + fd->offset = offset + ret.ret; ret.ret *= sizeof(dent_t); diff --git a/kernel-src/sys/syscalls/open.c b/kernel-src/sys/syscalls/open.c index b1b55d78..f10e8167 100644 --- a/kernel-src/sys/syscalls/open.c +++ b/kernel-src/sys/syscalls/open.c @@ -8,7 +8,7 @@ syscallret_t syscall_openat(context_t *context, int dirfd, const char *path, int flags, mode_t mode) { syscallret_t ret = { .ret = -1, - .errno = 0 + .errno = -1 }; // transform O_RDONLY, O_WRONLY, O_RDWR into FILE_READ and FILE_WRITE @@ -89,13 +89,9 @@ syscallret_t syscall_openat(context_t *context, int dirfd, const char *path, int VOP_RELEASE(vnode); } - if (newfile) { - if (ret.errno) { - __assert(fd_close(newfd) == 0); - } else { - fd_release(newfile); - } - } + if (newfile && ret.errno) + __assert(fd_close(newfd) == 0); + if (pathbuf) free(pathbuf); diff --git a/kernel-src/sys/syscalls/pipe2.c b/kernel-src/sys/syscalls/pipe2.c new file mode 100644 index 00000000..519cc07b --- /dev/null +++ b/kernel-src/sys/syscalls/pipe2.c @@ -0,0 +1,73 @@ +#include +#include +#include +#include +#include +#include +#include + +static void open(file_t *file, vnode_t *node, int flags) { + file->vnode = node; + file->flags = flags; + file->offset = 0; + file->mode = 0777; + VOP_OPEN(&node, fileflagstovnodeflags(flags), &_cpu()->thread->proc->cred); +} + +syscallret_t syscall_pipe2(context_t *, int flags) { + // TODO support them + __assert(flags == 0); + syscallret_t ret = { + .ret = -1 + }; + + vnode_t *node = NULL; + int readfd, writefd; + file_t *readfile = NULL; + file_t *writefile = NULL; + + ret.errno = pipefs_newpipe(&node); + if (ret.errno) + goto error; + + // node held again because two files will point to the same node + VOP_HOLD(node); + + ret.errno = fd_new(0, &readfile, &readfd); + if (ret.errno) + goto error; + + open(readfile, node, FILE_READ); + + ret.errno = fd_new(0, &writefile, &writefd); + if (ret.errno) + goto error; + + open(writefile, node, FILE_WRITE); + + ret.errno = 0; + ret.ret = (uint64_t)readfd | ((uint64_t)writefd << 32); + printf("pipe2: readfd %d writefd %d wrefcount %d\n", readfd, writefd, writefile->refcount); + + return ret; + + error: + + if (readfile) { + fd_release(readfile); + fd_close(readfd); + } + + if (writefile) { + fd_release(writefile); + fd_close(writefd); + } + + if (node) { + // released twice because of two holds (one at pipefs_newpipe and the manual hold) + VOP_RELEASE(node); + VOP_RELEASE(node); + } + + return ret; +} diff --git a/kernel-src/sys/syscalls/read.c b/kernel-src/sys/syscalls/read.c index 36f35b5f..8ad881a1 100644 --- a/kernel-src/sys/syscalls/read.c +++ b/kernel-src/sys/syscalls/read.c @@ -1,7 +1,9 @@ #include #include #include +#include #include +#include syscallret_t syscall_read(context_t *context, int fd, void *buffer, size_t size) { syscallret_t ret = { @@ -23,12 +25,13 @@ syscallret_t syscall_read(context_t *context, int fd, void *buffer, size_t size) // TODO pass flags size_t bytesread; + uintmax_t offset = file->offset; ret.errno = vfs_read(file->vnode, kernelbuff, size, file->offset, &bytesread, 0); if (ret.errno) goto cleanup; - file->offset += bytesread; + file->offset = offset + bytesread; ret.ret = bytesread; ret.errno = 0; diff --git a/kernel-src/sys/syscalls/seek.c b/kernel-src/sys/syscalls/seek.c index 2549f9ab..4a8359c5 100644 --- a/kernel-src/sys/syscalls/seek.c +++ b/kernel-src/sys/syscalls/seek.c @@ -30,21 +30,24 @@ syscallret_t syscall_seek(context_t *context, int fd, off_t offset, int whence) // TODO device check seek + uintmax_t curroffset = file->offset; + uintmax_t newoffset = 0; + switch (whence) { case SEEK_SET: - file->offset = offset; + newoffset = offset; break; case SEEK_CUR: { - uintmax_t newoffset = file->offset + offset; - if (offset > 0 && newoffset < file->offset) { + uintmax_t newoffset = curroffset + offset; + if (offset > 0 && newoffset < curroffset) { ret.errno = EOVERFLOW; goto cleanup; } - if (offset < 0 && newoffset > file->offset) { + if (offset < 0 && newoffset > curroffset) { ret.errno = EINVAL; goto cleanup; } - file->offset = newoffset; + newoffset = newoffset; break; } case SEEK_END: { @@ -52,11 +55,12 @@ syscallret_t syscall_seek(context_t *context, int fd, off_t offset, int whence) ret.errno = VOP_GETATTR(file->vnode, &attr, &_cpu()->thread->proc->cred); if (ret.errno) goto cleanup; - file->offset = attr.size; + newoffset = attr.size; break; } } + file->offset = newoffset; ret.ret = file->offset; ret.errno = 0; diff --git a/kernel-src/sys/syscalls/waitpid.c b/kernel-src/sys/syscalls/waitpid.c index f12938d7..bbdbd906 100644 --- a/kernel-src/sys/syscalls/waitpid.c +++ b/kernel-src/sys/syscalls/waitpid.c @@ -72,6 +72,8 @@ syscallret_t syscall_waitpid(context_t *context, pid_t pid, int *status, int opt ret.ret = iterator->pid; ret.errno = 0; + + __assert(iterator); sched_destroyproc(iterator); return ret; } diff --git a/kernel-src/sys/syscalls/write.c b/kernel-src/sys/syscalls/write.c index b970d243..9802e242 100644 --- a/kernel-src/sys/syscalls/write.c +++ b/kernel-src/sys/syscalls/write.c @@ -26,12 +26,13 @@ syscallret_t syscall_write(context_t *context, int fd, void *buffer, size_t size // TODO pass flags size_t byteswritten; - ret.errno = vfs_write(file->vnode, kernelbuff, size, file->offset, &byteswritten, 0); + uintmax_t offset = file->offset; + ret.errno = vfs_write(file->vnode, kernelbuff, size, offset, &byteswritten, 0); if (ret.errno) goto cleanup; - file->offset += byteswritten; + file->offset = offset + byteswritten; ret.ret = byteswritten; ret.errno = 0; cleanup: