RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 10:37:45 +00:00
Code Issues Activity Actions

Fix broken SpinLock.

Browse source 
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.
This commit is contained in:
Andreas Kling 2018-10-29 21:54:11 +01:00
parent bea106fdb2
commit e6284a8774
24 changed files with 195 additions and 77 deletions
Download patch file Download diff file Expand all files Collapse all files

10
VirtualFileSystem/DiskBackedFileSystem.cpp
View file

@ -7,6 +7,7 @@ typedef int InterruptDisabler;
#endif
//#define DBFS_DEBUG
#define BLOCK_CACHE
DiskBackedFileSystem::DiskBackedFileSystem(RetainPtr<DiskDevice>&& 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<DiskOffset>(index) * static_cast<DiskOffset>(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;
}

1
VirtualFileSystem/DiskDevice.cpp
View file

@ -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();

1
VirtualFileSystem/Ext2FileSystem.cpp
View file

@ -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<Unix::off_t>(count));
kprintf("nread = %d\n", nread);
memcpy(buffer, e2inode->i_block + offset, nread);
return nread;
}

10
VirtualFileSystem/FileHandle.cpp
View file

@ -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())

5
VirtualFileSystem/FileSystem.cpp
View file

@ -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;
}

1
VirtualFileSystem/Makefile
View file

@ -6,6 +6,7 @@ AK_OBJS = \
../AK/MappedFile.o \
../AK/TemporaryFile.o \
../AK/SimpleMalloc.o \
../AK/StringBuilder.o \
../AK/kmalloc.o
VFS_OBJS = \

10
VirtualFileSystem/SyntheticFileSystem.cpp
View file

@ -2,6 +2,11 @@
#include "FileHandle.h"
#include <AK/StdLib.h>
#ifndef SERENITY
typedef int InterruptDisabler;
#define ASSERT_INTERRUPTS_DISABLED()
#endif
//#define SYNTHFS_DEBUG
RetainPtr<SyntheticFileSystem> 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;
}

58
VirtualFileSystem/VirtualFileSystem.cpp
View file

@ -103,10 +103,11 @@ auto VirtualFileSystem::getOrCreateNode(InodeIdentifier inode) -> RetainPtr<Node
bool VirtualFileSystem::mount(RetainPtr<FileSystem>&& 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>&& fileSystem)
auto VirtualFileSystem::allocateNode() -> RetainPtr<Node>
{
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<FileHandle> 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<FileHandle> VirtualFileSystem::open(const String& path, int& error, int o
OwnPtr<FileHandle> 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<FileHandle> VirtualFileSystem::create(const String& path, InodeIdentifier
OwnPtr<FileHandle> 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 { };

6
VirtualFileSystem/VirtualFileSystem.h
View file

@ -104,9 +104,11 @@ public:
private:
friend class FileHandle;
String absolutePathInternal(InodeIdentifier, Locker&);
void enumerateDirectoryInode(InodeIdentifier, Function<bool(const FileSystem::DirectoryEntry&)>);
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<Node> allocateNode();
void freeNode(Node*);

12
VirtualFileSystem/test.cpp
View file

@ -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;