kernel/fs: ext2: introduce write batching

6 files changed, 96 insertions, 95 deletions

diff --git a/kernel/kernel/filesystem/ext2/filesystem.cpp b/kernel/kernel/filesystem/ext2/filesystem.cpp
index 7031cf5f..3609f510 100644
--- a/kernel/kernel/filesystem/ext2/filesystem.cpp
+++ b/kernel/kernel/filesystem/ext2/filesystem.cpp
@@ -8,6 +8,7 @@
 #include <kernel/filesystem/ext2/linked_directory_entry.hpp>
 #include <kernel/filesystem/ext2/mount_state.hpp>
 #include <kernel/filesystem/ext2/superblock.hpp>
+#include <kernel/filesystem/ext2/write_batch.hpp>
 #include <kernel/filesystem/filesystem.hpp>
 #include <kernel/filesystem/inode.hpp>
 
@@ -252,7 +253,9 @@ namespace kernel::filesystem::ext2
       return kstd::failure(vfs_errc::invalid_driver_data);
     }
 
-    auto maybe_inode_number = allocate_inode(*mount_state);
+    auto batch = write_batch{*this, *mount_state};
+
+    auto maybe_inode_number = allocate_inode(*mount_state, batch);
     if (!maybe_inode_number)
     {
       return kstd::failure(maybe_inode_number.error());
@@ -280,7 +283,7 @@ namespace kernel::filesystem::ext2
     created->set_owning_mount(ext2_parent->owning_mount().lock());
 
     // TODO: preallocate blocks depending on type and m_superblock.prealloc_dir_blocks or m_superblock.prealloc_blocks;
-    if (auto result = add_directory_entry(*ext2_parent, name, *created, data.mode, *mount_state); !result)
+    if (auto result = add_directory_entry(*ext2_parent, name, *created, data.mode, *mount_state, batch); !result)
     {
       return kstd::failure(result.error());
     }
@@ -288,17 +291,14 @@ namespace kernel::filesystem::ext2
     if (created->is_directory())
     {
       // TODO increment used_dirs_count in block group descriptor
-      if (auto result = init_directory(*created, *ext2_parent, *mount_state); !result)
+      if (auto result = init_directory(*created, *ext2_parent, *mount_state, batch); !result)
       {
         return kstd::failure(result.error());
       }
       created->set_size(block_size(*mount_state));
     }
 
-    if (auto result = write_inode(inode_number, created->data(), *mount_state); !result)
-    {
-      return kstd::failure(result.error());
-    }
+    batch.mark_inode(*created);
 
     return created;
   }
@@ -348,7 +348,7 @@ namespace kernel::filesystem::ext2
   }
 
   auto filesystem::add_directory_entry(inode & directory, std::string_view name, inode & child, std::uint16_t mode,
-                                       mount_state & state) -> kstd::result<void>
+                                       mount_state & state, write_batch & batch) -> kstd::result<void>
   {
     auto const last_block_index = block_count(directory, state) - 1;
     auto const global_block = inode_block_number(last_block_index, directory, state);
@@ -407,7 +407,7 @@ namespace kernel::filesystem::ext2
     }
     else
     {
-      if (!directory.append_blocks(1))
+      if (!directory.append_blocks(1, batch))
       {
         return kstd::failure(ext2_errc::not_enough_free_blocks);
       }
@@ -439,12 +439,14 @@ namespace kernel::filesystem::ext2
 
     auto size = block_count(directory, state) * block_size(state);
     directory.data().size = size.value;
-    return write_inode(directory.number(), directory.data(), state);
+    batch.mark_inode(directory);
+    return kstd::success();
   }
 
