From e5eab8bae45d8494730ae6751cc3a18bb8e4e2a9 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Thu, 23 Mar 2023 23:22:31 +0200 Subject: [PATCH] Kernel: Ext2FS now does allocations better We only have to allocate at the beginning of the all functions and can properly exit before any disk reads if we run out of memory. This makes development little bit 'harder' since the {read,write}_block user must allocate a buffer of atleast block_size bytes. I also made disk access to cause kernel panic on error since the error handling during file write is something I don't want to think now. The filesystem can easily corrupt so, I feel like when disk io starts to fail I'll come back to this. --- kernel/include/kernel/FS/Ext2.h | 4 +- kernel/kernel/FS/Ext2.cpp | 144 +++++++++++++++++++------------- 2 files changed, 86 insertions(+), 62 deletions(-) diff --git a/kernel/include/kernel/FS/Ext2.h b/kernel/include/kernel/FS/Ext2.h index 9eac447489..44b54990fb 100644 --- a/kernel/include/kernel/FS/Ext2.h +++ b/kernel/include/kernel/FS/Ext2.h @@ -185,8 +185,8 @@ namespace Kernel BAN::ErrorOr delete_inode(uint32_t); BAN::ErrorOr resize_inode(uint32_t, size_t); - BAN::ErrorOr> read_block(uint32_t); - BAN::ErrorOr write_block(uint32_t, BAN::Span); + void read_block(uint32_t, BAN::Span); + void write_block(uint32_t, BAN::Span); const Ext2::Superblock& superblock() const { return m_superblock; } diff --git a/kernel/kernel/FS/Ext2.cpp b/kernel/kernel/FS/Ext2.cpp index 3be945ed17..e7d8b2306f 100644 --- a/kernel/kernel/FS/Ext2.cpp +++ b/kernel/kernel/FS/Ext2.cpp @@ -152,9 +152,12 @@ namespace Kernel BAN::ErrorOr> Ext2Inode::create(Ext2FS& fs, uint32_t inode_inode, BAN::StringView name) { + BAN::Vector block_buffer; + TRY(block_buffer.resize(fs.block_size())); + auto inode_location = TRY(fs.locate_inode(inode_inode)); - auto inode_buffer = TRY(fs.read_block(inode_location.block)); - auto& inode = *(Ext2::Inode*)(inode_buffer.data() + inode_location.offset); + fs.read_block(inode_location.block, block_buffer.span()); + auto& inode = *(Ext2::Inode*)(block_buffer.data() + inode_location.offset); Ext2Inode* result = new Ext2Inode(fs, inode, name, inode_inode); if (result == nullptr) return BAN::Error::from_errno(ENOMEM); @@ -166,8 +169,7 @@ namespace Kernel uint32_t data_blocks_count = m_inode.blocks / (2 << m_fs.superblock().log_block_size); uint32_t blocks_per_array = (1024 << m_fs.superblock().log_block_size) / sizeof(uint32_t); - if (asked_data_block >= data_blocks_count) - return BAN::Error::from_c_string("Ext2: no such block"); + ASSERT(asked_data_block < data_blocks_count); // Direct block if (asked_data_block < 12) @@ -180,13 +182,17 @@ namespace Kernel asked_data_block -= 12; + uint32_t block_size = m_fs.block_size(); + BAN::Vector block_buffer; + TRY(block_buffer.resize(block_size)); + // Singly indirect block if (asked_data_block < blocks_per_array) { if (m_inode.block[12] == 0) return BAN::Error::from_errno(EIO); - auto block_array = TRY(m_fs.read_block(m_inode.block[12])); - uint32_t block = ((uint32_t*)block_array.data())[asked_data_block]; + m_fs.read_block(m_inode.block[12], block_buffer.span()); // Block array + uint32_t block = ((uint32_t*)block_buffer.data())[asked_data_block]; if (block == 0) return BAN::Error::from_errno(EIO); return block; @@ -197,12 +203,12 @@ namespace Kernel // Doubly indirect blocks if (asked_data_block < blocks_per_array * blocks_per_array) { - auto singly_indirect_array = TRY(m_fs.read_block(m_inode.block[13])); - uint32_t direct_block = ((uint32_t*)singly_indirect_array.data())[asked_data_block / blocks_per_array]; + m_fs.read_block(m_inode.block[13], block_buffer.span()); // Singly indirect array + uint32_t direct_block = ((uint32_t*)block_buffer.data())[asked_data_block / blocks_per_array]; if (direct_block == 0) return BAN::Error::from_errno(EIO); - auto block_array = TRY(m_fs.read_block(direct_block)); - uint32_t block = ((uint32_t*)block_array.data())[asked_data_block % blocks_per_array]; + m_fs.read_block(direct_block, block_buffer.span()); // Block array + uint32_t block = ((uint32_t*)block_buffer.data())[asked_data_block % blocks_per_array]; if (block == 0) return BAN::Error::from_errno(EIO); return block; @@ -213,16 +219,16 @@ namespace Kernel // Triply indirect blocks if (asked_data_block < blocks_per_array * blocks_per_array * blocks_per_array) { - auto doubly_indirect_array = TRY(m_fs.read_block(m_inode.block[14])); - uint32_t singly_indirect_block = ((uint32_t*)doubly_indirect_array.data())[asked_data_block / (blocks_per_array * blocks_per_array)]; + m_fs.read_block(m_inode.block[14], block_buffer.span()); // Doubly indirect array + uint32_t singly_indirect_block = ((uint32_t*)block_buffer.data())[asked_data_block / (blocks_per_array * blocks_per_array)]; if (singly_indirect_block == 0) return BAN::Error::from_errno(EIO); - auto singly_indirect_array = TRY(m_fs.read_block(singly_indirect_block)); - uint32_t direct_block = ((uint32_t*)singly_indirect_array.data())[(asked_data_block / blocks_per_array) % blocks_per_array]; + m_fs.read_block(singly_indirect_block, block_buffer.span()); // Singly indirect array + uint32_t direct_block = ((uint32_t*)block_buffer.data())[(asked_data_block / blocks_per_array) % blocks_per_array]; if (direct_block == 0) return BAN::Error::from_errno(EIO); - auto block_array = TRY(m_fs.read_block(direct_block)); - uint32_t block = ((uint32_t*)block_array.data())[asked_data_block % blocks_per_array]; + m_fs.read_block(direct_block, block_buffer.span()); // Block array + uint32_t block = ((uint32_t*)block_buffer.data())[asked_data_block % blocks_per_array]; if (block == 0) return BAN::Error::from_errno(EIO); return block; @@ -243,7 +249,9 @@ namespace Kernel if (offset + count > m_inode.size) count = m_inode.size - offset; - const uint32_t block_size = 1024 << m_fs.superblock().log_block_size; + uint32_t block_size = 1024 << m_fs.superblock().log_block_size; + BAN::Vector block_buffer; + TRY(block_buffer.resize(block_size)); const uint32_t first_block = offset / block_size; const uint32_t last_block = BAN::Math::div_round_up(offset + count, block_size); @@ -253,12 +261,11 @@ namespace Kernel for (uint32_t block = first_block; block < last_block; block++) { uint32_t block_index = TRY(data_block_index(block)); - auto block_data = TRY(m_fs.read_block(block_index)); - ASSERT(block_data.size() == block_size); + m_fs.read_block(block_index, block_buffer.span()); uint32_t copy_offset = (offset + n_read) % block_size; uint32_t to_copy = BAN::Math::min(block_size - copy_offset, count - n_read); - memcpy((uint8_t*)buffer + n_read, block_data.data() + copy_offset, to_copy); + memcpy((uint8_t*)buffer + n_read, block_buffer.data() + copy_offset, to_copy); n_read += to_copy; } @@ -274,6 +281,10 @@ namespace Kernel if (name.size() > 255) return BAN::Error::from_errno(ENAMETOOLONG); + uint32_t block_size = m_fs.block_size(); + BAN::Vector block_buffer; + TRY(block_buffer.resize(block_size)); + auto error_or = directory_find_impl(name); if (!error_or.is_error()) return BAN::Error::from_errno(EEXISTS); @@ -307,15 +318,15 @@ namespace Kernel // Insert inode to this directory uint32_t data_block_count = m_inode.blocks / (2 << m_fs.superblock().log_block_size); uint32_t block_index = TRY(data_block_index(data_block_count - 1)); - auto block_data = TRY(m_fs.read_block(block_index)); + m_fs.read_block(block_index, block_buffer.span()); - const uint8_t* block_data_end = block_data.data() + block_data.size(); - const uint8_t* entry_addr = block_data.data(); + const uint8_t* block_buffer_end = block_buffer.data() + block_size; + const uint8_t* entry_addr = block_buffer.data(); uint32_t needed_entry_len = sizeof(Ext2::LinkedDirectoryEntry) + name.size(); bool insered = false; - while (entry_addr < block_data_end) + while (entry_addr < block_buffer_end) { auto& entry = *(Ext2::LinkedDirectoryEntry*)entry_addr; @@ -327,12 +338,12 @@ namespace Kernel auto& new_entry = *(Ext2::LinkedDirectoryEntry*)(entry_addr + entry.rec_len); new_entry.inode = inode_index; - new_entry.rec_len = block_data_end - (uint8_t*)&new_entry; + new_entry.rec_len = block_buffer_end - (uint8_t*)&new_entry; new_entry.name_len = name.size(); new_entry.file_type = Ext2::Enum::REG_FILE; memcpy(new_entry.name, name.data(), name.size()); - TRY(m_fs.write_block(block_index, block_data.span())); + m_fs.write_block(block_index, block_buffer.span()); insered = true; break; @@ -352,17 +363,21 @@ namespace Kernel if (!ifdir()) return BAN::Error::from_errno(ENOTDIR); + uint32_t block_size = m_fs.block_size(); + BAN::Vector block_buffer; + TRY(block_buffer.resize(block_size)); + uint32_t data_block_count = m_inode.blocks / (2 << m_fs.superblock().log_block_size); for (uint32_t i = 0; i < data_block_count; i++) { uint32_t block_index = TRY(data_block_index(i)); - auto block_data = TRY(m_fs.read_block(block_index)); + m_fs.read_block(block_index, block_buffer.span()); - const uint8_t* block_data_end = block_data.data() + block_data.size(); - const uint8_t* entry_addr = block_data.data(); + const uint8_t* block_buffer_end = block_buffer.data() + block_size; + const uint8_t* entry_addr = block_buffer.data(); - while (entry_addr < block_data_end) + while (entry_addr < block_buffer_end) { const auto& entry = *(const Ext2::LinkedDirectoryEntry*)entry_addr; BAN::StringView entry_name(entry.name, entry.name_len); @@ -380,6 +395,10 @@ namespace Kernel if (!ifdir()) return BAN::Error::from_errno(ENOTDIR); + uint32_t block_size = m_fs.block_size(); + BAN::Vector block_buffer; + TRY(block_buffer.resize(block_size)); + uint32_t data_block_count = m_inode.blocks / (2 << m_fs.superblock().log_block_size); BAN::Vector> inodes; @@ -387,11 +406,11 @@ namespace Kernel for (uint32_t i = 0; i < data_block_count; i++) { uint32_t block_index = TRY(data_block_index(i)); - auto block_data = TRY(m_fs.read_block(block_index)); + m_fs.read_block(block_index, block_buffer.span()); - const uint8_t* block_data_end = block_data.data() + block_data.size(); - const uint8_t* entry_addr = block_data.data(); - while (entry_addr < block_data_end) + const uint8_t* block_buffer_end = block_buffer.data() + block_size; + const uint8_t* entry_addr = block_buffer.data(); + while (entry_addr < block_buffer_end) { const auto& entry = *(const Ext2::LinkedDirectoryEntry*)entry_addr; if (entry.inode) @@ -496,17 +515,23 @@ namespace Kernel { ASSERT(ext2_inode.size == 0); + uint32_t block_size = this->block_size(); + BAN::Vector bgd_buffer; + TRY(bgd_buffer.resize(block_size)); + BAN::Vector inode_bitmap; + TRY(inode_bitmap.resize(block_size)); + uint32_t number_of_block_groups = BAN::Math::div_round_up(superblock().inodes_count, superblock().inodes_per_group); for (uint32_t group = 0; group < number_of_block_groups; group++) { - auto bgd_location = this->locate_block_group_descriptior(group); - auto bgd_buffer = TRY(this->read_block(bgd_location.block)); + auto bgd_location = locate_block_group_descriptior(group); + read_block(bgd_location.block, bgd_buffer.span()); auto& bgd = *(Ext2::BlockGroupDescriptor*)(bgd_buffer.data() + bgd_location.offset); if (bgd.free_inodes_count == 0) continue; - auto inode_bitmap = TRY(read_block(bgd.inode_bitmap)); + read_block(bgd.inode_bitmap, inode_bitmap.span()); for (uint32_t inode_offset = 0; inode_offset < superblock().inodes_per_group; inode_offset++) { uint32_t byte = inode_offset / 8; @@ -514,10 +539,10 @@ namespace Kernel if ((inode_bitmap[byte] & (1 << bit)) == 0) { inode_bitmap[byte] |= (1 << bit); - TRY(write_block(bgd.inode_bitmap, inode_bitmap.span())); + write_block(bgd.inode_bitmap, inode_bitmap.span()); bgd.free_inodes_count--; - TRY(write_block(bgd_location.block, bgd_buffer.span())); + write_block(bgd_location.block, bgd_buffer.span()); return group * superblock().inodes_per_group + inode_offset + 1; } @@ -527,37 +552,34 @@ namespace Kernel return BAN::Error::from_c_string("No free inodes available in the whole filesystem"); } - BAN::ErrorOr> Ext2FS::read_block(uint32_t block) + void Ext2FS::read_block(uint32_t block, BAN::Span buffer) { uint32_t sector_size = m_partition.device().sector_size(); uint32_t block_size = this->block_size(); uint32_t sectors_per_block = block_size / sector_size; - - BAN::Vector block_buffer; - TRY(block_buffer.resize(block_size)); - TRY(m_partition.read_sectors(block * sectors_per_block, sectors_per_block, block_buffer.data())); - return block_buffer; + ASSERT(buffer.size() >= block_size); + + MUST(m_partition.read_sectors(block * sectors_per_block, sectors_per_block, buffer.data())); } - BAN::ErrorOr Ext2FS::write_block(uint32_t block, BAN::Span data) + void Ext2FS::write_block(uint32_t block, BAN::Span data) { uint32_t sector_size = m_partition.device().sector_size(); uint32_t block_size = this->block_size(); uint32_t sectors_per_block = block_size / sector_size; ASSERT(data.size() <= block_size); - TRY(m_partition.write_sectors(block * sectors_per_block, sectors_per_block, data.data())); - - return {}; + MUST(m_partition.write_sectors(block * sectors_per_block, sectors_per_block, data.data())); } BAN::ErrorOr Ext2FS::locate_inode(uint32_t inode_index) { - if (inode_index >= superblock().inodes_count) - return BAN::Error::from_format("Asked to read inode {}, but only {} exist in the filesystem", inode_index, superblock().inodes_count); + ASSERT(inode_index < superblock().inodes_count); uint32_t block_size = this->block_size(); + BAN::Vector bgd_buffer; + TRY(bgd_buffer.resize(block_size)); uint32_t inode_block_group = (inode_index - 1) / superblock().inodes_per_group; uint32_t local_inode_index = (inode_index - 1) % superblock().inodes_per_group; @@ -565,20 +587,22 @@ namespace Kernel uint32_t inode_table_byte_offset = (local_inode_index * superblock().inode_size); auto bgd_location = locate_block_group_descriptior(inode_block_group); - auto bgd_buffer = TRY(read_block(bgd_location.block)); + read_block(bgd_location.block, bgd_buffer.span()); auto& bgd = *(Ext2::BlockGroupDescriptor*)(bgd_buffer.data() + bgd_location.offset); -#if VERIFY_INODE_EXISTANCE - ASSERT(superblock().inodes_per_group <= block_size * 8); - auto inode_bitmap = TRY(read_block(bgd.inode_bitmap)); - uint32_t byte = local_inode_index / 8; - uint32_t bit = local_inode_index % 8; - ASSERT(inode_bitmap[byte] & (1 << bit)); -#endif - BlockLocation location; location.block = bgd.inode_table + inode_table_byte_offset / block_size; location.offset = inode_table_byte_offset % block_size; + +#if VERIFY_INODE_EXISTANCE + // Note we reuse the bgd_buffer since it is not needed anymore + ASSERT(superblock().inodes_per_group <= block_size * 8); + read_block(bgd.inode_bitmap, bgd_buffer.span()); + uint32_t byte = local_inode_index / 8; + uint32_t bit = local_inode_index % 8; + ASSERT(bgd_buffer[byte] & (1 << bit)); +#endif + return location; }