From e6284a8774b387ec23cda03b2af0ba3dcc09286f Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 29 Oct 2018 21:54:11 +0100 Subject: [PATCH] Fix broken SpinLock. The SpinLock was all backwards and didn't actually work. Fixing it exposed how wrong most of the locking here is. I need to come up with a better granularity here. --- AK/Lock.h | 52 ++++++++++++++++--- AK/StringBuilder.cpp | 2 +- AK/Traits.h | 2 +- AK/test.cpp | 9 ++++ ELFLoader/ELFImage.cpp | 2 +- Kernel/Console.cpp | 3 +- Kernel/Syscall.cpp | 10 ++-- Kernel/Task.cpp | 17 ++++--- Kernel/init.cpp | 14 ++++++ Kernel/kassert.h | 1 + Kernel/kmalloc.cpp | 2 +- Kernel/kprintf.cpp | 17 ++++++- Kernel/kprintf.h | 1 + Userland/sh.cpp | 26 +++++++--- VirtualFileSystem/DiskBackedFileSystem.cpp | 10 +++- VirtualFileSystem/DiskDevice.cpp | 1 + VirtualFileSystem/Ext2FileSystem.cpp | 1 - VirtualFileSystem/FileHandle.cpp | 10 ++-- VirtualFileSystem/FileSystem.cpp | 5 +- VirtualFileSystem/Makefile | 1 + VirtualFileSystem/SyntheticFileSystem.cpp | 10 ++-- VirtualFileSystem/VirtualFileSystem.cpp | 58 +++++++++++++--------- VirtualFileSystem/VirtualFileSystem.h | 6 ++- VirtualFileSystem/test.cpp | 12 +++-- 24 files changed, 195 insertions(+), 77 deletions(-) diff --git a/AK/Lock.h b/AK/Lock.h index 3bcdf32bc1b..27c7a73c7f0 100644 --- a/AK/Lock.h +++ b/AK/Lock.h @@ -2,6 +2,20 @@ #include "Types.h" +#ifdef SERENITY +#include "i386.h" +#else +typedef int InterruptDisabler; +#endif + +//#define DEBUG_LOCKS + +void log_try_lock(const char*); +void log_locked(const char*); +void log_unlocked(const char*); + +void yield(); + namespace AK { static inline dword CAS(volatile dword* mem, dword newval, dword oldval) @@ -9,28 +23,46 @@ static inline dword CAS(volatile dword* mem, dword newval, dword oldval) dword ret; asm volatile( "cmpxchgl %2, %1" - :"=a"(ret), "=m"(*mem) - :"r"(newval), "m"(*mem), "0"(oldval)); + :"=a"(ret), "+m"(*mem) + :"r"(newval), "0"(oldval) + :"memory"); return ret; } class SpinLock { public: SpinLock() { } - ~SpinLock() { unlock(); } + ~SpinLock() { } - void lock() + void lock(const char* func = nullptr) { +#ifdef DEBUG_LOCKS + { + InterruptDisabler dis; + log_try_lock(func); + } +#endif for (;;) { - if (CAS(&m_lock, 1, 0) == 1) + if (CAS(&m_lock, 1, 0) == 0) { +#ifdef DEBUG_LOCKS + InterruptDisabler dis; + log_locked(func); +#endif return; + } + yield(); } } - void unlock() + void unlock(const char* func = nullptr) { // barrier(); + ASSERT(m_lock); m_lock = 0; +#ifdef DEBUG_LOCKS + InterruptDisabler dis; + log_unlocked(func); +#endif } void init() @@ -44,14 +76,18 @@ private: class Locker { public: - explicit Locker(SpinLock& l) : m_lock(l) { m_lock.lock(); } + explicit Locker(SpinLock& l, const char* func) : m_lock(l), m_func(func) { lock(); } ~Locker() { unlock(); } - void unlock() { m_lock.unlock(); } + void unlock() { m_lock.unlock(m_func); } + void lock() { m_lock.lock(m_func); } private: SpinLock& m_lock; + const char* m_func { nullptr }; }; +#define LOCKER(lock) Locker locker(lock, __FUNCTION__) + } using AK::SpinLock; diff --git a/AK/StringBuilder.cpp b/AK/StringBuilder.cpp index a9662710dce..6414d840904 100644 --- a/AK/StringBuilder.cpp +++ b/AK/StringBuilder.cpp @@ -18,7 +18,7 @@ String StringBuilder::build() if (strings.isEmpty()) return String::empty(); - size_t sizeNeeded = 1; + size_t sizeNeeded = 0; for (auto& string : strings) sizeNeeded += string.length(); diff --git a/AK/Traits.h b/AK/Traits.h index d9e64b04357..b3c89571a38 100644 --- a/AK/Traits.h +++ b/AK/Traits.h @@ -29,7 +29,7 @@ struct Traits { #ifdef SERENITY return intHash((dword)p); #else - return intHash((ptrdiff_t)p & 0xffffffff); + return intHash((unsigned long long)p & 0xffffffff); #endif } static void dump(const T* p) { kprintf("%p", p); } diff --git a/AK/test.cpp b/AK/test.cpp index fa8d87e0ff2..8ac8811bb4c 100644 --- a/AK/test.cpp +++ b/AK/test.cpp @@ -11,13 +11,22 @@ #include "WeakPtr.h" #include "CircularQueue.h" #include "FileSystemPath.h" +#include "Lock.h" static void testWeakPtr(); +void log_locked() { } +void log_unlocked() { } + int main(int c, char** v) { StringImpl::initializeGlobals(); + { + SpinLock lock; + Locker locker(lock); + } + { const char* testpath = "/proc/../proc/1/../../proc/1/vm"; if (c == 2) diff --git a/ELFLoader/ELFImage.cpp b/ELFLoader/ELFImage.cpp index 7d2d1426a55..f42f1be7717 100644 --- a/ELFLoader/ELFImage.cpp +++ b/ELFLoader/ELFImage.cpp @@ -178,7 +178,7 @@ const ELFImage::RelocationSection ELFImage::Section::relocations() const { // FIXME: This is ugly. char relocationSectionName[128]; - int x = ksprintf(relocationSectionName, ".rel%s", name()); + ksprintf(relocationSectionName, ".rel%s", name()); #ifdef ELFIMAGE_DEBUG kprintf("looking for '%s'\n", relocationSectionName); diff --git a/Kernel/Console.cpp b/Kernel/Console.cpp index 2ce58760e6d..207bd3115dc 100644 --- a/Kernel/Console.cpp +++ b/Kernel/Console.cpp @@ -28,11 +28,10 @@ bool Console::hasDataAvailableForRead() const return false; } -ssize_t Console::read(byte* buffer, size_t bufferSize) +ssize_t Console::read(byte*, size_t) { // FIXME: Implement reading from the console. // Maybe we could use a ring buffer for this device? - // A generalized ring buffer would probably be useful. return 0; } diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp index ba9884e0d5a..5c5fb3f3f00 100644 --- a/Kernel/Syscall.cpp +++ b/Kernel/Syscall.cpp @@ -41,18 +41,15 @@ asm( namespace Syscall { -static SpinLock* s_lock; - void initialize() { - s_lock = new SpinLock; registerUserCallableInterruptHandler(0x80, syscall_ISR); kprintf("syscall: int 0x80 handler installed\n"); } DWORD handle(DWORD function, DWORD arg1, DWORD arg2, DWORD arg3) { - Locker locker(*s_lock); + ASSERT_INTERRUPTS_ENABLED(); switch (function) { case Syscall::Yield: yield(); @@ -73,8 +70,7 @@ DWORD handle(DWORD function, DWORD arg1, DWORD arg2, DWORD arg3) case Syscall::PosixGetcwd: return current->sys$getcwd((char*)arg1, (size_t)arg2); case Syscall::PosixOpen: - //kprintf("syscall: open('%s', %u)\n", arg1, arg2); - return current->sys$open((const char*)arg1, (size_t)arg2); + return current->sys$open((const char*)arg1, (int)arg2); case Syscall::PosixClose: //kprintf("syscall: close(%d)\n", arg1); return current->sys$close((int)arg1); @@ -104,7 +100,7 @@ DWORD handle(DWORD function, DWORD arg1, DWORD arg2, DWORD arg3) return current->sys$gethostname((char*)arg1, (size_t)arg2); case Syscall::PosixExit: cli(); - locker.unlock(); + //locker.unlock(); current->sys$exit((int)arg1); ASSERT_NOT_REACHED(); return 0; diff --git a/Kernel/Task.cpp b/Kernel/Task.cpp index 60d5340d8e6..ab655aaa4f9 100644 --- a/Kernel/Task.cpp +++ b/Kernel/Task.cpp @@ -17,6 +17,7 @@ //#define DEBUG_IO //#define TASK_DEBUG +//#define SCHEDULER_DEBUG #define VALIDATE_USER_BUFFER(b, s) \ do { \ @@ -605,12 +606,12 @@ bool scheduleNewTask() } } -#if 0 - kprintf("Scheduler choices:\n"); +#ifdef SCHEDULER_DEBUG + dbgprintf("Scheduler choices:\n"); for (auto* task = s_tasks->head(); task; task = task->next()) { - if (task->state() == Task::BlockedWait || task->state() == Task::BlockedSleep) - continue; - kprintf("%w %s(%u)\n", task->state(), task->name().characters(), task->pid()); + //if (task->state() == Task::BlockedWait || task->state() == Task::BlockedSleep) +// continue; + dbgprintf("%w %s(%u)\n", task->state(), task->name().characters(), task->pid()); } #endif @@ -621,7 +622,9 @@ bool scheduleNewTask() auto* task = s_tasks->head(); if (task->state() == Task::Runnable || task->state() == Task::Running) { - //kprintf("switch to %s (%p vs %p)\n", task->name().characters(), task, current); +#ifdef SCHEDULER_DEBUG + dbgprintf("switch to %s(%u) (%p vs %p)\n", task->name().characters(), task->pid(), task, current); +#endif return contextSwitch(task); } @@ -844,7 +847,7 @@ int Task::sys$open(const char* path, int options) if (m_fileHandles.size() >= m_maxFileHandles) return -EMFILE; int error; - auto handle = VirtualFileSystem::the().open(path, error, 0, cwdInode()); + auto handle = VirtualFileSystem::the().open(path, error, options, cwdInode()); if (!handle) return error; if (options & O_DIRECTORY && !handle->isDirectory()) diff --git a/Kernel/init.cpp b/Kernel/init.cpp index 5cdcb1cd90b..5a70233312b 100644 --- a/Kernel/init.cpp +++ b/Kernel/init.cpp @@ -247,3 +247,17 @@ void init() } } +void log_try_lock(const char* where) +{ + kprintf("[%u] >>> locking... (%s)\n", current->pid(), where); +} + +void log_locked(const char* where) +{ + kprintf("[%u] >>> locked() in %s\n", current->pid(), where); +} + +void log_unlocked(const char* where) +{ + kprintf("[%u] <<< unlocked()\n", current->pid(), where); +} diff --git a/Kernel/kassert.h b/Kernel/kassert.h index edcfe8ae5c1..cd858bca70e 100644 --- a/Kernel/kassert.h +++ b/Kernel/kassert.h @@ -8,3 +8,4 @@ #define RELEASE_ASSERT(x) do { if (!(x)) CRASH(); } while(0) #define ASSERT_NOT_REACHED() ASSERT(false) #define ASSERT_INTERRUPTS_DISABLED() ASSERT(!(cpuFlags() & 0x200)) +#define ASSERT_INTERRUPTS_ENABLED() ASSERT(cpuFlags() & 0x200) diff --git a/Kernel/kmalloc.cpp b/Kernel/kmalloc.cpp index fbb56569c60..6d6825a5e2d 100644 --- a/Kernel/kmalloc.cpp +++ b/Kernel/kmalloc.cpp @@ -20,7 +20,7 @@ typedef struct } PACKED allocation_t; #define CHUNK_SIZE 128 -#define POOL_SIZE (512 * 1024) +#define POOL_SIZE (1024 * 1024) #define BASE_PHYS 0x200000 diff --git a/Kernel/kprintf.cpp b/Kernel/kprintf.cpp index 463a3ce915a..f084eafc8e5 100644 --- a/Kernel/kprintf.cpp +++ b/Kernel/kprintf.cpp @@ -1,10 +1,11 @@ #include "kprintf.h" #include "Console.h" +#include "IO.h" #include #include #include -static void console_putch(char*, char ch) +static void console_putch(char*&, char ch) { Console::the().write((byte*)&ch, 1); } @@ -32,3 +33,17 @@ int ksprintf(char* buffer, const char* fmt, ...) va_end(ap); return ret; } + +static void debugger_putch(char*&, char ch) +{ + IO::out8(0xe9, ch); +} + +int dbgprintf(const char* fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + int ret = printfInternal(debugger_putch, nullptr, fmt, ap); + va_end(ap); + return ret; +} diff --git a/Kernel/kprintf.h b/Kernel/kprintf.h index d35d9d492ac..bb7eeb01442 100644 --- a/Kernel/kprintf.h +++ b/Kernel/kprintf.h @@ -2,6 +2,7 @@ #include +int dbgprintf(const char *fmt, ...); int kprintf(const char *fmt, ...); int ksprintf(char* buf, const char *fmt, ...); diff --git a/Userland/sh.cpp b/Userland/sh.cpp index a0e3d1f9765..e501ec02f8a 100644 --- a/Userland/sh.cpp +++ b/Userland/sh.cpp @@ -17,7 +17,7 @@ static void prompt() if (getuid() == 0) printf("# "); else - printf("\033[32;1m%s\033[0m:\033[34;1m%s\033[0m$> ", g->hostname, g->cwd.characters()); + printf("(%u) \033[32;1m%s\033[0m:\033[34;1m%s\033[0m$> ", getpid(), g->hostname, g->cwd.characters()); } static int sh_pwd(int, const char**) @@ -91,6 +91,21 @@ static bool handle_builtin(int argc, const char** argv, int& retval) return false; } +static int try_spawn(const char* path, const char** argv) +{ + int ret = spawn(path, argv); + if (ret >= 0) + return ret; + + const char* search_path = "/bin"; + char pathbuf[128]; + sprintf(pathbuf, "%s/%s", search_path, argv[0]); + ret = spawn(pathbuf, argv); + if (ret == -1) + return -1; + return ret; +} + static int runcmd(char* cmd) { if (cmd[0] == 0) @@ -115,15 +130,12 @@ static int runcmd(char* cmd) return 0; } - const char* search_path = "/bin"; - - char pathbuf[128]; - sprintf(pathbuf, "%s/%s", search_path, argv[0]); - int ret = spawn(pathbuf, argv); - if (ret == -1) { + int ret = try_spawn(argv[0], argv); + if (ret < 0) { printf("spawn failed: %s (%s)\n", cmd, strerror(errno)); return 1; } + // FIXME: waitpid should give us the spawned process's exit status int wstatus = 0; waitpid(ret, &wstatus, 0); diff --git a/VirtualFileSystem/DiskBackedFileSystem.cpp b/VirtualFileSystem/DiskBackedFileSystem.cpp index 845e470eac6..1651e328784 100644 --- a/VirtualFileSystem/DiskBackedFileSystem.cpp +++ b/VirtualFileSystem/DiskBackedFileSystem.cpp @@ -7,6 +7,7 @@ typedef int InterruptDisabler; #endif //#define DBFS_DEBUG +#define BLOCK_CACHE DiskBackedFileSystem::DiskBackedFileSystem(RetainPtr&& device) : m_device(move(device)) @@ -42,6 +43,8 @@ ByteBuffer DiskBackedFileSystem::readBlock(unsigned index) const #ifdef DBFS_DEBUG kprintf("DiskBackedFileSystem::readBlock %u\n", index); #endif + +#ifdef BLOCK_CACHE { InterruptDisabler disabler; auto it = m_blockCache.find(index); @@ -49,19 +52,24 @@ ByteBuffer DiskBackedFileSystem::readBlock(unsigned index) const return (*it).value; } } +#endif auto buffer = ByteBuffer::createUninitialized(blockSize()); + //kprintf("created block buffer with size %u\n", blockSize()); DiskOffset baseOffset = static_cast(index) * static_cast(blockSize()); auto* bufferPointer = buffer.pointer(); - device().read(baseOffset, blockSize(), bufferPointer); + bool success = device().read(baseOffset, blockSize(), bufferPointer); + ASSERT(success); ASSERT(buffer.size() == blockSize()); +#ifdef BLOCK_CACHE { InterruptDisabler disabler; if (m_blockCache.size() >= 32) m_blockCache.removeOneRandomly(); m_blockCache.set(index, buffer); } +#endif return buffer; } diff --git a/VirtualFileSystem/DiskDevice.cpp b/VirtualFileSystem/DiskDevice.cpp index 54f6953b9aa..122fb773d9e 100644 --- a/VirtualFileSystem/DiskDevice.cpp +++ b/VirtualFileSystem/DiskDevice.cpp @@ -10,6 +10,7 @@ DiskDevice::~DiskDevice() bool DiskDevice::read(DiskOffset offset, unsigned length, byte* out) const { + //kprintf("DD::read %u x%u\n", offset, length); ASSERT((offset % blockSize()) == 0); ASSERT((length % blockSize()) == 0); dword firstBlock = offset / blockSize(); diff --git a/VirtualFileSystem/Ext2FileSystem.cpp b/VirtualFileSystem/Ext2FileSystem.cpp index 4c583162b3c..c8e03507ca4 100644 --- a/VirtualFileSystem/Ext2FileSystem.cpp +++ b/VirtualFileSystem/Ext2FileSystem.cpp @@ -293,7 +293,6 @@ Unix::ssize_t Ext2FileSystem::readInodeBytes(InodeIdentifier inode, Unix::off_t static const unsigned maxInlineSymlinkLength = 60; if (isSymbolicLink(e2inode->i_mode) && e2inode->i_size < maxInlineSymlinkLength) { Unix::ssize_t nread = min((Unix::off_t)e2inode->i_size - offset, static_cast(count)); - kprintf("nread = %d\n", nread); memcpy(buffer, e2inode->i_block + offset, nread); return nread; } diff --git a/VirtualFileSystem/FileHandle.cpp b/VirtualFileSystem/FileHandle.cpp index 4e61f1b172c..55b9f70cb7c 100644 --- a/VirtualFileSystem/FileHandle.cpp +++ b/VirtualFileSystem/FileHandle.cpp @@ -25,7 +25,7 @@ bool additionWouldOverflow(Unix::off_t a, Unix::off_t b) int FileHandle::stat(Unix::stat* buffer) { - Locker locker(VirtualFileSystem::lock()); + LOCKER(VirtualFileSystem::lock()); if (!m_vnode) return -EBADF; @@ -52,7 +52,7 @@ int FileHandle::stat(Unix::stat* buffer) Unix::off_t FileHandle::seek(Unix::off_t offset, int whence) { - Locker locker(VirtualFileSystem::lock()); + LOCKER(VirtualFileSystem::lock()); if (!m_vnode) return -EBADF; @@ -96,7 +96,7 @@ Unix::off_t FileHandle::seek(Unix::off_t offset, int whence) Unix::ssize_t FileHandle::read(byte* buffer, Unix::size_t count) { - Locker locker(VirtualFileSystem::lock()); + LOCKER(VirtualFileSystem::lock()); if (m_vnode->isCharacterDevice()) { // FIXME: What should happen to m_currentOffset? @@ -116,7 +116,7 @@ bool FileHandle::hasDataAvailableForRead() ByteBuffer FileHandle::readEntireFile() { - Locker locker(VirtualFileSystem::lock()); + LOCKER(VirtualFileSystem::lock()); if (m_vnode->isCharacterDevice()) { auto buffer = ByteBuffer::createUninitialized(1024); @@ -135,7 +135,7 @@ bool FileHandle::isDirectory() const ssize_t FileHandle::get_dir_entries(byte* buffer, Unix::size_t size) { - Locker locker(VirtualFileSystem::lock()); + LOCKER(VirtualFileSystem::lock()); auto metadata = m_vnode->metadata(); if (!metadata.isValid()) diff --git a/VirtualFileSystem/FileSystem.cpp b/VirtualFileSystem/FileSystem.cpp index 6bb738245d3..ad243640b91 100644 --- a/VirtualFileSystem/FileSystem.cpp +++ b/VirtualFileSystem/FileSystem.cpp @@ -78,11 +78,13 @@ ByteBuffer FileSystem::readEntireInode(InodeIdentifier inode, FileHandle* handle auto contents = ByteBuffer::createUninitialized(initialSize); Unix::ssize_t nread; - byte buffer[512]; + byte buffer[4096]; byte* out = contents.pointer(); Unix::off_t offset = 0; for (;;) { nread = readInodeBytes(inode, offset, sizeof(buffer), buffer, handle); + //kprintf("nread: %u, bufsiz: %u, initialSize: %u\n", nread, sizeof(buffer), initialSize); + ASSERT(nread <= sizeof(buffer)); if (nread <= 0) break; memcpy(out, buffer, nread); @@ -95,6 +97,7 @@ ByteBuffer FileSystem::readEntireInode(InodeIdentifier inode, FileHandle* handle return nullptr; } + contents.trim(offset); return contents; } diff --git a/VirtualFileSystem/Makefile b/VirtualFileSystem/Makefile index f54a7461312..cac239bf7c9 100644 --- a/VirtualFileSystem/Makefile +++ b/VirtualFileSystem/Makefile @@ -6,6 +6,7 @@ AK_OBJS = \ ../AK/MappedFile.o \ ../AK/TemporaryFile.o \ ../AK/SimpleMalloc.o \ + ../AK/StringBuilder.o \ ../AK/kmalloc.o VFS_OBJS = \ diff --git a/VirtualFileSystem/SyntheticFileSystem.cpp b/VirtualFileSystem/SyntheticFileSystem.cpp index db8b34e8d5c..743e10a3d5d 100644 --- a/VirtualFileSystem/SyntheticFileSystem.cpp +++ b/VirtualFileSystem/SyntheticFileSystem.cpp @@ -2,6 +2,11 @@ #include "FileHandle.h" #include +#ifndef SERENITY +typedef int InterruptDisabler; +#define ASSERT_INTERRUPTS_DISABLED() +#endif + //#define SYNTHFS_DEBUG RetainPtr SyntheticFileSystem::create() @@ -31,11 +36,10 @@ bool SyntheticFileSystem::initialize() rootDir->metadata.mtime = mepoch; m_inodes.set(RootInodeIndex, move(rootDir)); -#if 0 #ifndef SERENITY addFile(createTextFile("file", "I'm a synthetic file!\n")); addFile(createTextFile("message", "Hey! This isn't my bottle!\n")); -#endif + addFile(createGeneratedFile("lunk", [] { return String("/home/andreas/file1").toByteBuffer(); }, 00120777)); #endif return true; } @@ -60,7 +64,7 @@ auto SyntheticFileSystem::createTextFile(String&& name, String&& text) -> OwnPtr file->metadata.size = file->data.size(); file->metadata.uid = 100; file->metadata.gid = 200; - file->metadata.mode = 0040644; + file->metadata.mode = 0010644; file->metadata.mtime = mepoch; return file; } diff --git a/VirtualFileSystem/VirtualFileSystem.cpp b/VirtualFileSystem/VirtualFileSystem.cpp index 6da36472a7f..cf9fe49a873 100644 --- a/VirtualFileSystem/VirtualFileSystem.cpp +++ b/VirtualFileSystem/VirtualFileSystem.cpp @@ -103,10 +103,11 @@ auto VirtualFileSystem::getOrCreateNode(InodeIdentifier inode) -> RetainPtr&& fileSystem, const String& path) { + kprintf("mount\n"); + LOCKER(VirtualFileSystem::lock()); ASSERT(fileSystem); - int error; - auto inode = resolvePath(path, error); + auto inode = resolvePath(path, error, locker); if (!inode.isValid()) { kprintf("[VFS] mount can't resolve mount point '%s'\n", path.characters()); return false; @@ -150,6 +151,7 @@ bool VirtualFileSystem::mountRoot(RetainPtr&& fileSystem) auto VirtualFileSystem::allocateNode() -> RetainPtr { + if (m_nodeFreeList.isEmpty()) { kprintf("[VFS] allocateNode has no nodes left\n"); return nullptr; @@ -172,6 +174,7 @@ void VirtualFileSystem::freeNode(Node* node) m_nodeFreeList.append(move(node)); } +#ifndef SERENITY bool VirtualFileSystem::isDirectory(const String& path, InodeIdentifier base) { int error; @@ -181,6 +184,7 @@ bool VirtualFileSystem::isDirectory(const String& path, InodeIdentifier base) return inode.metadata().isDirectory(); } +#endif auto VirtualFileSystem::findMountForHost(InodeIdentifier inode) -> Mount* { @@ -227,6 +231,7 @@ void VirtualFileSystem::enumerateDirectoryInode(InodeIdentifier directoryInode, }); } +#ifndef SERENITY void VirtualFileSystem::listDirectory(const String& path) { int error; @@ -351,13 +356,14 @@ void VirtualFileSystem::listDirectoryRecursively(const String& path) return true; }); } +#endif bool VirtualFileSystem::touch(const String& path) { - Locker locker(VirtualFileSystem::lock()); + LOCKER(VirtualFileSystem::lock()); int error; - auto inode = resolvePath(path, error); + auto inode = resolvePath(path, error, locker); if (!inode.isValid()) return false; return inode.fileSystem()->setModificationTime(inode, ktime(nullptr)); @@ -365,9 +371,9 @@ bool VirtualFileSystem::touch(const String& path) OwnPtr VirtualFileSystem::open(const String& path, int& error, int options, InodeIdentifier base) { - Locker locker(VirtualFileSystem::lock()); + LOCKER(VirtualFileSystem::lock()); - auto inode = resolvePath(path, error, base, options); + auto inode = resolvePath(path, error, locker, base, options); if (!inode.isValid()) return nullptr; auto vnode = getOrCreateNode(inode); @@ -378,7 +384,7 @@ OwnPtr VirtualFileSystem::open(const String& path, int& error, int o OwnPtr VirtualFileSystem::create(const String& path, InodeIdentifier base) { - Locker locker(VirtualFileSystem::lock()); + LOCKER(VirtualFileSystem::lock()); // FIXME: Do the real thing, not just this fake thing! (void) path; @@ -388,26 +394,29 @@ OwnPtr VirtualFileSystem::create(const String& path, InodeIdentifier OwnPtr VirtualFileSystem::mkdir(const String& path, InodeIdentifier base) { - Locker locker(VirtualFileSystem::lock()); - + LOCKER(VirtualFileSystem::lock()); // FIXME: Do the real thing, not just this fake thing! (void) path; m_rootNode->fileSystem()->makeDirectory(m_rootNode->fileSystem()->rootInode(), "mydir", 0400755); return nullptr; } -InodeIdentifier VirtualFileSystem::resolveSymbolicLink(InodeIdentifier base, InodeIdentifier symlinkInode, int& error) +InodeIdentifier VirtualFileSystem::resolveSymbolicLink(InodeIdentifier base, InodeIdentifier symlinkInode, int& error, Locker& locker) { + locker.unlock(); auto symlinkContents = symlinkInode.readEntireFile(); + locker.lock(); if (!symlinkContents) return { }; - return resolvePath((const char*)symlinkContents.pointer(), error, base); + auto linkee = String((const char*)symlinkContents.pointer(), symlinkContents.size()); +#ifdef VFS_DEBUG + kprintf("linkee (%s)(%u) from %u:%u\n", linkee.characters(), linkee.length(), base.fileSystemID(), base.index()); +#endif + return resolvePath(linkee, error, locker, base); } -String VirtualFileSystem::absolutePath(InodeIdentifier inode) +String VirtualFileSystem::absolutePathInternal(InodeIdentifier inode, Locker& locker) { - Locker locker(VirtualFileSystem::lock()); - if (!inode.isValid()) return String(); @@ -419,7 +428,7 @@ String VirtualFileSystem::absolutePath(InodeIdentifier inode) else lineage.append(inode); if (inode.metadata().isDirectory()) { - inode = resolvePath("..", error, inode); + inode = resolvePath("..", error, locker, inode); } else inode = inode.fileSystem()->findParentOfInode(inode); ASSERT(inode.isValid()); @@ -439,7 +448,13 @@ String VirtualFileSystem::absolutePath(InodeIdentifier inode) return builder.build(); } -InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, InodeIdentifier base, int options) +String VirtualFileSystem::absolutePath(InodeIdentifier inode) +{ + LOCKER(VirtualFileSystem::lock()); + return absolutePathInternal(inode, locker); +} + +InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, Locker& locker, InodeIdentifier base, int options) { if (path.isEmpty()) return { }; @@ -465,7 +480,7 @@ InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, I } if (!metadata.isDirectory()) { #ifdef VFS_DEBUG - kprintf("not directory\n"); + kprintf("parent of <%s> not directory, it's inode %u:%u / %u:%u, mode: %u, size: %u\n", part.characters(), inode.fileSystemID(), inode.index(), metadata.inode.fileSystemID(), metadata.inode.index(), metadata.mode, metadata.size); #endif error = -EIO; return { }; @@ -474,7 +489,7 @@ InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, I inode = inode.fileSystem()->childOfDirectoryInodeWithName(inode, part); if (!inode.isValid()) { #ifdef VFS_DEBUG - kprintf("bad child\n"); + kprintf("child <%s>(%u) not found in directory, %02u:%08u\n", part.characters(), part.length(), parent.fileSystemID(), parent.index()); #endif error = -ENOENT; return { }; @@ -506,12 +521,7 @@ InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, I if (options & O_NOFOLLOW_NOERROR) return inode; } - char buf[4096] = ""; - char* p = buf; - for (unsigned j = 0; j < i; ++j) { - p += ksprintf(p, "/%s", parts[j].characters()); - } - inode = resolveSymbolicLink(parent, inode, error); + inode = resolveSymbolicLink(parent, inode, error, locker); if (!inode.isValid()) { kprintf("Symbolic link resolution failed :(\n"); return { }; diff --git a/VirtualFileSystem/VirtualFileSystem.h b/VirtualFileSystem/VirtualFileSystem.h index 183fba5a1ca..2c47915ab70 100644 --- a/VirtualFileSystem/VirtualFileSystem.h +++ b/VirtualFileSystem/VirtualFileSystem.h @@ -104,9 +104,11 @@ public: private: friend class FileHandle; + String absolutePathInternal(InodeIdentifier, Locker&); + void enumerateDirectoryInode(InodeIdentifier, Function); - InodeIdentifier resolvePath(const String& path, int& error, InodeIdentifier base = InodeIdentifier(), int options = 0); - InodeIdentifier resolveSymbolicLink(InodeIdentifier base, InodeIdentifier symlinkInode, int& error); + InodeIdentifier resolvePath(const String& path, int& error, Locker&, InodeIdentifier base = InodeIdentifier(), int options = 0); + InodeIdentifier resolveSymbolicLink(InodeIdentifier base, InodeIdentifier symlinkInode, int& error, Locker&); RetainPtr allocateNode(); void freeNode(Node*); diff --git a/VirtualFileSystem/test.cpp b/VirtualFileSystem/test.cpp index a883155a3d7..6a987c351a5 100644 --- a/VirtualFileSystem/test.cpp +++ b/VirtualFileSystem/test.cpp @@ -53,7 +53,8 @@ int main(int c, char** v) //return 0; if (!strcmp(v[0], "./vcat")) { - auto handle = vfs.open(v[2]); + int error; + auto handle = vfs.open(v[2], error); if (!handle) { printf("failed to open %s inside fs image\n", v[2]); return 1; @@ -149,7 +150,8 @@ int main(int c, char** v) if (cmd == "stat" && parts.size() > 1) { char buf[1024]; sprintf(buf, "%s/%s", currentDirectory.characters(), parts[1].characters()); - auto handle = vfs.open(buf); + int error; + auto handle = vfs.open(buf, error); if (!handle) { printf("Can't open '%s' :(\n", buf); continue; @@ -179,7 +181,8 @@ int main(int c, char** v) if (cmd == "cat" && parts.size() > 1) { char pathbuf[1024]; sprintf(pathbuf, "%s/%s", currentDirectory.characters(), parts[1].characters()); - auto handle = vfs.open(pathbuf); + int error; + auto handle = vfs.open(pathbuf, error); if (!handle) { printf("failed to open %s\n", pathbuf); continue; @@ -192,7 +195,8 @@ int main(int c, char** v) if (cmd == "kat" && parts.size() > 1) { char pathbuf[1024]; sprintf(pathbuf, "%s/%s", currentDirectory.characters(), parts[1].characters()); - auto handle = vfs.open(pathbuf); + int error; + auto handle = vfs.open(pathbuf, error); if (!handle) { printf("failed to open %s\n", pathbuf); continue;