-  auto filesystem::init_directory(inode & directory, inode & parent, mount_state & state) -> kstd::result<void>
+  auto filesystem::init_directory(inode & directory, inode & parent, mount_state & state, write_batch & batch)
+      -> kstd::result<void>
   {
-    if (!directory.append_blocks(1))
+    if (!directory.append_blocks(1, batch))
     {
       return kstd::failure(ext2_errc::not_enough_free_blocks);
     }
@@ -477,10 +479,10 @@ namespace kernel::filesystem::ext2
     auto & parent_inode_data = parent.data();
     parent_inode_data.links_count++;
 
-    return write_inode(parent.number(), parent_inode_data, state)
-        .and_then([&]() { return global_block; })
-        .and_then([&](auto block_number) { return write_block(block_number, buffer, state); })
-        .transform([](auto) {});
+    batch.mark_inode(parent);
+
+    return global_block.and_then([&](auto number) { return write_block(number, buffer, state); }).transform([](auto) {
+    });
   }
 
   auto filesystem::write_directory_entry_to_buffer(std::span<std::byte> buffer, kstd::bytes offset, inode & target,
@@ -579,7 +581,8 @@ namespace kernel::filesystem::ext2
     return inode_offset;
   }
 
-  auto filesystem::allocate_blocks(size_t count, mount_state & state) -> kstd::result<kstd::vector<uint32_t>>
+  auto filesystem::allocate_blocks(size_t count, mount_state & state, write_batch & batch)
+      -> kstd::result<kstd::vector<uint32_t>>
   {
     auto & [superblock, block_group_descriptors, backing_inode] = state;
 
@@ -588,73 +591,70 @@ namespace kernel::filesystem::ext2
       return kstd::failure(ext2_errc::not_enough_free_blocks);
     }
 
-    kstd::vector<uint32_t> allocated_blocks{};
+    struct pending_group
+    {
+      std::size_t index;
+      std::size_t claimed;
+      kstd::vector<std::byte> bitmap;
+    };
+
+    auto pending = kstd::vector<pending_group>{};
+    auto allocated_blocks = kstd::vector<uint32_t>{};
 
-    for (auto block_group_descriptor_index = 0uz; block_group_descriptor_index < block_group_descriptors.size();
-         ++block_group_descriptor_index)
+    for (auto group_index = 0uz; group_index < block_group_descriptors.size() && allocated_blocks.size() < count;
+         ++group_index)
     {
-      auto & block_group_descriptor = block_group_descriptors.at(block_group_descriptor_index);
-      if (block_group_descriptor.free_blocks_count == 0)
+      auto & descriptor = block_group_descriptors.at(group_index);
+      if (descriptor.free_blocks_count == 0)
       {
         continue;
       }
 
       auto block_bitmap = kstd::vector<std::byte>{block_size(state).value};
-      if (auto read_result = read_block(block_group_descriptor.block_bitmap, block_bitmap, state); !read_result)
+      if (auto read_result = read_block(descriptor.block_bitmap, block_bitmap, state); !read_result)
       {
         return kstd::failure(read_result.error());
       }
 
-      bool is_bitmap_dirty = false;
+      auto claimed = 0uz;
       for (auto i = 0uz; i < static_cast<size_t>(superblock.blocks_per_group) && allocated_blocks.size() < count; ++i)
       {
         if (!bitmap_is_set(block_bitmap, i))
         {
-          is_bitmap_dirty = true;
-
           bitmap_set(block_bitmap, i);
-          block_group_descriptor.free_blocks_count--;
-          superblock.free_blocks_count--;
-
-          allocated_blocks.push_back(i + block_group_descriptor_index * superblock.blocks_per_group);
+          ++claimed;
+          allocated_blocks.push_back(i + group_index * superblock.blocks_per_group);
         }
       }
 
-      if (is_bitmap_dirty)
+      if (claimed > 0)
       {
-        if (auto write_result = write_block(block_group_descriptor.block_bitmap, block_bitmap, state); !write_result)
-        {
-          return kstd::failure(write_result.error());
-        }
-
-        // TODO update all block group descriptors
-        if (auto write_result =
-                backing_inode->write(kstd::raw_bytes(block_group_descriptor),
-                                     block_group_descriptor_table_offset(state) +
-                                         block_group_descriptor_index * kstd::size_of(block_group_descriptor));
-            !write_result)
-        {
-          return kstd::failure(write_result.error());
-        }
+        pending.push_back({group_index, claimed, std::move(block_bitmap)});
       }
     }
 
