From 552dd7abd39ae79b58a420f926cea58af9e08c22 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 7 Aug 2021 23:50:37 +0200 Subject: [PATCH] Kernel: Port BlockBasedFileSystem to ProtectedValue :^) --- Kernel/FileSystem/BlockBasedFileSystem.cpp | 156 ++++++++++----------- Kernel/FileSystem/BlockBasedFileSystem.h | 4 +- 2 files changed, 80 insertions(+), 80 deletions(-) diff --git a/Kernel/FileSystem/BlockBasedFileSystem.cpp b/Kernel/FileSystem/BlockBasedFileSystem.cpp index 0a0c29c6a19..c417c24c64d 100644 --- a/Kernel/FileSystem/BlockBasedFileSystem.cpp +++ b/Kernel/FileSystem/BlockBasedFileSystem.cpp @@ -129,7 +129,10 @@ bool BlockBasedFileSystem::initialize() if (!disk_cache) return false; - m_cache = move(disk_cache); + m_cache.with_exclusive([&](auto& cache) { + cache = move(disk_cache); + }); + return true; } @@ -139,31 +142,31 @@ KResult BlockBasedFileSystem::write_block(BlockIndex index, const UserOrKernelBu VERIFY(offset + count <= block_size()); dbgln_if(BBFS_DEBUG, "BlockBasedFileSystem::write_block {}, size={}", index, count); - MutexLocker locker(m_cache_lock); + return m_cache.with_exclusive([&](auto& cache) -> KResult { + if (!allow_cache) { + flush_specific_block_if_needed(index); + auto base_offset = index.value() * block_size() + offset; + auto nwritten = file_description().write(base_offset, data, count); + if (nwritten.is_error()) + return nwritten.error(); + VERIFY(nwritten.value() == count); + return KSuccess; + } - if (!allow_cache) { - flush_specific_block_if_needed(index); - auto base_offset = index.value() * block_size() + offset; - auto nwritten = file_description().write(base_offset, data, count); - if (nwritten.is_error()) - return nwritten.error(); - VERIFY(nwritten.value() == count); + auto& entry = cache->get(index); + if (count < block_size()) { + // Fill the cache first. + auto result = read_block(index, nullptr, block_size()); + if (result.is_error()) + return result; + } + if (!data.read(entry.data + offset, count)) + return EFAULT; + + cache->mark_dirty(entry); + entry.has_data = true; return KSuccess; - } - - auto& entry = cache().get(index); - if (count < block_size()) { - // Fill the cache first. - auto result = read_block(index, nullptr, block_size()); - if (result.is_error()) - return result; - } - if (!data.read(entry.data + offset, count)) - return EFAULT; - - cache().mark_dirty(entry); - entry.has_data = true; - return KSuccess; + }); } bool BlockBasedFileSystem::raw_read(BlockIndex index, UserOrKernelBuffer& buffer) @@ -224,31 +227,31 @@ KResult BlockBasedFileSystem::read_block(BlockIndex index, UserOrKernelBuffer* b VERIFY(offset + count <= block_size()); dbgln_if(BBFS_DEBUG, "BlockBasedFileSystem::read_block {}", index); - MutexLocker locker(m_cache_lock); + return m_cache.with_exclusive([&](auto& cache) -> KResult { + if (!allow_cache) { + const_cast(this)->flush_specific_block_if_needed(index); + auto base_offset = index.value() * block_size() + offset; + auto nread = file_description().read(*buffer, base_offset, count); + if (nread.is_error()) + return nread.error(); + VERIFY(nread.value() == count); + return KSuccess; + } - if (!allow_cache) { - const_cast(this)->flush_specific_block_if_needed(index); - auto base_offset = index.value() * block_size() + offset; - auto nread = file_description().read(*buffer, base_offset, count); - if (nread.is_error()) - return nread.error(); - VERIFY(nread.value() == count); + auto& entry = cache->get(index); + if (!entry.has_data) { + auto base_offset = index.value() * block_size(); + auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data); + auto nread = file_description().read(entry_data_buffer, base_offset, block_size()); + if (nread.is_error()) + return nread.error(); + VERIFY(nread.value() == block_size()); + entry.has_data = true; + } + if (buffer && !buffer->write(entry.data + offset, count)) + return EFAULT; return KSuccess; - } - - auto& entry = cache().get(index); - if (!entry.has_data) { - auto base_offset = index.value() * block_size(); - auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data); - auto nread = file_description().read(entry_data_buffer, base_offset, block_size()); - if (nread.is_error()) - return nread.error(); - VERIFY(nread.value() == block_size()); - entry.has_data = true; - } - if (buffer && !buffer->write(entry.data + offset, count)) - return EFAULT; - return KSuccess; + }); } KResult BlockBasedFileSystem::read_blocks(BlockIndex index, unsigned count, UserOrKernelBuffer& buffer, bool allow_cache) const @@ -271,38 +274,40 @@ KResult BlockBasedFileSystem::read_blocks(BlockIndex index, unsigned count, User void BlockBasedFileSystem::flush_specific_block_if_needed(BlockIndex index) { - MutexLocker locker(m_cache_lock); - if (!cache().is_dirty()) - return; - Vector cleaned_entries; - cache().for_each_dirty_entry([&](CacheEntry& entry) { - if (entry.block_index != index) { - size_t base_offset = entry.block_index.value() * block_size(); - auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data); - [[maybe_unused]] auto rc = file_description().write(base_offset, entry_data_buffer, block_size()); - cleaned_entries.append(&entry); - } + m_cache.with_exclusive([&](auto& cache) { + if (!cache->is_dirty()) + return; + Vector cleaned_entries; + cache->for_each_dirty_entry([&](CacheEntry& entry) { + if (entry.block_index != index) { + size_t base_offset = entry.block_index.value() * block_size(); + auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data); + [[maybe_unused]] auto rc = file_description().write(base_offset, entry_data_buffer, block_size()); + cleaned_entries.append(&entry); + } + }); + // NOTE: We make a separate pass to mark entries clean since marking them clean + // moves them out of the dirty list which would disturb the iteration above. + for (auto* entry : cleaned_entries) + cache->mark_clean(*entry); }); - // NOTE: We make a separate pass to mark entries clean since marking them clean - // moves them out of the dirty list which would disturb the iteration above. - for (auto* entry : cleaned_entries) - cache().mark_clean(*entry); } void BlockBasedFileSystem::flush_writes_impl() { - MutexLocker locker(m_cache_lock); - if (!cache().is_dirty()) - return; - u32 count = 0; - cache().for_each_dirty_entry([&](CacheEntry& entry) { - auto base_offset = entry.block_index.value() * block_size(); - auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data); - [[maybe_unused]] auto rc = file_description().write(base_offset, entry_data_buffer, block_size()); - ++count; + size_t count = 0; + m_cache.with_exclusive([&](auto& cache) { + if (!cache->is_dirty()) + return; + cache->for_each_dirty_entry([&](CacheEntry& entry) { + auto base_offset = entry.block_index.value() * block_size(); + auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data); + [[maybe_unused]] auto rc = file_description().write(base_offset, entry_data_buffer, block_size()); + ++count; + }); + cache->mark_all_clean(); + dbgln("{}: Flushed {} blocks to disk", class_name(), count); }); - cache().mark_all_clean(); - dbgln("{}: Flushed {} blocks to disk", class_name(), count); } void BlockBasedFileSystem::flush_writes() @@ -310,9 +315,4 @@ void BlockBasedFileSystem::flush_writes() flush_writes_impl(); } -DiskCache& BlockBasedFileSystem::cache() const -{ - return *m_cache; -} - } diff --git a/Kernel/FileSystem/BlockBasedFileSystem.h b/Kernel/FileSystem/BlockBasedFileSystem.h index 0e78e9ef10d..ccb0ec4a0be 100644 --- a/Kernel/FileSystem/BlockBasedFileSystem.h +++ b/Kernel/FileSystem/BlockBasedFileSystem.h @@ -7,6 +7,7 @@ #pragma once #include +#include namespace Kernel { @@ -43,8 +44,7 @@ private: DiskCache& cache() const; void flush_specific_block_if_needed(BlockIndex index); - mutable Mutex m_cache_lock; - mutable OwnPtr m_cache; + mutable ProtectedValue> m_cache; }; }