Bananymous/banan-os
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Kernel: Reduce ext2 locking

Browse Source 
Replace the mutex with a rwlock and lock when its not necessary
This commit is contained in:
Bananymous
2026-05-14 17:23:06 +03:00
parent dd8a9b1793
commit c352fb600f
2 changed files with 109 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
kernel/include/kernel/FS/Ext2/Inode.h
View File

@@ -4,7 +4,7 @@
#include <BAN/StringView.h> #include <BAN/StringView.h>
#include <kernel/FS/Ext2/Definitions.h> #include <kernel/FS/Ext2/Definitions.h>
#include <kernel/FS/Inode.h> #include <kernel/FS/Inode.h>
#include <kernel/Lock/Mutex.h> #include <kernel/Lock/RWLock.h>
namespace Kernel namespace Kernel
{ {
@@ -58,25 +58,30 @@ namespace Kernel
virtual bool has_hungup_impl() const override { return false; } virtual bool has_hungup_impl() const override { return false; }
private: private:
uint32_t block_group() const;
// Returns maximum number of data blocks in use // Returns maximum number of data blocks in use
// NOTE: the inode might have more blocks than what this suggests if it has been shrinked // NOTE: the inode might have more blocks than what this suggests if it has been shrinked
uint32_t max_used_data_block_count() const { return size() / blksize(); } uint32_t max_used_data_block_count() const { return size() / blksize(); }
BAN::ErrorOr<BAN::Optional<uint32_t>> block_from_indirect_block(uint32_t& block, uint32_t index, uint32_t depth, bool allocate); BAN::ErrorOr<void> sync_no_lock();
BAN::ErrorOr<BAN::Optional<uint32_t>> fs_block_of_data_block_index(uint32_t data_block_index, bool allocate);
BAN::ErrorOr<void> link_inode_to_directory(Ext2Inode&, BAN::StringView name); BAN::ErrorOr<bool> is_directory_empty_no_lock();
BAN::ErrorOr<void> remove_inode_from_directory(BAN::StringView name, bool cleanup_directory); BAN::ErrorOr<BAN::RefPtr<Inode>> find_inode_no_lock(BAN::StringView);
BAN::ErrorOr<bool> is_directory_empty();
BAN::ErrorOr<void> cleanup_indirect_block(uint32_t block, uint32_t depth); /* needs write end of the lock when allocate is true*/
BAN::ErrorOr<void> cleanup_default_links(); BAN::ErrorOr<BAN::Optional<uint32_t>> block_from_indirect_block_no_lock(uint32_t& block, uint32_t index, uint32_t depth, bool allocate);
BAN::ErrorOr<void> cleanup_data_blocks(); BAN::ErrorOr<BAN::Optional<uint32_t>> fs_block_of_data_block_index_no_lock(uint32_t data_block_index, bool allocate);
BAN::ErrorOr<void> cleanup_from_fs();
BAN::ErrorOr<void> sync(); /* needs write end of the lock */
BAN::ErrorOr<void> link_inode_to_directory_no_lock(Ext2Inode&, BAN::StringView name);
BAN::ErrorOr<void> remove_inode_from_directory_no_lock(BAN::StringView name, bool cleanup_directory);
uint32_t block_group() const; /* needs write end of the lock */
BAN::ErrorOr<void> cleanup_indirect_block_no_lock(uint32_t block, uint32_t depth);
BAN::ErrorOr<void> cleanup_default_links_no_lock();
BAN::ErrorOr<void> cleanup_data_blocks_no_lock();
BAN::ErrorOr<void> cleanup_from_fs_no_lock();
private: private:
Ext2Inode(Ext2FS& fs, Ext2::Inode inode, uint32_t ino) Ext2Inode(Ext2FS& fs, Ext2::Inode inode, uint32_t ino)
@@ -98,7 +103,7 @@ namespace Kernel
{ {
if (memcmp(&inode.m_inode, &inode_info, sizeof(Ext2::Inode)) == 0) if (memcmp(&inode.m_inode, &inode_info, sizeof(Ext2::Inode)) == 0)
return; return;
if (auto ret = inode.sync(); ret.is_error()) if (auto ret = inode.sync_no_lock(); ret.is_error())
dwarnln("failed to sync inode: {}", ret.error()); dwarnln("failed to sync inode: {}", ret.error());
} }
@@ -111,8 +116,7 @@ namespace Kernel
Ext2::Inode m_inode; Ext2::Inode m_inode;
const uint32_t m_ino; const uint32_t m_ino;
// TODO: try to reduce locking or replace this with rwlock(?) RWLock m_lock;
Mutex m_lock;
friend class Ext2FS; friend class Ext2FS;
friend class BAN::RefPtr<Ext2Inode>; friend class BAN::RefPtr<Ext2Inode>;

197
kernel/kernel/FS/Ext2/Inode.cpp
View File

@@ -47,7 +47,7 @@ namespace Kernel
{ {
if (m_inode.links_count > 0) if (m_inode.links_count > 0)
return; return;
if (auto ret = cleanup_from_fs(); ret.is_error()) if (auto ret = cleanup_from_fs_no_lock(); ret.is_error())
dwarnln("Could not cleanup inode from FS: {}", ret.error()); dwarnln("Could not cleanup inode from FS: {}", ret.error());
} }
@@ -56,13 +56,11 @@ namespace Kernel
return &m_fs; return &m_fs;
} }
BAN::ErrorOr<BAN::Optional<uint32_t>> Ext2Inode::block_from_indirect_block(uint32_t& block, uint32_t index, uint32_t depth, bool allocate) BAN::ErrorOr<BAN::Optional<uint32_t>> Ext2Inode::block_from_indirect_block_no_lock(uint32_t& block, uint32_t index, uint32_t depth, bool allocate)
{ {
const uint32_t inode_blocks_per_fs_block = blksize() / 512; const uint32_t inode_blocks_per_fs_block = blksize() / 512;
const uint32_t indices_per_fs_block = blksize() / sizeof(uint32_t); const uint32_t indices_per_fs_block = blksize() / sizeof(uint32_t);
LockGuard _(m_lock);
if (block == 0 && !allocate) if (block == 0 && !allocate)
return BAN::Optional<uint32_t>(); return BAN::Optional<uint32_t>();
@@ -104,7 +102,7 @@ namespace Kernel
uint32_t& new_block = block_buffer.span().as_span<uint32_t>()[(index / divisor) % indices_per_fs_block]; uint32_t& new_block = block_buffer.span().as_span<uint32_t>()[(index / divisor) % indices_per_fs_block];
const auto old_block = new_block; const auto old_block = new_block;
const auto result = TRY(block_from_indirect_block(new_block, index, depth - 1, allocate)); const auto result = TRY(block_from_indirect_block_no_lock(new_block, index, depth - 1, allocate));
if (needs_write || old_block != new_block) if (needs_write || old_block != new_block)
TRY(m_fs.write_block(block, block_buffer)); TRY(m_fs.write_block(block, block_buffer));
@@ -112,13 +110,11 @@ namespace Kernel
return result; return result;
} }
BAN::ErrorOr<BAN::Optional<uint32_t>> Ext2Inode::fs_block_of_data_block_index(uint32_t data_block_index, bool allocate) BAN::ErrorOr<BAN::Optional<uint32_t>> Ext2Inode::fs_block_of_data_block_index_no_lock(uint32_t data_block_index, bool allocate)
{ {
const uint32_t inode_blocks_per_fs_block = blksize() / 512; const uint32_t inode_blocks_per_fs_block = blksize() / 512;
const uint32_t indices_per_block = blksize() / sizeof(uint32_t); const uint32_t indices_per_block = blksize() / sizeof(uint32_t);
LockGuard _(m_lock);
if (data_block_index < 12) if (data_block_index < 12)
{ {
if (m_inode.block[data_block_index] != 0) if (m_inode.block[data_block_index] != 0)
@@ -140,15 +136,15 @@ namespace Kernel
data_block_index -= 12; data_block_index -= 12;
if (data_block_index < indices_per_block) if (data_block_index < indices_per_block)
return block_from_indirect_block(m_inode.block[12], data_block_index, 1, allocate); return block_from_indirect_block_no_lock(m_inode.block[12], data_block_index, 1, allocate);
data_block_index -= indices_per_block; data_block_index -= indices_per_block;
if (data_block_index < indices_per_block * indices_per_block) if (data_block_index < indices_per_block * indices_per_block)
return block_from_indirect_block(m_inode.block[13], data_block_index, 2, allocate); return block_from_indirect_block_no_lock(m_inode.block[13], data_block_index, 2, allocate);
data_block_index -= indices_per_block * indices_per_block; data_block_index -= indices_per_block * indices_per_block;
if (data_block_index < indices_per_block * indices_per_block * indices_per_block) if (data_block_index < indices_per_block * indices_per_block * indices_per_block)
return block_from_indirect_block(m_inode.block[14], data_block_index, 3, allocate); return block_from_indirect_block_no_lock(m_inode.block[14], data_block_index, 3, allocate);
ASSERT_NOT_REACHED(); ASSERT_NOT_REACHED();
} }
@@ -157,7 +153,7 @@ namespace Kernel
{ {
ASSERT(mode().iflnk()); ASSERT(mode().iflnk());
LockGuard _(m_lock); RWLockRDGuard _(m_lock);
if (m_inode.size < sizeof(m_inode.block)) if (m_inode.size < sizeof(m_inode.block))
{ {
@@ -176,16 +172,15 @@ namespace Kernel
{ {
ASSERT(mode().iflnk()); ASSERT(mode().iflnk());
LockGuard _(m_lock); RWLockWRGuard _(m_lock);
if (target.size() < sizeof(m_inode.block)) if (target.size() < sizeof(m_inode.block))
{ {
if (m_inode.size >= sizeof(m_inode.block)) if (m_inode.size >= sizeof(m_inode.block))
TRY(cleanup_data_blocks()); TRY(cleanup_data_blocks_no_lock());
memset(m_inode.block, 0, sizeof(m_inode.block)); memset(m_inode.block, 0, sizeof(m_inode.block));
memcpy(m_inode.block, target.data(), target.size()); memcpy(m_inode.block, target.data(), target.size());
m_inode.size = target.size(); m_inode.size = target.size();
TRY(sync()); TRY(sync_no_lock());
return {}; return {};
} }
@@ -204,7 +199,7 @@ namespace Kernel
if (static_cast<BAN::make_unsigned_t<decltype(offset)>>(offset) >= UINT32_MAX || buffer.size() >= UINT32_MAX || buffer.size() >= (size_t)(UINT32_MAX - offset)) if (static_cast<BAN::make_unsigned_t<decltype(offset)>>(offset) >= UINT32_MAX || buffer.size() >= UINT32_MAX || buffer.size() >= (size_t)(UINT32_MAX - offset))
return BAN::Error::from_errno(EOVERFLOW); return BAN::Error::from_errno(EOVERFLOW);
LockGuard _0(m_lock); RWLockRDGuard _0(m_lock);
if (static_cast<BAN::make_unsigned_t<decltype(offset)>>(offset) >= m_inode.size) if (static_cast<BAN::make_unsigned_t<decltype(offset)>>(offset) >= m_inode.size)
return 0; return 0;
@@ -226,7 +221,7 @@ namespace Kernel
for (uint32_t data_block_index = first_block; data_block_index < last_block; data_block_index++) for (uint32_t data_block_index = first_block; data_block_index < last_block; data_block_index++)
{ {
auto block_index = TRY(fs_block_of_data_block_index(data_block_index, false)); auto block_index = TRY(fs_block_of_data_block_index_no_lock(data_block_index, false));
if (block_index.has_value()) if (block_index.has_value())
TRY(m_fs.read_block(block_index.value(), block_buffer)); TRY(m_fs.read_block(block_index.value(), block_buffer));
else else
@@ -252,7 +247,7 @@ namespace Kernel
if (static_cast<BAN::make_unsigned_t<decltype(offset)>>(offset) >= UINT32_MAX || buffer.size() >= UINT32_MAX || buffer.size() >= (size_t)(UINT32_MAX - offset)) if (static_cast<BAN::make_unsigned_t<decltype(offset)>>(offset) >= UINT32_MAX || buffer.size() >= UINT32_MAX || buffer.size() >= (size_t)(UINT32_MAX - offset))
return BAN::Error::from_errno(EOVERFLOW); return BAN::Error::from_errno(EOVERFLOW);
LockGuard _0(m_lock); RWLockWRGuard _0(m_lock);
if (m_inode.size < offset + buffer.size()) if (m_inode.size < offset + buffer.size())
TRY(truncate_impl(offset + buffer.size())); TRY(truncate_impl(offset + buffer.size()));
@@ -269,7 +264,7 @@ namespace Kernel
// Write partial block // Write partial block
if (offset % block_size) if (offset % block_size)
{ {
const auto block_index = TRY(fs_block_of_data_block_index(offset / block_size, true)); const auto block_index = TRY(fs_block_of_data_block_index_no_lock(offset / block_size, true));
ASSERT(block_index.has_value()); ASSERT(block_index.has_value());
TRY(m_fs.read_block(block_index.value(), block_buffer)); TRY(m_fs.read_block(block_index.value(), block_buffer));
@@ -287,7 +282,7 @@ namespace Kernel
while (to_write >= block_size) while (to_write >= block_size)
{ {
const auto block_index = TRY(fs_block_of_data_block_index(offset / block_size, true)); const auto block_index = TRY(fs_block_of_data_block_index_no_lock(offset / block_size, true));
ASSERT(block_index.has_value()); ASSERT(block_index.has_value());
memcpy(block_buffer.data(), buffer.data() + written, block_buffer.size()); memcpy(block_buffer.data(), buffer.data() + written, block_buffer.size());
@@ -300,7 +295,7 @@ namespace Kernel
if (to_write > 0) if (to_write > 0)
{ {
const auto block_index = TRY(fs_block_of_data_block_index(offset / block_size, true)); const auto block_index = TRY(fs_block_of_data_block_index_no_lock(offset / block_size, true));
ASSERT(block_index.has_value()); ASSERT(block_index.has_value());
TRY(m_fs.read_block(block_index.value(), block_buffer)); TRY(m_fs.read_block(block_index.value(), block_buffer));
@@ -314,7 +309,7 @@ namespace Kernel
BAN::ErrorOr<void> Ext2Inode::truncate_impl(size_t new_size) BAN::ErrorOr<void> Ext2Inode::truncate_impl(size_t new_size)
{ {
LockGuard _(m_lock); RWLockWRGuard _(m_lock);
if (m_inode.size == new_size) if (m_inode.size == new_size)
return {}; return {};
@@ -324,7 +319,7 @@ namespace Kernel
const auto old_size = m_inode.size; const auto old_size = m_inode.size;
m_inode.size = new_size; m_inode.size = new_size;
if (auto ret = sync(); ret.is_error()) if (auto ret = sync_no_lock(); ret.is_error())
{ {
m_inode.size = old_size; m_inode.size = old_size;
return ret.release_error(); return ret.release_error();
@@ -337,8 +332,7 @@ namespace Kernel
{ {
ASSERT((mode & Inode::Mode::TYPE_MASK) == 0); ASSERT((mode & Inode::Mode::TYPE_MASK) == 0);
// TODO: this could be atomic RWLockWRGuard _(m_lock);
LockGuard _(m_lock);
if (m_inode.mode == mode) if (m_inode.mode == mode)
return {}; return {};
@@ -346,7 +340,7 @@ namespace Kernel
const auto old_mode = m_inode.mode; const auto old_mode = m_inode.mode;
m_inode.mode = (m_inode.mode & Inode::Mode::TYPE_MASK) | mode; m_inode.mode = (m_inode.mode & Inode::Mode::TYPE_MASK) | mode;
if (auto ret = sync(); ret.is_error()) if (auto ret = sync_no_lock(); ret.is_error())
{ {
m_inode.mode = old_mode; m_inode.mode = old_mode;
return ret.release_error(); return ret.release_error();
@@ -357,8 +351,7 @@ namespace Kernel
BAN::ErrorOr<void> Ext2Inode::chown_impl(uid_t uid, gid_t gid) BAN::ErrorOr<void> Ext2Inode::chown_impl(uid_t uid, gid_t gid)
{ {
// TODO: this could be atomic RWLockWRGuard _(m_lock);
LockGuard _(m_lock);
if (m_inode.uid == uid && m_inode.gid == gid) if (m_inode.uid == uid && m_inode.gid == gid)
return {}; return {};
@@ -368,7 +361,7 @@ namespace Kernel
m_inode.uid = uid; m_inode.uid = uid;
m_inode.gid = gid; m_inode.gid = gid;
if (auto ret = sync(); ret.is_error()) if (auto ret = sync_no_lock(); ret.is_error())
{ {
m_inode.uid = old_uid; m_inode.uid = old_uid;
m_inode.gid = old_gid; m_inode.gid = old_gid;
@@ -380,8 +373,7 @@ namespace Kernel
BAN::ErrorOr<void> Ext2Inode::utimens_impl(const timespec times[2]) BAN::ErrorOr<void> Ext2Inode::utimens_impl(const timespec times[2])
{ {
// TODO: this could be atomic RWLockWRGuard _(m_lock);
LockGuard _(m_lock);
const uint32_t old_times[2] { const uint32_t old_times[2] {
m_inode.atime, m_inode.atime,
@@ -393,7 +385,7 @@ namespace Kernel
if (times[1].tv_nsec != UTIME_OMIT) if (times[1].tv_nsec != UTIME_OMIT)
m_inode.mtime = times[1].tv_sec; m_inode.mtime = times[1].tv_sec;
if (auto ret = sync(); ret.is_error()) if (auto ret = sync_no_lock(); ret.is_error())
{ {
m_inode.atime = old_times[0]; m_inode.atime = old_times[0];
m_inode.mtime = old_times[1]; m_inode.mtime = old_times[1];
@@ -405,19 +397,17 @@ namespace Kernel
BAN::ErrorOr<void> Ext2Inode::fsync_impl() BAN::ErrorOr<void> Ext2Inode::fsync_impl()
{ {
LockGuard _(m_lock); RWLockRDGuard _(m_lock);
for (size_t i = 0; i < max_used_data_block_count(); i++) for (size_t i = 0; i < max_used_data_block_count(); i++)
if (const auto fs_block = TRY(fs_block_of_data_block_index(i, false)); fs_block.has_value()) if (const auto fs_block = TRY(fs_block_of_data_block_index_no_lock(i, false)); fs_block.has_value())
TRY(m_fs.sync_block(fs_block.value())); TRY(m_fs.sync_block(fs_block.value()));
return {}; return {};
} }
BAN::ErrorOr<void> Ext2Inode::cleanup_indirect_block(uint32_t block, uint32_t depth) BAN::ErrorOr<void> Ext2Inode::cleanup_indirect_block_no_lock(uint32_t block, uint32_t depth)
{ {
ASSERT(block); ASSERT(block);
LockGuard _(m_lock);
if (depth == 0) if (depth == 0)
{ {
TRY(m_fs.release_block(block)); TRY(m_fs.release_block(block));
@@ -433,17 +423,15 @@ namespace Kernel
const uint32_t next_block = block_buffer.span().as_span<uint32_t>()[i]; const uint32_t next_block = block_buffer.span().as_span<uint32_t>()[i];
if (next_block == 0) if (next_block == 0)
continue; continue;
TRY(cleanup_indirect_block(next_block, depth - 1)); TRY(cleanup_indirect_block_no_lock(next_block, depth - 1));
} }
TRY(m_fs.release_block(block)); TRY(m_fs.release_block(block));
return {}; return {};
} }
BAN::ErrorOr<void> Ext2Inode::cleanup_data_blocks() BAN::ErrorOr<void> Ext2Inode::cleanup_data_blocks_no_lock()
{ {
LockGuard _(m_lock);
if (mode().iflnk() && (size_t)size() < sizeof(m_inode.block)) if (mode().iflnk() && (size_t)size() < sizeof(m_inode.block))
goto done; goto done;
@@ -454,25 +442,25 @@ namespace Kernel
// cleanup indirect blocks // cleanup indirect blocks
if (m_inode.block[12]) if (m_inode.block[12])
TRY(cleanup_indirect_block(m_inode.block[12], 1)); TRY(cleanup_indirect_block_no_lock(m_inode.block[12], 1));
if (m_inode.block[13]) if (m_inode.block[13])
TRY(cleanup_indirect_block(m_inode.block[13], 2)); TRY(cleanup_indirect_block_no_lock(m_inode.block[13], 2));
if (m_inode.block[14]) if (m_inode.block[14])
TRY(cleanup_indirect_block(m_inode.block[14], 3)); TRY(cleanup_indirect_block_no_lock(m_inode.block[14], 3));
done: done:
// mark blocks as deleted // mark blocks as deleted
memset(m_inode.block, 0x00, sizeof(m_inode.block)); memset(m_inode.block, 0x00, sizeof(m_inode.block));
TRY(sync()); TRY(sync_no_lock());
return {}; return {};
} }
BAN::ErrorOr<void> Ext2Inode::cleanup_from_fs() BAN::ErrorOr<void> Ext2Inode::cleanup_from_fs_no_lock()
{ {
ASSERT(m_inode.links_count == 0); ASSERT(m_inode.links_count == 0);
TRY(cleanup_data_blocks()); TRY(cleanup_data_blocks_no_lock());
TRY(m_fs.delete_inode(ino())); TRY(m_fs.delete_inode(ino()));
return {}; return {};
} }
@@ -482,12 +470,12 @@ done:
ASSERT(mode().ifdir()); ASSERT(mode().ifdir());
ASSERT(offset >= 0); ASSERT(offset >= 0);
LockGuard _(m_lock); RWLockRDGuard _(m_lock);
if (static_cast<BAN::make_unsigned_t<decltype(offset)>>(offset) >= max_used_data_block_count()) if (static_cast<BAN::make_unsigned_t<decltype(offset)>>(offset) >= max_used_data_block_count())
return 0; return 0;
const auto block_index = TRY(fs_block_of_data_block_index(offset, false)); const auto block_index = TRY(fs_block_of_data_block_index_no_lock(offset, false));
if (!block_index.has_value()) if (!block_index.has_value())
return BAN::Error::from_errno(ENODATA); return BAN::Error::from_errno(ENODATA);
@@ -557,9 +545,8 @@ done:
{ {
ASSERT(mode_has_valid_type(mode)); ASSERT(mode_has_valid_type(mode));
timespec current_time = SystemTimer::get().real_time(); const timespec current_time = SystemTimer::get().real_time();
return Ext2::Inode return Ext2::Inode {
{
.mode = (uint16_t)mode, .mode = (uint16_t)mode,
.uid = (uint16_t)uid, .uid = (uint16_t)uid,
.size = 0, .size = 0,
@@ -585,11 +572,6 @@ done:
{ {
ASSERT(this->mode().ifdir()); ASSERT(this->mode().ifdir());
LockGuard _(m_lock);
if (!find_inode_impl(name).is_error())
return BAN::Error::from_errno(EEXIST);
switch (mode & Inode::Mode::TYPE_MASK) switch (mode & Inode::Mode::TYPE_MASK)
{ {
case Inode::Mode::IFLNK: case Inode::Mode::IFLNK:
@@ -600,6 +582,11 @@ done:
return BAN::Error::from_errno(ENOTSUP); return BAN::Error::from_errno(ENOTSUP);
} }
RWLockWRGuard _(m_lock);
if (!find_inode_no_lock(name).is_error())
return BAN::Error::from_errno(EEXIST);
const uint32_t new_ino = TRY(m_fs.create_inode(initialize_new_inode_info(mode, uid, gid))); const uint32_t new_ino = TRY(m_fs.create_inode(initialize_new_inode_info(mode, uid, gid)));
auto inode_or_error = Ext2Inode::create(m_fs, new_ino); auto inode_or_error = Ext2Inode::create(m_fs, new_ino);
@@ -611,7 +598,7 @@ done:
auto inode = inode_or_error.release_value(); auto inode = inode_or_error.release_value();
TRY(link_inode_to_directory(*inode, name)); TRY(link_inode_to_directory_no_lock(*inode, name));
return {}; return {};
} }
@@ -621,9 +608,9 @@ done:
ASSERT(this->mode().ifdir()); ASSERT(this->mode().ifdir());
ASSERT(Mode(mode).ifdir()); ASSERT(Mode(mode).ifdir());
LockGuard _(m_lock); RWLockWRGuard _(m_lock);
if (!find_inode_impl(name).is_error()) if (!find_inode_no_lock(name).is_error())
return BAN::Error::from_errno(EEXIST); return BAN::Error::from_errno(EEXIST);
const uint32_t new_ino = TRY(m_fs.create_inode(initialize_new_inode_info(mode, uid, gid))); const uint32_t new_ino = TRY(m_fs.create_inode(initialize_new_inode_info(mode, uid, gid)));
@@ -638,14 +625,14 @@ done:
auto inode = inode_or_error.release_value(); auto inode = inode_or_error.release_value();
// link . and .. // link . and ..
if (auto ret = inode->link_inode_to_directory(*inode, "."_sv); ret.is_error()) if (auto ret = inode->link_inode_to_directory_no_lock(*inode, "."_sv); ret.is_error())
return ({ TRY(inode->cleanup_from_fs()); ret.release_error(); }); return ({ TRY(inode->cleanup_from_fs_no_lock()); ret.release_error(); });
if (auto ret = inode->link_inode_to_directory(*this, ".."_sv); ret.is_error()) if (auto ret = inode->link_inode_to_directory_no_lock(*this, ".."_sv); ret.is_error())
return ({ TRY(inode->cleanup_from_fs()); ret.release_error(); }); return ({ TRY(inode->cleanup_from_fs_no_lock()); ret.release_error(); });
// link to parent // link to parent
if (auto ret = link_inode_to_directory(*inode, name); ret.is_error()) if (auto ret = link_inode_to_directory_no_lock(*inode, name); ret.is_error())
return ({ TRY(inode->cleanup_from_fs()); ret.release_error(); }); return ({ TRY(inode->cleanup_from_fs_no_lock()); ret.release_error(); });
return {}; return {};
} }
@@ -656,13 +643,13 @@ done:
ASSERT(!inode->mode().ifdir()); ASSERT(!inode->mode().ifdir());
ASSERT(&m_fs == inode->filesystem()); ASSERT(&m_fs == inode->filesystem());
LockGuard _(m_lock); RWLockWRGuard _(m_lock);
if (!find_inode_impl(name).is_error()) if (!find_inode_no_lock(name).is_error())
return BAN::Error::from_errno(EEXIST); return BAN::Error::from_errno(EEXIST);
auto ext2_inode = static_cast<Ext2Inode*>(inode.ptr()); auto ext2_inode = static_cast<Ext2Inode*>(inode.ptr());
TRY(link_inode_to_directory(*ext2_inode, name)); TRY(link_inode_to_directory_no_lock(*ext2_inode, name));
return {}; return {};
} }
@@ -676,30 +663,30 @@ done:
auto* ext2_parent = static_cast<Ext2Inode*>(old_parent.ptr()); auto* ext2_parent = static_cast<Ext2Inode*>(old_parent.ptr());
// FIXME: is this a possible deadlock? // FIXME: is this a possible deadlock?
LockGuard _0(ext2_parent->m_lock); RWLockWRGuard _0(ext2_parent->m_lock);
LockGuard _1(m_lock); RWLockWRGuard _1(m_lock);
auto old_inode = TRY(ext2_parent->find_inode_impl(old_name)); auto old_inode = TRY(ext2_parent->find_inode_no_lock(old_name));
auto* ext2_inode = static_cast<Ext2Inode*>(old_inode.ptr()); auto* ext2_inode = static_cast<Ext2Inode*>(old_inode.ptr());
if (auto find_result = find_inode_impl(new_name); find_result.is_error()) if (auto find_result = find_inode_no_lock(new_name); find_result.is_error())
{ {
if (find_result.error().get_error_code() != ENOENT) if (find_result.error().get_error_code() != ENOENT)
return find_result.release_error(); return find_result.release_error();
} }
else else
{ {
TRY(unlink_impl(new_name)); TRY(remove_inode_from_directory_no_lock(new_name, true));
} }
TRY(link_inode_to_directory(*ext2_inode, new_name)); TRY(link_inode_to_directory_no_lock(*ext2_inode, new_name));
TRY(ext2_parent->remove_inode_from_directory(old_name, false)); TRY(ext2_parent->remove_inode_from_directory_no_lock(old_name, false));
return {}; return {};
} }
BAN::ErrorOr<void> Ext2Inode::link_inode_to_directory(Ext2Inode& inode, BAN::StringView name) BAN::ErrorOr<void> Ext2Inode::link_inode_to_directory_no_lock(Ext2Inode& inode, BAN::StringView name)
{ {
if (!this->mode().ifdir()) if (!this->mode().ifdir())
return BAN::Error::from_errno(ENOTDIR); return BAN::Error::from_errno(ENOTDIR);
@@ -707,15 +694,13 @@ done:
if (name.size() > 255) if (name.size() > 255)
return BAN::Error::from_errno(ENAMETOOLONG); return BAN::Error::from_errno(ENAMETOOLONG);
LockGuard _(m_lock);
if (m_inode.flags & Ext2::Enum::INDEX_FL) if (m_inode.flags & Ext2::Enum::INDEX_FL)
{ {
dwarnln("file creation to indexed directory not supported"); dwarnln("file creation to indexed directory not supported");
return BAN::Error::from_errno(ENOTSUP); return BAN::Error::from_errno(ENOTSUP);
} }
auto error_or = find_inode_impl(name); auto error_or = find_inode_no_lock(name);
if (!error_or.is_error()) if (!error_or.is_error())
return BAN::Error::from_errno(EEXIST); return BAN::Error::from_errno(EEXIST);
if (error_or.error().get_error_code() != ENOENT) if (error_or.error().get_error_code() != ENOENT)
@@ -747,7 +732,7 @@ done:
memcpy(new_entry.name, name.data(), name.size()); memcpy(new_entry.name, name.data(), name.size());
inode.m_inode.links_count++; inode.m_inode.links_count++;
TRY(inode.sync()); TRY(inode.sync_no_lock());
return {}; return {};
}; };
@@ -765,7 +750,7 @@ done:
goto needs_new_block; goto needs_new_block;
// Try to insert inode to last data block // Try to insert inode to last data block
block_index = TRY(fs_block_of_data_block_index(data_block_count - 1, true)).value(); block_index = TRY(fs_block_of_data_block_index_no_lock(data_block_count - 1, true)).value();
TRY(m_fs.read_block(block_index, block_buffer)); TRY(m_fs.read_block(block_index, block_buffer));
while (entry_offset < block_size) while (entry_offset < block_size)
@@ -796,7 +781,7 @@ done:
} }
needs_new_block: needs_new_block:
block_index = TRY(fs_block_of_data_block_index(data_block_count, true)).value(); block_index = TRY(fs_block_of_data_block_index_no_lock(data_block_count, true)).value();
m_inode.size += blksize(); m_inode.size += blksize();
memset(block_buffer.data(), 0x00, block_buffer.size()); memset(block_buffer.data(), 0x00, block_buffer.size());
@@ -806,18 +791,16 @@ needs_new_block:
return {}; return {};
} }
BAN::ErrorOr<bool> Ext2Inode::is_directory_empty() BAN::ErrorOr<bool> Ext2Inode::is_directory_empty_no_lock()
{ {
ASSERT(mode().ifdir()); ASSERT(mode().ifdir());
auto block_buffer = TRY(m_fs.get_block_buffer()); auto block_buffer = TRY(m_fs.get_block_buffer());
LockGuard _(m_lock);
// Confirm that this doesn't contain anything else than '.' or '..' // Confirm that this doesn't contain anything else than '.' or '..'
for (uint32_t i = 0; i < max_used_data_block_count(); i++) for (uint32_t i = 0; i < max_used_data_block_count(); i++)
{ {
const auto block_index = TRY(fs_block_of_data_block_index(i, false)); const auto block_index = TRY(fs_block_of_data_block_index_no_lock(i, false));
if (!block_index.has_value()) if (!block_index.has_value())
continue; continue;
TRY(m_fs.read_block(block_index.value(), block_buffer)); TRY(m_fs.read_block(block_index.value(), block_buffer));
@@ -841,14 +824,12 @@ needs_new_block:
return true; return true;
} }
BAN::ErrorOr<void> Ext2Inode::cleanup_default_links() BAN::ErrorOr<void> Ext2Inode::cleanup_default_links_no_lock()
{ {
ASSERT(mode().ifdir()); ASSERT(mode().ifdir());
auto block_buffer = TRY(m_fs.get_block_buffer()); auto block_buffer = TRY(m_fs.get_block_buffer());
LockGuard _(m_lock);
if (m_inode.flags & Ext2::Enum::INDEX_FL) if (m_inode.flags & Ext2::Enum::INDEX_FL)
{ {
dwarnln("deletion of indexed directory is not supported"); dwarnln("deletion of indexed directory is not supported");
@@ -857,7 +838,7 @@ needs_new_block:
for (uint32_t i = 0; i < max_used_data_block_count(); i++) for (uint32_t i = 0; i < max_used_data_block_count(); i++)
{ {
const auto block_index = TRY(fs_block_of_data_block_index(i, false)); const auto block_index = TRY(fs_block_of_data_block_index_no_lock(i, false));
if (!block_index.has_value()) if (!block_index.has_value())
continue; continue;
TRY(m_fs.read_block(block_index.value(), block_buffer)); TRY(m_fs.read_block(block_index.value(), block_buffer));
@@ -876,13 +857,13 @@ needs_new_block:
if (entry_name == "."_sv) if (entry_name == "."_sv)
{ {
m_inode.links_count--; m_inode.links_count--;
TRY(sync()); TRY(sync_no_lock());
} }
else if (entry_name == ".."_sv) else if (entry_name == ".."_sv)
{ {
auto parent = TRY(Ext2Inode::create(m_fs, entry.inode)); auto parent = TRY(Ext2Inode::create(m_fs, entry.inode));
parent->m_inode.links_count--; parent->m_inode.links_count--;
TRY(parent->sync()); TRY(parent->sync_no_lock());
} }
else else
ASSERT_NOT_REACHED(); ASSERT_NOT_REACHED();
@@ -901,14 +882,12 @@ needs_new_block:
return {}; return {};
} }
BAN::ErrorOr<void> Ext2Inode::remove_inode_from_directory(BAN::StringView name, bool cleanup_directory) BAN::ErrorOr<void> Ext2Inode::remove_inode_from_directory_no_lock(BAN::StringView name, bool cleanup_directory)
{ {
ASSERT(mode().ifdir()); ASSERT(mode().ifdir());
auto block_buffer = TRY(m_fs.get_block_buffer()); auto block_buffer = TRY(m_fs.get_block_buffer());
LockGuard _(m_lock);
if (m_inode.flags & Ext2::Enum::INDEX_FL) if (m_inode.flags & Ext2::Enum::INDEX_FL)
{ {
dwarnln("deletion from indexed directory is not supported"); dwarnln("deletion from indexed directory is not supported");
@@ -917,7 +896,7 @@ needs_new_block:
for (uint32_t i = 0; i < max_used_data_block_count(); i++) for (uint32_t i = 0; i < max_used_data_block_count(); i++)
{ {
const auto block_index = TRY(fs_block_of_data_block_index(i, false)); const auto block_index = TRY(fs_block_of_data_block_index_no_lock(i, false));
if (!block_index.has_value()) if (!block_index.has_value())
continue; continue;
TRY(m_fs.read_block(block_index.value(), block_buffer)); TRY(m_fs.read_block(block_index.value(), block_buffer));
@@ -931,9 +910,9 @@ needs_new_block:
auto inode = TRY(Ext2Inode::create(m_fs, entry.inode)); auto inode = TRY(Ext2Inode::create(m_fs, entry.inode));
if (cleanup_directory && inode->mode().ifdir()) if (cleanup_directory && inode->mode().ifdir())
{ {
if (!TRY(inode->is_directory_empty())) if (!TRY(inode->is_directory_empty_no_lock()))
return BAN::Error::from_errno(ENOTEMPTY); return BAN::Error::from_errno(ENOTEMPTY);
TRY(inode->cleanup_default_links()); TRY(inode->cleanup_default_links_no_lock());
} }
if (inode->nlink() == 0) if (inode->nlink() == 0)
@@ -941,7 +920,7 @@ needs_new_block:
else else
inode->m_inode.links_count--; inode->m_inode.links_count--;
TRY(sync()); TRY(sync_no_lock());
// NOTE: If this was the last link to inode we must // NOTE: If this was the last link to inode we must
// remove it from inode cache to trigger cleanup // remove it from inode cache to trigger cleanup
@@ -965,18 +944,17 @@ needs_new_block:
BAN::ErrorOr<void> Ext2Inode::unlink_impl(BAN::StringView name) BAN::ErrorOr<void> Ext2Inode::unlink_impl(BAN::StringView name)
{ {
TRY(remove_inode_from_directory(name, true)); RWLockWRGuard _(m_lock);
TRY(remove_inode_from_directory_no_lock(name, true));
return {}; return {};
} }
BAN::ErrorOr<void> Ext2Inode::sync() BAN::ErrorOr<void> Ext2Inode::sync_no_lock()
{ {
auto inode_location = TRY(m_fs.locate_inode(ino())); auto inode_location = TRY(m_fs.locate_inode(ino()));
auto block_buffer = TRY(m_fs.get_block_buffer()); auto block_buffer = TRY(m_fs.get_block_buffer());
TRY(m_fs.read_block(inode_location.block, block_buffer)); TRY(m_fs.read_block(inode_location.block, block_buffer));
LockGuard _(m_lock);
if (memcmp(block_buffer.data() + inode_location.offset, &m_inode, sizeof(Ext2::Inode))) if (memcmp(block_buffer.data() + inode_location.offset, &m_inode, sizeof(Ext2::Inode)))
{ {
memcpy(block_buffer.data() + inode_location.offset, &m_inode, sizeof(Ext2::Inode)); memcpy(block_buffer.data() + inode_location.offset, &m_inode, sizeof(Ext2::Inode));
@@ -987,15 +965,20 @@ needs_new_block:
} }
BAN::ErrorOr<BAN::RefPtr<Inode>> Ext2Inode::find_inode_impl(BAN::StringView file_name) BAN::ErrorOr<BAN::RefPtr<Inode>> Ext2Inode::find_inode_impl(BAN::StringView file_name)
{
RWLockRDGuard _(m_lock);
return find_inode_no_lock(file_name);
}
BAN::ErrorOr<BAN::RefPtr<Inode>> Ext2Inode::find_inode_no_lock(BAN::StringView file_name)
{ {
ASSERT(mode().ifdir()); ASSERT(mode().ifdir());
auto block_buffer = TRY(m_fs.get_block_buffer()); auto block_buffer = TRY(m_fs.get_block_buffer());
LockGuard _(m_lock);
for (uint32_t i = 0; i < max_used_data_block_count(); i++) for (uint32_t i = 0; i < max_used_data_block_count(); i++)
{ {
const auto block_index = TRY(fs_block_of_data_block_index(i, false)); const auto block_index = TRY(fs_block_of_data_block_index_no_lock(i, false));
if (!block_index.has_value()) if (!block_index.has_value())
continue; continue;
TRY(m_fs.read_block(block_index.value(), block_buffer)); TRY(m_fs.read_block(block_index.value(), block_buffer));