-    // TODO update all superblock copies
-    if (auto write_result = backing_inode->write(kstd::raw_bytes(superblock), constants::superblock_offset);
-        !write_result)
+    if (allocated_blocks.size() != count)
     {
-      return kstd::failure(write_result.error());
+      return kstd::failure(ext2_errc::not_enough_free_blocks);
     }
 
-    if (allocated_blocks.size() != count)
+    for (auto & group : pending)
     {
-      return kstd::failure(ext2_errc::not_enough_free_blocks);
+      auto & descriptor = block_group_descriptors.at(group.index);
+      descriptor.free_blocks_count -= group.claimed;
+      superblock.free_blocks_count -= group.claimed;
+
+      if (auto result = write_block(descriptor.block_bitmap, group.bitmap, state); !result)
+      {
+        return kstd::failure(result.error());
+      }
+      batch.mark_group(group.index);
     }
 
     return allocated_blocks;
   }
 
-  auto filesystem::allocate_inode(mount_state & state) -> kstd::result<uint32_t>
+  auto filesystem::allocate_inode(mount_state & state, write_batch & batch) -> kstd::result<uint32_t>
   {
     auto & [superblock, block_group_descriptors, _] = state;
 
@@ -691,18 +691,7 @@ namespace kernel::filesystem::ext2
             return kstd::failure(write_result.error());
           }
 
-          if (auto write_result =
-                  write_block_group_descriptor(block_group_descriptor, block_group_descriptor_index, state);
-              !write_result)
-          {
-            return kstd::failure(write_result.error());
-          }
-
-          // TODO update all superblocks
-          if (auto write_result = write_superblock(state); !write_result)
-          {
-            return kstd::failure(write_result.error());
-          }
+          batch.mark_group(block_group_descriptor_index);
 
           return block_group_descriptor_index * superblock.inodes_per_group + i + 1;
         }
