Ext2 better block allocation (#18)

* fix(ext2): lots of issues linked to block bitmaps management

* feat(ext2): add reserved blocks for files owned by superuser

src/dev/mod.rs

@@ -4,15 +4,14 @@ use alloc::borrow::{Cow, ToOwned};
 use alloc::boxed::Box;
 use alloc::slice;
 use alloc::vec::Vec;
-#[cfg(feature = "std")]
 use core::cell::RefCell;
 use core::iter::Step;
 use core::mem::{size_of, transmute_copy};
 use core::ops::{Deref, DerefMut, Range};
 use core::ptr::{addr_of, slice_from_raw_parts};
-#[cfg(feature = "std")]
+#[cfg(any(feature = "std", test))]
 use std::fs::File;
-#[cfg(feature = "std")]
+#[cfg(any(feature = "std", test))]
 use std::io::ErrorKind;
 
 use self::sector::Address;
@@ -375,7 +374,7 @@ impl<E: core::error::Error, T: Base<Error = E> + Read + Write + Seek> Device<u8,
     }
 }
 
-#[cfg(feature = "std")]
+#[cfg(any(feature = "std", test))]
 impl<E: core::error::Error> Device<u8, E> for RefCell<File> {
     #[inline]
     fn size(&self) -> Size {

src/error.rs

@@ -21,7 +21,7 @@ pub enum Error<E: core::error::Error> {
     Path(PathError),
 
     /// Standard I/O error
-    #[cfg(feature = "std")]
+    #[cfg(any(feature = "std", test))]
     IO(std::io::Error),
 }
 
@@ -32,7 +32,7 @@ impl<E: core::error::Error> Display for Error<E> {
             Self::Device(device_error) => write!(formatter, "Device Error: {device_error}"),
             Self::Fs(fs_error) => write!(formatter, "Filesystem Error: {fs_error}"),
             Self::Path(path_error) => write!(formatter, "Path Error: {path_error}"),
-            #[cfg(feature = "std")]
+            #[cfg(any(feature = "std", test))]
             Self::IO(io_error) => write!(formatter, "I/O Error: {io_error}"),
         }
     }

src/fs/ext2/block.rs

@@ -9,13 +9,13 @@ use alloc::vec;
 use alloc::vec::Vec;
 
 use super::error::Ext2Error;
+use super::superblock::Superblock;
 use super::Ext2;
 use crate::dev::celled::Celled;
 use crate::dev::sector::Address;
 use crate::dev::Device;
 use crate::error::Error;
 use crate::fs::error::FsError;
-use crate::fs::ext2::block_group::BlockGroupDescriptor;
 use crate::io::{Base, Read, Seek, SeekFrom, Write};
 
 /// An ext2 block.
@@ -46,15 +46,15 @@ impl<Dev: Device<u8, Ext2Error>> Block<Dev> {
     /// Returns the containing block group of this block.
     #[inline]
     #[must_use]
-    pub fn block_group(&self) -> u32 {
-        self.number / self.filesystem.borrow().superblock().base().blocks_per_group
+    pub const fn block_group(&self, superblock: &Superblock) -> u32 {
+        self.number / superblock.base().blocks_per_group
     }
 
     /// Returns the offset of this block in its containing block group.
     #[inline]
     #[must_use]
-    pub fn group_index(&self) -> u32 {
-        self.number % self.filesystem.borrow().superblock().base().blocks_per_group
+    pub const fn group_index(&self, superblock: &Superblock) -> u32 {
+        self.number % superblock.base().blocks_per_group
     }
 
     /// Reads all the content from this block and returns it in a vector.
@@ -74,42 +74,26 @@ impl<Dev: Device<u8, Ext2Error>> Block<Dev> {
         Ok(buffer)
     }
 
-    /// Returns whether this block is currently free or not.
+    /// Returns whether this block is currently free or not from the block bitmap in which the block resides.
     ///
-    /// As this operation needs to read directly from the given device, it is quite costly in computational time.
-    ///
-    /// # Errors
-    ///
-    /// Returns a [`Error`] if the device cannot be read.
+    /// The `bitmap` argument is usually the result of the method [`get_block_bitmap`](../struct.Ext2.html#method.get_block_bitmap).
+    #[allow(clippy::indexing_slicing)]
     #[inline]
-    pub fn is_free(&self) -> Result<bool, Error<Ext2Error>> {
-        let fs = self.filesystem.borrow();
-
-        let block_group = self.block_group();
-        let block_group_descriptor = BlockGroupDescriptor::parse(&fs.device, fs.superblock(), block_group)?;
-        let mut block_group_descriptor_bitmap_block = Self::new(self.filesystem.clone(), block_group_descriptor.block_bitmap);
-
-        let bitmap_index = u64::from(self.group_index());
-        let byte_index = bitmap_index / 8;
-        let byte_offset = bitmap_index % 8;
-
-        let mut buffer = [0_u8];
-        block_group_descriptor_bitmap_block.seek(SeekFrom::Start(byte_index))?;
-        block_group_descriptor_bitmap_block.read(&mut buffer)?;
-
-        Ok((buffer[0] >> byte_offset) & 1 == 0)
+    #[must_use]
+    pub const fn is_free(&self, superblock: &Superblock, bitmap: &[u8]) -> bool {
+        let index = self.group_index(superblock) / 8;
+        let offset = self.number % 8;
+        bitmap[index as usize] >> offset & 1 == 0
     }
 
-    /// Returns whether this block is currently used or not.
-    ///
-    /// As this operation needs to read directly from the given device, it is quite costly in computational time.
+    /// Returns whether this block is currently used or not from the block bitmap in which the block resides.
     ///
-    /// # Errors
-    ///
-    /// Returns a [`Error`] if the device cannot be read.
+    /// The `bitmap` argument is usually the result of the method [`get_block_bitmap`](../struct.Ext2.html#method.get_block_bitmap).
+    #[allow(clippy::indexing_slicing)]
     #[inline]
-    pub fn is_used(&self) -> Result<bool, Error<Ext2Error>> {
-        self.is_free().map(|is_free| !is_free)
+    #[must_use]
+    pub const fn is_used(&self, superblock: &Superblock, bitmap: &[u8]) -> bool {
+        !self.is_free(superblock, bitmap)
     }
 
     /// Sets the current block usage in the block bitmap, and updates the superblock accordingly.
@@ -121,32 +105,8 @@ impl<Dev: Device<u8, Ext2Error>> Block<Dev> {
     /// Returns an [`BlockAlreadyFree`](Ext2Error::BlockAlreadyFree) error if the given block was already free.
     ///
     /// Otherwise, returns an [`Error`] if the device cannot be written.
-    fn set_usage(&mut self, used: bool) -> Result<(), Error<Ext2Error>> {
-        let fs = self.filesystem.borrow();
-
-        let block_group = self.block_group();
-        let block_group_descriptor = BlockGroupDescriptor::parse(&fs.device, fs.superblock(), block_group)?;
-        let mut block_group_descriptor_bitmap_block = Self::new(self.filesystem.clone(), block_group_descriptor.block_bitmap);
-        let bitmap_index = self.group_index();
-
-        let byte_index = bitmap_index / 8;
-        let byte_offset = bitmap_index % 8;
-
-        let mut buffer = [0_u8];
-
-        block_group_descriptor_bitmap_block.seek(SeekFrom::Start(u64::from(byte_index)))?;
-        block_group_descriptor_bitmap_block.read(&mut buffer)?;
-
-        if (buffer[0] >> byte_offset) & 1 == 1 && used {
-            Err(Ext2Error::BlockAlreadyInUse(self.number).into())
-        } else if (buffer[0] >> byte_offset) & 1 == 0 && !used {
-            Err(Ext2Error::BlockAlreadyFree(self.number).into())
-        } else {
-            buffer[0] ^= 1 << byte_offset;
-            block_group_descriptor_bitmap_block.seek(SeekFrom::Current(-1_i64))?;
-            block_group_descriptor_bitmap_block.write(&buffer)?;
-            Ok(())
-        }
+    fn set_usage(&mut self, usage: bool) -> Result<(), Error<Ext2Error>> {
+        self.filesystem.borrow_mut().locate_blocs(&[self.number], usage)
     }
 
     /// Sets the current block as free in the block bitmap, and updates the superblock accordingly.
@@ -283,6 +243,7 @@ mod test {
     use crate::dev::sector::Address;
     use crate::dev::Device;
     use crate::fs::ext2::block::Block;
+    use crate::fs::ext2::block_group::BlockGroupDescriptor;
     use crate::fs::ext2::error::Ext2Error;
     use crate::fs::ext2::superblock::Superblock;
     use crate::fs::ext2::Ext2;
@@ -357,22 +318,38 @@ mod test {
                 .unwrap(),
         );
         let ext2 = Celled::new(Ext2::new(file, 0).unwrap());
+        let superblock = ext2.borrow().superblock().clone();
 
-        let mut block = Block::new(ext2, BLOCK_NUMBER);
+        let mut block = Block::new(ext2.clone(), BLOCK_NUMBER);
+        let block_group = block.block_group(&superblock);
 
-        assert!(block.is_used().unwrap());
+        let fs = ext2.borrow();
+        let block_group_descriptor = BlockGroupDescriptor::parse(&fs.device, fs.superblock(), block_group).unwrap();
+        let free_block_count = block_group_descriptor.free_blocks_count;
+
+        let bitmap = fs.get_block_bitmap(block_group).unwrap();
+
+        drop(fs);
+
+        assert!(block.is_used(&superblock, &bitmap));
 
         block.set_free().unwrap();
 
-        assert!(block.is_free().unwrap());
+        let fs = ext2.borrow();
+        let new_free_block_count = BlockGroupDescriptor::parse(&fs.device, fs.superblock(), block.block_group(&superblock))
+            .unwrap()
+            .free_blocks_count;
+
+        assert!(block.is_free(&superblock, &fs.get_block_bitmap(block_group).unwrap()));
+        assert_eq!(free_block_count + 1, new_free_block_count);
 
         fs::remove_file("./tests/fs/ext2/io_operations_copy_block_set_free.ext2").unwrap();
     }
 
     #[test]
     fn block_set_used() {
         // This block should not be used
-        const BLOCK_NUMBER: u32 = 1234;
+        const BLOCK_NUMBER: u32 = 1920;
 
         fs::copy("./tests/fs/ext2/io_operations.ext2", "./tests/fs/ext2/io_operations_copy_block_set_used.ext2").unwrap();
 
@@ -384,14 +361,30 @@ mod test {
                 .unwrap(),
         );
         let ext2 = Celled::new(Ext2::new(file, 0).unwrap());
+        let superblock = ext2.borrow().superblock().clone();
 
-        let mut block = Block::new(ext2, BLOCK_NUMBER);
+        let mut block = Block::new(ext2.clone(), BLOCK_NUMBER);
+        let block_group = block.block_group(&superblock);
+
+        let fs = ext2.borrow();
+        let block_group_descriptor = BlockGroupDescriptor::parse(&fs.device, fs.superblock(), block_group).unwrap();
+        let free_block_count = block_group_descriptor.free_blocks_count;
 
-        assert!(block.is_free().unwrap());
+        let bitmap = fs.get_block_bitmap(block_group).unwrap();
+
+        drop(fs);
+
+        assert!(block.is_free(&superblock, &bitmap));
 
         block.set_used().unwrap();
 
-        assert!(block.is_used().unwrap());
+        let fs = ext2.borrow();
+        let new_free_block_count = BlockGroupDescriptor::parse(&fs.device, fs.superblock(), block.block_group(&superblock))
+            .unwrap()
+            .free_blocks_count;
+
+        assert!(block.is_used(&superblock, &fs.get_block_bitmap(block_group).unwrap()));
+        assert_eq!(free_block_count - 1, new_free_block_count);
 
         fs::remove_file("./tests/fs/ext2/io_operations_copy_block_set_used.ext2").unwrap();
     }

src/fs/ext2/block_group.rs

@@ -30,7 +30,7 @@ pub struct BlockGroupDescriptor {
     /// Starting block address of inode table.
     pub inode_table: u32,
 
-    /// Number of unallocated blocks in groupµ.
+    /// Number of unallocated blocks in group.
     pub free_blocks_count: u16,
 
     /// Number of unallocated inodes in group.
@@ -63,9 +63,7 @@ impl BlockGroupDescriptor {
         };
 
         let superblock_end_address = SUPERBLOCK_START_BYTE + SUPERBLOCK_SIZE;
-        Ok(Address::new(
-            superblock_end_address + if superblock_end_address % (superblock.block_size() as usize) == 0 { 0 } else { 1 },
-        ))
+        Ok(Address::new(superblock_end_address + BLOCK_GROUP_DESCRIPTOR_SIZE * n as usize))
     }
 
     /// Parse the `n`th block group descriptor from the given device (starting at 0).
@@ -84,9 +82,8 @@ impl BlockGroupDescriptor {
     ) -> Result<Self, Error<Ext2Error>> {
         let device = celled_device.borrow();
 
-        let table_start_address = Self::starting_addr(superblock, n)?;
+        let block_group_descriptor_address = Self::starting_addr(superblock, n)?;
 
-        let block_group_descriptor_address = table_start_address + (n as usize * BLOCK_GROUP_DESCRIPTOR_SIZE);
         // SAFETY: all the possible failures are catched in the resulting error
         unsafe { device.read_at::<Self>(block_group_descriptor_address) }
     }

src/fs/ext2/file.rs

@@ -25,6 +25,10 @@ use crate::fs::PATH_MAX;
 use crate::io::{Base, Read, Seek, SeekFrom, Write};
 use crate::types::{Blkcnt, Blksize, Dev, Gid, Ino, Mode, Nlink, Off, Time, Timespec, Uid};
 
+/// Arbitrary number of supplementary reserved blocks for each file owned by the UID or the GID declared in [the
+/// superblock]((struct.Base.html#structfield.def_resuid)).
+pub const SUPPLEMENTARY_RESERVED_BLOCKS_PER_WRITE: u64 = 8;
+
 /// Limit in bytes for the length of a pointed path of a symbolic link to be store in an inode and not in a separate data block.
 pub const SYMBOLIC_LINK_INODE_STORE_LIMIT: usize = 60;
 
@@ -255,10 +259,18 @@ impl<D: Device<u8, Ext2Error>> Write for File<D> {
             return Err(Error::Fs(FsError::Implementation(Ext2Error::OutOfBounds(buf.len() as i128))));
         }
 
+        let reserved_blocks =
+            if self.inode.uid == fs.superblock().base().def_resuid || self.inode.gid == fs.superblock().base().def_resgid {
+                SUPPLEMENTARY_RESERVED_BLOCKS_PER_WRITE
+            } else {
+                0
+            };
+
         // Calcul of the number of needed data blocks
         let bytes_to_write = buf.len() as u64;
-        let blocks_needed =
-            (bytes_to_write + self.io_offset) / block_size + u64::from((bytes_to_write + self.io_offset) % block_size != 0);
+        let blocks_needed = (bytes_to_write + self.io_offset) / block_size
+            + u64::from((bytes_to_write + self.io_offset) % block_size != 0)
+            + reserved_blocks;
         let (
             initial_direct_block_pointers,
             (initial_singly_indirect_block_pointer, initial_singly_indirect_blocks),
@@ -437,14 +449,14 @@ impl<D: Device<u8, Ext2Error>> Write for File<D> {
             }
         }
 
-        // Add the free blocks where it's necessary.
-        let free_block_numbers = &mut self
+        let free_blocks = self
             .filesystem
+            .borrow()
             // SAFETY: `blocks_to_request <= blocks_needed < u32::MAX`
-            .free_blocks(unsafe { u32::try_from(total_blocks_to_request).unwrap_unchecked() })?
-            .into_iter();
+            .free_blocks(unsafe { u32::try_from(total_blocks_to_request).unwrap_unchecked() })?;
 
-        let mut free_block_copied = free_block_numbers.clone();
+        // Add the free blocks where it's necessary.
+        let free_block_numbers = &mut free_blocks.clone().into_iter();
 
         // Direct block pointers
         direct_block_pointers.append(&mut free_block_numbers.take(12 - direct_block_pointers.len()).collect_vec());
@@ -655,13 +667,12 @@ impl<D: Device<u8, Ext2Error>> Write for File<D> {
         updated_inode.size = unsafe { u32::try_from(new_size & u64::from(u32::MAX)).unwrap_unchecked() };
         // TODO: update `updated_inode.blocks`
 
-        assert!(u32::try_from(new_size).is_ok(), "Search how to deal with bigger files");
+        assert!(u32::try_from(new_size).is_ok(), "TODO: Search how to deal with bigger files");
 
         // SAFETY: the updated inode contains the right inode created in this function
         unsafe { self.set_inode(&updated_inode) }?;
 
-        // TODO: be smarter to avoid make 1000000 calls to device's `write`
-        free_block_copied.try_for_each(|block| Block::new(self.filesystem.clone(), block).set_used())?;
+        self.filesystem.as_ref().borrow_mut().allocate_blocs(&free_blocks)?;
 
         Ok(written_bytes)
     }

src/fs/ext2/inode.rs

@@ -70,8 +70,12 @@ pub struct Inode {
     /// Count of hard links (directory entries) to this inode. When this reaches 0, the data blocks are marked as unallocated.
     pub links_count: u16,
 
-    /// Count of disk sectors (not Ext2 blocks) in use by this inode, not counting the actual inode structure nor directory entries
-    /// linking to the inode.
+    /// Indicates the amount of blocks reserved for the associated file data. This includes both currently in used
+    /// and currently reserved blocks in case the file grows in size.
+    ///
+    ///  Since this value represents 512-byte blocks and not file system blocks, this value should not be directly used as an index
+    /// to the i_block array. Rather, the maximum index of the i_block array should be computed from `i_blocks /
+    /// ((1024<<s_log_block_size)/512)`, or once simplified, `i_blocks/(2<<s_log_block_size)`.
     pub blocks: u32,
 
     /// Flags.
@@ -451,6 +455,7 @@ impl Inode {
 
         let block_group = Self::block_group(superblock, n);
         let block_group_descriptor = BlockGroupDescriptor::parse(celled_device, superblock, block_group)?;
+
         let inode_table_starting_block = block_group_descriptor.inode_table;
         let index = Self::group_index(superblock, n);