@@ -788,8 +777,8 @@ namespace kernel::filesystem::ext2
   }
 
   auto filesystem::write_global_block_number_to_inode_block_index(size_t inode_block_index, inode_data & data,
-                                                                  uint32_t global_block_number, mount_state & state)
-      -> kstd::result<void>
+                                                                  uint32_t global_block_number, mount_state & state,
+                                                                  write_batch & batch) -> kstd::result<void>
   {
     if (inode_block_index < constants::direct_block_count)
     {
@@ -820,7 +809,7 @@ namespace kernel::filesystem::ext2
 
         if (idx == 0 && inode_block_index == 0)
         {
-          auto allocated = allocate_blocks(1, state);
+          auto allocated = allocate_blocks(1, state, batch);
           if (!allocated)
           {
             return kstd::failure(allocated.error());
diff --git a/kernel/kernel/filesystem/ext2/filesystem.hpp b/kernel/kernel/filesystem/ext2/filesystem.hpp
index f43e357c..1bce8d71 100644
--- a/kernel/kernel/filesystem/ext2/filesystem.hpp
+++ b/kernel/kernel/filesystem/ext2/filesystem.hpp
@@ -6,6 +6,7 @@
 #include <kernel/filesystem/ext2/linked_directory_entry.hpp>
 #include <kernel/filesystem/ext2/mount_state.hpp>
 #include <kernel/filesystem/ext2/superblock.hpp>
+#include <kernel/filesystem/ext2/write_batch.hpp>
 #include <kernel/filesystem/filesystem.hpp>
 #include <kernel/filesystem/inode.hpp>
 
@@ -154,8 +155,10 @@ namespace kernel::filesystem::ext2
     //!
     //! @param count The number of blocks to allocate.
     //! @param state The state to operate on.
+    //! @param batch The active write batch.
     //! @return A vector of the allocated block numbers.
-    auto allocate_blocks(size_t count, mount_state & state) -> kstd::result<kstd::vector<uint32_t>>;
+    auto allocate_blocks(size_t count, mount_state & state, write_batch & batch)
+        -> kstd::result<kstd::vector<uint32_t>>;
 
     //! Update the number of blocks allocated to an inode.
     //!
@@ -183,12 +186,13 @@ namespace kernel::filesystem::ext2
     //! Write a global block number to an inode block index.
     //!
     //! @param block_index The index of the block within the inode.
-    //! @param state The state to operate on.
     //! @param data The inode data.
     //! @param global_block_number The global block number to write.
+    //! @param state The state to operate on.
+    //! @param batch The active write batch.
     auto write_global_block_number_to_inode_block_index(size_t block_index, inode_data & data,
-                                                        uint32_t global_block_number, mount_state & state)
-        -> kstd::result<void>;
+                                                        uint32_t global_block_number, mount_state & state,
+                                                        write_batch & batch) -> kstd::result<void>;
 
   private:
     friend struct write_batch;
@@ -196,8 +200,9 @@ namespace kernel::filesystem::ext2
     //! Allocate a single inode in the file system.
     //!
     //! @param state The driver state to operate on.
+    //! @param batch The active write batch.
     //! @return An inode number on success, an error otherwise.
-    auto allocate_inode(mount_state & state) -> kstd::result<uint32_t>;
+    auto allocate_inode(mount_state & state, write_batch & batch) -> kstd::result<uint32_t>;
 
     //! Add a new directory entry to a given directory.
     //!
@@ -206,9 +211,10 @@ namespace kernel::filesystem::ext2
     //! @param child The inode number of the child.
     //! @param mode The file mode of the entry.
     //! @param state The driver state to operate on.
+    //! @param batch The active write batch.
     //! @return Nothing on success, an error code otherwise.
     auto add_directory_entry(inode & directory, std::string_view name, inode & child, std::uint16_t mode,
-                             mount_state & state) -> kstd::result<void>;
+                             mount_state & state, write_batch & batch) -> kstd::result<void>;
 
     //! Initialize a new directory inode.
     //!
@@ -218,8 +224,10 @@ namespace kernel::filesystem::ext2
     //! @param directory The inode to initialize.
     //! @param parent The parent of this newly initialized directory.
     //! @param state The driver state to operate on.
+    //! @param batch The active write batch.
     //! @return Nothing on success, an error otherwise.
-    auto init_directory(inode & directory, inode & parent, mount_state & state) -> kstd::result<void>;
+    auto init_directory(inode & directory, inode & parent, mount_state & state, write_batch & batch)
+        -> kstd::result<void>;
 
     //! Write a new directory entry into the given buffer.
     //!
diff --git a/kernel/kernel/filesystem/ext2/filesystem.tests.cpp b/kernel/kernel/filesystem/ext2/filesystem.tests.cpp
index 8d1fc3d9..28aabed7 100644
--- a/kernel/kernel/filesystem/ext2/filesystem.tests.cpp
+++ b/kernel/kernel/filesystem/ext2/filesystem.tests.cpp
@@ -7,6 +7,7 @@
 #include <kernel/filesystem/ext2/inode.hpp>
 #include <kernel/filesystem/ext2/mount_state.hpp>
 #include <kernel/filesystem/ext2/superblock.hpp>
+#include <kernel/filesystem/ext2/write_batch.hpp>
 #include <kernel/filesystem/filesystem.hpp>
 #include <kernel/filesystem/inode.hpp>
 #include <kernel/filesystem/mount.hpp>
@@ -313,30 +314,32 @@ SCENARIO("Ext2 block writing includes direct and all indirect levels", "[filesys
       constexpr auto doubly_global_block_number = 303;
       constexpr auto triply_global_block_number = 404;
 
-      REQUIRE(
-          fs->write_global_block_number_to_inode_block_index(0, inode_data, direct_global_block_number, *mount_state));
+      auto batch = kernel::filesystem::ext2::write_batch{*fs, *mount_state};
+
+      REQUIRE(fs->write_global_block_number_to_inode_block_index(0, inode_data, direct_global_block_number,
+                                                                 *mount_state, batch));
       REQUIRE(inode_data.block[0] == direct_global_block_number);
 
       REQUIRE(fs->write_global_block_number_to_inode_block_index(singly_start, inode_data, singly_global_block_number,
-                                                                 *mount_state));
+                                                                 *mount_state, batch));
       REQUIRE(inode_data.block[12] == 16);
       REQUIRE(read_u32(static_cast<kstd::bytes>(inode_data.block[12]) * block_size.value) ==
               singly_global_block_number);
 
       REQUIRE(fs->write_global_block_number_to_inode_block_index(singly_start + 1, inode_data,
-                                                                 singly_global_block_number + 1, *mount_state));
+                                                                 singly_global_block_number + 1, *mount_state, batch));
       REQUIRE(read_u32(static_cast<kstd::bytes>(inode_data.block[12]) * block_size.value +
                        1 * kstd::size_of<uint32_t>()) == singly_global_block_number + 1);
 
       REQUIRE(fs->write_global_block_number_to_inode_block_index(doubly_start, inode_data, doubly_global_block_number,
-                                                                 *mount_state));
+                                                                 *mount_state, batch));
       REQUIRE(inode_data.block[13] == 17);
       auto const doubly_leaf_table = read_u32(static_cast<size_t>(inode_data.block[13]) * block_size);
       REQUIRE(doubly_leaf_table == 18);
       REQUIRE(read_u32(static_cast<size_t>(doubly_leaf_table) * block_size) == doubly_global_block_number);
 
       REQUIRE(fs->write_global_block_number_to_inode_block_index(triply_start, inode_data, triply_global_block_number,
-                                                                 *mount_state));
+                                                                 *mount_state, batch));
       REQUIRE(inode_data.block[14] == 19);
       auto const triply_middle_table = read_u32(static_cast<size_t>(inode_data.block[14]) * block_size);
       REQUIRE(triply_middle_table == 20);
diff --git a/kernel/kernel/filesystem/ext2/inode.cpp b/kernel/kernel/filesystem/ext2/inode.cpp
index 34452ac8..56174638 100644
--- a/kernel/kernel/filesystem/ext2/inode.cpp
+++ b/kernel/kernel/filesystem/ext2/inode.cpp
@@ -1,8 +1,10 @@
 #include <kernel/filesystem/ext2/inode.hpp>
 
 #include <kernel/filesystem/error.hpp>
+#include <kernel/filesystem/ext2/error.hpp>
 #include <kernel/filesystem/ext2/filesystem.hpp>
 #include <kernel/filesystem/ext2/mount_state.hpp>
+#include <kernel/filesystem/ext2/write_batch.hpp>
 #include <kernel/filesystem/inode.hpp>
 #include <kernel/filesystem/mount.hpp>
 
@@ -136,15 +138,16 @@ namespace kernel::filesystem::ext2
     auto const current_block_count = (*filesystem)->block_count(*this, **state);
     auto const max_new_inode_size_without_new_blocks = current_block_count * block_size;
 
+    auto batch = write_batch{**filesystem, **state};
+
     if (new_inode_size > max_new_inode_size_without_new_blocks)
     {
       auto const additional_blocks_needed =
           (new_inode_size - max_new_inode_size_without_new_blocks + block_size - 1_B) / block_size;
 
-      if (!append_blocks(additional_blocks_needed))
+      if (!append_blocks(additional_blocks_needed, batch))
       {
-        // TODO handle out of space
-        kapi::system::panic("[EXT2] Failed to allocate blocks for inode write");
+        return kstd::failure(ext2_errc::not_enough_free_blocks);
       }
     }
 
@@ -181,11 +184,7 @@ namespace kernel::filesystem::ext2
     }
 
     set_size(std::max(this->size(), offset + bytes_written));
-    if (auto write_result = (*filesystem)->write_inode(m_inode_number, m_data, **state); !write_result)
-    {
-      return kstd::failure(write_result.error());
-    }
-
+    batch.mark_inode(*this);
     return bytes_written;
   }
 
@@ -245,7 +244,7 @@ namespace kernel::filesystem::ext2
     return result;
   }
 
-  auto inode::append_blocks(size_t count) -> bool
+  auto inode::append_blocks(size_t count, write_batch & batch) -> bool
   {
     auto filesystem = get_filesystem();
     if (!filesystem)
@@ -259,7 +258,7 @@ namespace kernel::filesystem::ext2
       return false;
     }
 
-    auto new_blocks = (*filesystem)->allocate_blocks(count, **state);
+    auto new_blocks = (*filesystem)->allocate_blocks(count, **state, batch);
     if (!new_blocks)
     {
       return false;
@@ -270,9 +269,9 @@ namespace kernel::filesystem::ext2
       auto const block_index = (*filesystem)->block_count(*this, **state) + i;
       auto const global_block_number = new_blocks->at(i);
 
-      if (auto write_result =
-              (*filesystem)
-                  ->write_global_block_number_to_inode_block_index(block_index, m_data, global_block_number, **state);
+      if (auto write_result = (*filesystem)
+                                  ->write_global_block_number_to_inode_block_index(block_index, m_data,
+                                                                                   global_block_number, **state, batch);
           !write_result)
       {
         return false;
diff --git a/kernel/kernel/filesystem/ext2/inode.hpp b/kernel/kernel/filesystem/ext2/inode.hpp
index 735905e9..5393a853 100644
--- a/kernel/kernel/filesystem/ext2/inode.hpp
+++ b/kernel/kernel/filesystem/ext2/inode.hpp
@@ -2,6 +2,7 @@
 #define TEACHOS_KERNEL_FILESYSTEM_EXT2_INODE_HPP
 
 #include <kernel/filesystem/ext2/mount_state.hpp>
+#include <kernel/filesystem/ext2/write_batch.hpp>
 #include <kernel/filesystem/inode.hpp>
 
 #include <kapi/filesystem.hpp>
@@ -96,8 +97,9 @@ namespace kernel::filesystem::ext2
     //! Append the specified number of blocks to this inode.
     //!
     //! @param count The number of blocks to append.
+    //! @param batch The active write batch.
     //! @return true if the blocks were successfully appended, false otherwise.
-    auto append_blocks(size_t count) -> bool;
+    auto append_blocks(size_t count, write_batch & batch) -> bool;
 
     //! Set the size of the file represented by this inode.
     //!
diff --git a/kernel/kernel/filesystem/ext2/inode.tests.cpp b/kernel/kernel/filesystem/ext2/inode.tests.cpp
index 4d57768b..eab0aabd 100644
--- a/kernel/kernel/filesystem/ext2/inode.tests.cpp
+++ b/kernel/kernel/filesystem/ext2/inode.tests.cpp
@@ -529,7 +529,7 @@ SCENARIO("Ext2 inode write across block boundaries", "[filesystem][ext2][inode]"
     THEN("writing over the current size and over the number of free blocks")
     {
       auto const offset = 65 * block_size;
-      REQUIRE_THROWS_AS(inode.write(buffer, offset), kernel::tests::cpu::halt);
+      REQUIRE_FALSE(inode.write(buffer, offset));
     }
   }
 }