Improve directory entry handling efficiency in ext2

kernel/src/fs/ext2/dir.rs

@@ -50,7 +50,7 @@ impl DirEntry {
     }
 
     /// Returns the length of the header.
-    fn header_len() -> usize {
+    const fn header_len() -> usize {
         core::mem::size_of::<DirEntryHeader>()
     }
 
@@ -59,17 +59,12 @@ impl DirEntry {
         self.header.ino
     }
 
-    /// Modifies the inode number.
-    pub fn set_ino(&mut self, ino: u32) {
-        self.header.ino = ino;
-    }
-
     /// Returns the name.
     pub fn name(&self) -> &str {
         self.name.as_str().unwrap()
     }
 
-    /// Returns the type.
+    /// Returns the inode type of the entry.
     pub fn type_(&self) -> InodeType {
         InodeType::from(DirEntryFileType::try_from(self.header.inode_type).unwrap())
     }
@@ -87,19 +82,14 @@ impl DirEntry {
 
     /// Returns the actual length of the current entry.
     pub(super) fn actual_len(&self) -> usize {
-        (Self::header_len() + self.name.len()).align_up(4)
+        (Self::header_len() + self.header.name_len as usize).align_up(4)
-    }
-
-    /// Returns the length of the gap between the current entry and the next entry.
-    pub(super) fn gap_len(&self) -> usize {
-        self.record_len() - self.actual_len()
     }
 }
 
 /// The header of `DirEntry`.
 #[repr(C)]
 #[derive(Clone, Copy, Debug, Pod)]
-struct DirEntryHeader {
+pub(super) struct DirEntryHeader {
     /// Inode number
     ino: u32,
     /// Directory entry length
@@ -156,7 +146,14 @@ impl From<DirEntryFileType> for InodeType {
 /// A reader for reading `DirEntry` from the page cache.
 pub struct DirEntryReader<'a> {
     page_cache: &'a PageCache,
-    name_buf: [u8; MAX_FNAME_LEN],
+    from_offset: usize,
+    name_buf: Option<[u8; MAX_FNAME_LEN]>,
+}
+
+/// An iterator for iterating `DirEntryItem` from the
+/// page cache given a start offset.
+pub(super) struct DirEntryIter<'a> {
+    page_cache: &'a PageCache,
     offset: usize,
 }
 
@@ -165,13 +162,99 @@ impl<'a> DirEntryReader<'a> {
     pub(super) fn new(page_cache: &'a PageCache, from_offset: usize) -> Self {
         Self {
             page_cache,
-            name_buf: [0u8; MAX_FNAME_LEN],
+            from_offset,
-            offset: from_offset,
+            name_buf: None,
         }
     }
 
-    /// Reads one `DirEntry` from the current offset.
+    /// Returns an iterator for iterating `DirEntryItem`s.
-    pub fn read_entry(&mut self) -> Result<DirEntry> {
+    pub fn iter(&self) -> DirEntryIter<'a> {
+        DirEntryIter {
+            page_cache: self.page_cache,
+            offset: self.from_offset,
+        }
+    }
+
+    /// Returns an iterator for iterating `DirEntry`s.
+    pub fn iter_entries(&'a mut self) -> impl Iterator<Item = DirEntry> + 'a {
+        let iter = self.iter();
+        iter.filter_map(|entry_item| match self.read_name(&entry_item) {
+            Ok(name_buf) => Some(DirEntry {
+                header: entry_item.header,
+                name: CStr256::from(name_buf),
+            }),
+            Err(_) => None,
+        })
+    }
+
+    /// Whether the directory contains an entry with the given name.
+    pub fn contains_entry(&mut self, name: &str) -> bool {
+        let mut iter = self.iter();
+        iter.any(|entry_item| {
+            if entry_item.name_len() != name.len() {
+                return false;
+            }
+            match self.read_name(&entry_item) {
+                Ok(name_buf) => name_buf == name.as_bytes(),
+                Err(_) => false,
+            }
+        })
+    }
+
+    /// Returns the target entry with the given name.
+    pub fn find_entry_item(&mut self, name: &str) -> Option<DirEntryItem> {
+        let mut iter = self.iter();
+        iter.find(|entry_item| {
+            if entry_item.name_len() != name.len() {
+                return false;
+            }
+            match self.read_name(entry_item) {
+                Ok(name_buf) => name_buf == name.as_bytes(),
+                Err(_) => false,
+            }
+        })
+    }
+
+    /// Returns the number of entries in the directory.
+    pub fn entry_count(&self) -> usize {
+        self.iter().count()
+    }
+
+    /// Reads the name of the entry from the page cache to the inner buffer.
+    fn read_name(&mut self, entry_item: &DirEntryItem) -> Result<&[u8]> {
+        if self.name_buf.is_none() {
+            self.name_buf = Some([0; MAX_FNAME_LEN]);
+        }
+
+        let name_len = entry_item.name_len();
+        let name_buf = &mut self.name_buf.as_mut().unwrap()[..name_len];
+
+        let offset = entry_item.offset + DirEntry::header_len();
+        self.page_cache.pages().read_bytes(offset, name_buf)?;
+        Ok(name_buf)
+    }
+}
+
+impl DirEntryIter<'_> {
+    /// Reads a `DirEntryItem` at the current offset.
+    fn read_entry_item(&mut self) -> Result<DirEntryItem> {
+        if self.offset >= self.page_cache.pages().size() {
+            return_errno!(Errno::ENOENT);
+        }
+
+        let header = self.read_header()?;
+        let record_len = header.record_len as usize;
+        let item = DirEntryItem {
+            header,
+            offset: self.offset,
+        };
+
+        self.offset += record_len;
+        Ok(item)
+    }
+
+    /// Reads the header of the entry from the page cache.
+    fn read_header(&mut self) -> Result<DirEntryHeader> {
         let header = self
             .page_cache
             .pages()
@@ -179,35 +262,85 @@ impl<'a> DirEntryReader<'a> {
         if header.ino == 0 {
             return_errno!(Errno::ENOENT);
         }
-
+        Ok(header)
-        let name_len = header.name_len as usize;
-        self.page_cache.pages().read_bytes(
-            self.offset + DirEntry::header_len(),
-            &mut self.name_buf[..name_len],
-        )?;
-        let entry = DirEntry {
-            header,
-            name: CStr256::from(&self.name_buf[..name_len]),
-        };
-        self.offset += entry.record_len();
-
-        Ok(entry)
     }
 }
 
-impl Iterator for DirEntryReader<'_> {
+impl Iterator for DirEntryIter<'_> {
-    type Item = (usize, DirEntry);
+    type Item = DirEntryItem;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let offset = self.offset;
+        self.read_entry_item().ok()
-        let entry = match self.read_entry() {
-            Ok(entry) => entry,
-            Err(_) => {
-                return None;
     }
-        };
+}
 
-        Some((offset, entry))
+/// A directory entry item describes the basic information of a `DirEntry`,
+/// including the entry header and the entry's offset. The entry name is not
+/// present and will be retrieved from the page cache when needed.
+#[derive(Debug)]
+pub(super) struct DirEntryItem {
+    header: DirEntryHeader,
+    offset: usize,
+}
+
+impl DirEntryItem {
+    /// Returns a reference to the header.
+    pub fn header(&self) -> &DirEntryHeader {
+        &self.header
+    }
+
+    /// Returns the offset of the entry.
+    pub fn offset(&self) -> usize {
+        self.offset
+    }
+
+    /// Returns the inode number.
+    pub fn ino(&self) -> u32 {
+        self.header.ino as _
+    }
+
+    /// Modifies the inode number.
+    pub fn set_ino(&mut self, ino: u32) {
+        self.header.ino = ino as _;
+    }
+
+    /// Returns the length of the name.
+    pub fn name_len(&self) -> usize {
+        self.header.name_len as _
+    }
+
+    /// Returns the inode type of the entry.
+    pub fn type_(&self) -> InodeType {
+        InodeType::from(DirEntryFileType::try_from(self.header.inode_type).unwrap())
+    }
+
+    /// Returns the distance to the next entry.
+    pub fn record_len(&self) -> usize {
+        self.header.record_len as _
+    }
+
+    /// Modifies the distance to the next entry.
+    pub fn set_record_len(&mut self, record_len: usize) {
+        debug_assert!(record_len >= self.actual_len());
+        self.header.record_len = record_len as _;
+    }
+
+    /// Returns the actual length of the current entry.
+    pub fn actual_len(&self) -> usize {
+        (DirEntry::header_len() + self.name_len()).align_up(4)
+    }
+
+    /// Returns the length of the gap between the current entry and the next entry.
+    pub fn gap_len(&self) -> usize {
+        self.record_len() - self.actual_len()
+    }
+
+    /// Converts to a `DirEntry` given the name.
+    pub fn to_entry_with_name(&self, name: &str) -> DirEntry {
+        DirEntry {
+            header: self.header,
+            name: CStr256::from(name),
+        }
     }
 }
 
@@ -215,15 +348,16 @@ impl Iterator for DirEntryReader<'_> {
 pub struct DirEntryWriter<'a> {
     page_cache: &'a PageCache,
     offset: usize,
+    name_buf: Option<[u8; MAX_FNAME_LEN]>,
 }
 
-// TODO: Improve the efficiency of the writer operations.
 impl<'a> DirEntryWriter<'a> {
     /// Constructs a writer with the given page cache and offset.
     pub(super) fn new(page_cache: &'a PageCache, from_offset: usize) -> Self {
         Self {
             page_cache,
             offset: from_offset,
+            name_buf: None,
         }
     }
 
@@ -236,17 +370,26 @@ impl<'a> DirEntryWriter<'a> {
             self.offset + DirEntry::header_len(),
             entry.name().as_bytes(),
         )?;
+
         self.offset += entry.record_len();
         Ok(())
     }
 
+    /// Writes the header of a `DirEntry` at the current offset.
+    pub(super) fn write_header_only(&mut self, header: &DirEntryHeader) -> Result<()> {
+        self.page_cache.pages().write_val(self.offset, header)?;
+        self.offset += header.record_len as usize;
+        Ok(())
+    }
+
     /// Appends a new `DirEntry` starting from the current offset.
     ///
     /// If there is a gap between existing entries, inserts the new entry into the gap；
     /// If there is no available space, expands the size and appends the new entry at the end.
     pub fn append_entry(&mut self, mut new_entry: DirEntry) -> Result<()> {
-        let Some((offset, mut entry)) = DirEntryReader::new(self.page_cache, self.offset)
+        let Some(mut entry_item) = DirEntryReader::new(self.page_cache, self.offset)
-            .find(|(_, entry)| entry.gap_len() >= new_entry.record_len())
+            .iter()
+            .find(|entry| entry.gap_len() >= new_entry.record_len())
         else {
             // Resize and append it at the new block.
             let old_size = self.page_cache.pages().size();
@@ -259,10 +402,10 @@ impl<'a> DirEntryWriter<'a> {
         };
 
         // Write in the gap between existing entries.
-        new_entry.set_record_len(entry.gap_len());
+        new_entry.set_record_len(entry_item.gap_len());
-        entry.set_record_len(entry.actual_len());
+        entry_item.set_record_len(entry_item.actual_len());
-        self.offset = offset;
+        self.offset = entry_item.offset;
-        self.write_entry(&entry)?;
+        self.write_header_only(&entry_item.header)?;
         self.write_entry(&new_entry)?;
         Ok(())
     }
@@ -270,34 +413,39 @@ impl<'a> DirEntryWriter<'a> {
     /// Removes and returns an existing `DirEntry` indicated by `name`.
     pub fn remove_entry(&mut self, name: &str) -> Result<DirEntry> {
         let self_entry_record_len = DirEntry::self_entry(0).record_len();
-        let reader = DirEntryReader::new(self.page_cache, 0);
+        let reader = DirEntryReader::new(self.page_cache, 0).iter();
-        let next_reader = DirEntryReader::new(self.page_cache, self_entry_record_len);
+        let next_reader = DirEntryReader::new(self.page_cache, self_entry_record_len).iter();
-        let Some(((pre_offset, mut pre_entry), (offset, entry))) = reader
+        let Some((mut pre_entry_item, entry_item)) =
-            .zip(next_reader)
+            reader.zip(next_reader).find(|(_, entry_item)| {
-            .find(|((offset, _), (_, dir_entry))| dir_entry.name() == name)
+                entry_item.name_len() == name.len()
+                    && self.read_name(entry_item).unwrap() == name.as_bytes()
+            })
         else {
             return_errno!(Errno::ENOENT);
         };
 
-        if DirEntryReader::new(self.page_cache, offset)
+        let pre_offset = pre_entry_item.offset;
+        let offset = entry_item.offset;
+        if Bid::from_offset(pre_offset) != Bid::from_offset(offset)
+            && DirEntryReader::new(self.page_cache, entry_item.offset)
+                .iter()
                 .next()
                 .is_none()
-            && Bid::from_offset(pre_offset) != Bid::from_offset(offset)
         {
             // Shrink the size.
             let new_size = pre_offset.align_up(BLOCK_SIZE);
             self.page_cache.resize(new_size)?;
-            pre_entry.set_record_len(new_size - pre_offset);
+            pre_entry_item.set_record_len(new_size - pre_offset);
             self.offset = pre_offset;
-            self.write_entry(&pre_entry)?;
+            self.write_header_only(&pre_entry_item.header)?;
         } else {
             // Update the previous entry.
-            pre_entry.set_record_len(pre_entry.record_len() + entry.record_len());
+            pre_entry_item.set_record_len(pre_entry_item.record_len() + entry_item.record_len());
             self.offset = pre_offset;
-            self.write_entry(&pre_entry)?;
+            self.write_header_only(&pre_entry_item.header)?;
         }
 
-        Ok(entry)
+        Ok(entry_item.to_entry_with_name(name))
     }
 
     /// Renames the `DirEntry` from `old_name` to the `new_name` from the current offset.
@@ -305,15 +453,15 @@ impl<'a> DirEntryWriter<'a> {
     /// It will moves the `DirEntry` to another position,
     /// if the record length is not big enough.
     pub fn rename_entry(&mut self, old_name: &str, new_name: &str) -> Result<()> {
-        let (offset, entry) = DirEntryReader::new(self.page_cache, self.offset)
+        let entry_item = DirEntryReader::new(self.page_cache, self.offset)
-            .find(|(offset, entry)| entry.name() == old_name)
+            .find_entry_item(old_name)
             .ok_or(Error::new(Errno::ENOENT))?;
 
-        let mut new_entry = DirEntry::new(entry.ino(), new_name, entry.type_());
+        let mut new_entry = DirEntry::new(entry_item.ino(), new_name, entry_item.type_());
-        if new_entry.record_len() <= entry.record_len() {
+        if new_entry.record_len() <= entry_item.record_len() {
             // Just rename the entry.
-            new_entry.set_record_len(entry.record_len());
+            new_entry.set_record_len(entry_item.record_len());
-            self.offset = offset;
+            self.offset = entry_item.offset;
             self.write_entry(&new_entry)?;
         } else {
             // Move to another position.
@@ -321,6 +469,22 @@ impl<'a> DirEntryWriter<'a> {
             self.offset = 0;
             self.append_entry(new_entry)?;
         }
+
         Ok(())
     }
+
+    /// Reads the name of the entry from the page cache to the inner buffer.
+    fn read_name(&mut self, item: &DirEntryItem) -> Result<&[u8]> {
+        if self.name_buf.is_none() {
+            self.name_buf = Some([0; MAX_FNAME_LEN]);
+        }
+
+        let name_len = item.name_len();
+        let name_buf = &mut self.name_buf.as_mut().unwrap()[..name_len];
+
+        let offset = item.offset + DirEntry::header_len();
+        self.page_cache.pages().read_bytes(offset, name_buf)?;
+
+        Ok(name_buf)
+    }
 }

kernel/src/fs/ext2/inode.rs

@@ -10,7 +10,7 @@ use inherit_methods_macro::inherit_methods;
 
 use super::{
     block_ptr::{BidPath, BlockPtrs, Ext2Bid, BID_SIZE, MAX_BLOCK_PTRS},
-    dir::{DirEntry, DirEntryReader, DirEntryWriter},
+    dir::{DirEntry, DirEntryItem, DirEntryReader, DirEntryWriter},
     fs::Ext2,
     indirect_block_cache::{IndirectBlock, IndirectBlockCache},
     prelude::*,
@@ -129,7 +129,7 @@ impl Inode {
         if inner.hard_links() == 0 {
             return_errno_with_message!(Errno::ENOENT, "dir removed");
         }
-        if inner.get_entry(name).is_some() {
+        if inner.contains_entry(name) {
             return_errno!(Errno::EEXIST);
         }
 
@@ -144,7 +144,7 @@ impl Inode {
         let new_entry = DirEntry::new(inode.ino, name, inode_type);
 
         let mut inner = inner.upgrade();
-        if let Err(e) = inner.append_entry(new_entry) {
+        if let Err(e) = inner.append_entry(new_entry, inode_type, name) {
             self.fs().free_inode(inode.ino, is_dir).unwrap();
             return Err(e);
         }
@@ -156,10 +156,9 @@ impl Inode {
     }
 
     fn init(&self, dir_ino: u32) -> Result<()> {
-        let mut inner = self.inner.write();
+        match self.type_ {
-        match inner.inode_type() {
             InodeType::Dir => {
-                inner.init_dir(self.ino, dir_ino)?;
+                self.inner.write().init_dir(self.ino, dir_ino)?;
             }
             _ => {
                 // TODO: Reserve serval blocks for regular file?
@@ -181,7 +180,10 @@ impl Inode {
             return_errno_with_message!(Errno::ENOENT, "dir removed");
         }
 
-        let ino = inner.get_entry_ino(name).ok_or(Error::new(Errno::ENOENT))?;
+        let ino = inner
+            .find_entry_item(name)
+            .map(|entry| entry.ino())
+            .ok_or(Error::new(Errno::ENOENT))?;
         drop(inner);
         self.fs().lookup_inode(ino)
     }
@@ -204,13 +206,13 @@ impl Inode {
             return_errno!(Errno::EPERM);
         }
 
-        if inner.get_entry(name).is_some() {
+        if inner.contains_entry(name) {
             return_errno!(Errno::EEXIST);
         }
 
         let new_entry = DirEntry::new(inode.ino, name, inode_type);
         let mut inner = inner.upgrade();
-        inner.append_entry(new_entry)?;
+        inner.append_entry(new_entry, inode_type, name)?;
         let now = now();
         inner.set_mtime(now);
         inner.set_ctime(now);
@@ -239,8 +241,8 @@ impl Inode {
             return_errno_with_message!(Errno::ENOENT, "dir removed");
         }
         let (offset, new_ino) = self_inner
-            .get_entry(name)
+            .find_entry_item(name)
-            .map(|(offset, entry)| (offset, entry.ino()))
+            .map(|entry| (entry.offset(), entry.ino()))
             .ok_or(Error::new(Errno::ENOENT))?;
         if file.ino != new_ino {
             return_errno!(Errno::ENOENT);
@@ -284,8 +286,8 @@ impl Inode {
             return_errno_with_message!(Errno::ENOENT, "dir removed");
         }
         let (offset, new_ino) = self_inner
-            .get_entry(name)
+            .find_entry_item(name)
-            .map(|(offset, entry)| (offset, entry.ino()))
+            .map(|entry| (entry.offset(), entry.ino()))
             .ok_or(Error::new(Errno::ENOENT))?;
         if dir_inode.ino != new_ino {
             return_errno!(Errno::ENOENT);
@@ -323,13 +325,16 @@ impl Inode {
 
         let fs = self.fs();
         let (src_offset, src_inode, src_inode_typ) = {
-            let (offset, entry) = self_inner
+            let entry = self_inner
-                .get_entry(old_name)
+                .find_entry_item(old_name)
                 .ok_or(Error::new(Errno::ENOENT))?;
-            (offset, fs.lookup_inode(entry.ino())?, entry.type_())
+            (entry.offset(), fs.lookup_inode(entry.ino())?, entry.type_())
         };
 
-        let Some(dst_ino) = self_inner.get_entry_ino(new_name) else {
+        let Some(dst_ino) = self_inner
+            .find_entry_item(new_name)
+            .map(|entry| entry.ino())
+        else {
             let mut self_inner = self_inner.upgrade();
             self_inner.rename_entry_at(old_name, new_name, src_offset)?;
             let now = now();
@@ -354,8 +359,8 @@ impl Inode {
         }
 
         let (src_offset, new_src_ino) = self_inner
-            .get_entry(old_name)
+            .find_entry_item(old_name)
-            .map(|(offset, entry)| (offset, entry.ino()))
+            .map(|entry| (entry.offset(), entry.ino()))
             .ok_or(Error::new(Errno::ENOENT))?;
         if src_inode.ino != new_src_ino {
             return_errno!(Errno::ENOENT);
@@ -365,9 +370,10 @@ impl Inode {
             return_errno!(Errno::ENOENT);
         }
 
-        let (dst_offset, new_dst_entry) = self_inner
+        let new_dst_entry = self_inner
-            .get_entry(new_name)
+            .find_entry_item(new_name)
             .ok_or(Error::new(Errno::ENOENT))?;
+        let dst_offset = new_dst_entry.offset();
         if dst_inode.ino != new_dst_entry.ino() {
             return_errno!(Errno::ENOENT);
         }
@@ -435,10 +441,10 @@ impl Inode {
 
         let fs = self.fs();
         let (src_offset, src_inode, src_inode_typ) = {
-            let (offset, entry) = self_inner
+            let entry = self_inner
-                .get_entry(old_name)
+                .find_entry_item(old_name)
                 .ok_or(Error::new(Errno::ENOENT))?;
-            (offset, fs.lookup_inode(entry.ino())?, entry.type_())
+            (entry.offset(), fs.lookup_inode(entry.ino())?, entry.type_())
         };
         // Avoid renaming a directory to a subdirectory of itself
         if src_inode.ino == target.ino {
@@ -446,7 +452,10 @@ impl Inode {
         }
         let is_dir = src_inode_typ == InodeType::Dir;
 
-        let Some(dst_ino) = target_inner.get_entry_ino(new_name) else {
+        let Some(dst_ino) = target_inner
+            .find_entry_item(new_name)
+            .map(|entry| entry.ino())
+        else {
             drop(self_inner);
             drop(target_inner);
 
@@ -463,8 +472,8 @@ impl Inode {
                 return_errno_with_message!(Errno::ENOENT, "dir removed");
             }
             let (src_offset, new_src_ino) = self_inner
-                .get_entry(old_name)
+                .find_entry_item(old_name)
-                .map(|(offset, entry)| (offset, entry.ino()))
+                .map(|entry| (entry.offset(), entry.ino()))
                 .ok_or(Error::new(Errno::ENOENT))?;
             if src_inode.ino != new_src_ino {
                 return_errno!(Errno::ENOENT);
@@ -476,7 +485,7 @@ impl Inode {
 
             self_inner.remove_entry_at(old_name, src_offset)?;
             let new_entry = DirEntry::new(src_inode.ino, new_name, src_inode_typ);
-            target_inner.append_entry(new_entry)?;
+            target_inner.append_entry(new_entry, src_inode_typ, new_name)?;
             let now = now();
             self_inner.set_mtime(now);
             self_inner.set_ctime(now);
@@ -521,8 +530,8 @@ impl Inode {
         }
 
         let (src_offset, new_src_ino) = self_inner
-            .get_entry(old_name)
+            .find_entry_item(old_name)
-            .map(|(offset, entry)| (offset, entry.ino()))
+            .map(|entry| (entry.offset(), entry.ino()))
             .ok_or(Error::new(Errno::ENOENT))?;
         if src_inode.ino != new_src_ino {
             return_errno!(Errno::ENOENT);
@@ -532,9 +541,10 @@ impl Inode {
             return_errno!(Errno::ENOENT);
         }
 
-        let (dst_offset, new_dst_entry) = target_inner
+        let new_dst_entry = target_inner
-            .get_entry(new_name)
+            .find_entry_item(new_name)
             .ok_or(Error::new(Errno::ENOENT))?;
+        let dst_offset = new_dst_entry.offset();
         if dst_inode.ino != new_dst_entry.ino() {
             return_errno!(Errno::ENOENT);
         }
@@ -563,7 +573,7 @@ impl Inode {
         self_inner.remove_entry_at(old_name, src_offset)?;
         target_inner.remove_entry_at(new_name, dst_offset)?;
         let new_entry = DirEntry::new(src_inode.ino, new_name, src_inode_typ);
-        target_inner.append_entry(new_entry)?;
+        target_inner.append_entry(new_entry, src_inode_typ, new_name)?;
         dst_inner.dec_hard_links();
         let now = now();
         self_inner.set_mtime(now);
@@ -598,8 +608,8 @@ impl Inode {
             }
 
             let try_readdir = |offset: &mut usize, visitor: &mut dyn DirentVisitor| -> Result<()> {
-                let dir_entry_reader = DirEntryReader::new(&inner.page_cache, *offset);
+                let mut dir_entry_reader = DirEntryReader::new(&inner.page_cache, *offset);
-                for (_, dir_entry) in dir_entry_reader {
+                for dir_entry in dir_entry_reader.iter_entries() {
                     visitor.visit(
                         dir_entry.name(),
                         dir_entry.ino() as u64,
@@ -1019,33 +1029,41 @@ impl InodeInner {
     }
 
     fn init_dir(&mut self, self_ino: u32, parent_ino: u32) -> Result<()> {
-        self.append_entry(DirEntry::self_entry(self_ino))?;
+        debug_assert_eq!(self.inode_type(), InodeType::Dir);
-        self.append_entry(DirEntry::parent_entry(parent_ino))?;
+        self.append_entry(DirEntry::self_entry(self_ino), InodeType::Dir, ".")?;
+        self.append_entry(DirEntry::parent_entry(parent_ino), InodeType::Dir, "..")?;
         Ok(())
     }
 
-    pub fn get_entry_ino(&self, name: &str) -> Option<u32> {
+    pub fn contains_entry(&self, name: &str) -> bool {
-        self.get_entry(name).map(|(_, entry)| entry.ino())
+        DirEntryReader::new(&self.page_cache, 0).contains_entry(name)
     }
 
-    pub fn get_entry(&self, name: &str) -> Option<(usize, DirEntry)> {
+    pub fn find_entry_item(&self, name: &str) -> Option<DirEntryItem> {
-        DirEntryReader::new(&self.page_cache, 0).find(|(offset, entry)| entry.name() == name)
+        DirEntryReader::new(&self.page_cache, 0).find_entry_item(name)
     }
 
     pub fn entry_count(&self) -> usize {
-        DirEntryReader::new(&self.page_cache, 0).count()
+        DirEntryReader::new(&self.page_cache, 0).entry_count()
     }
 
-    pub fn append_entry(&mut self, entry: DirEntry) -> Result<()> {
+    pub fn append_entry(
-        let is_dir = entry.type_() == InodeType::Dir;
+        &mut self,
-        let is_parent = entry.name() == "..";
+        entry: DirEntry,
+        inode_type: InodeType,
+        name: &str,
+    ) -> Result<()> {
+        debug_assert!(inode_type == entry.type_() && entry.name() == name);
 
         DirEntryWriter::new(&self.page_cache, 0).append_entry(entry)?;
-        let file_size = self.inode_impl.file_size();
+        let file_size = self.file_size();
         let page_cache_size = self.page_cache.pages().size();
         if page_cache_size > file_size {
             self.inode_impl.resize(page_cache_size)?;
         }
+
+        let is_dir = inode_type == InodeType::Dir;
+        let is_parent = name == "..";
         if is_dir && !is_parent {
             self.inc_hard_links(); // for ".."
         }
@@ -1055,7 +1073,7 @@ impl InodeInner {
     pub fn remove_entry_at(&mut self, name: &str, offset: usize) -> Result<()> {
         let entry = DirEntryWriter::new(&self.page_cache, offset).remove_entry(name)?;
         let is_dir = entry.type_() == InodeType::Dir;
-        let file_size = self.inode_impl.file_size();
+        let file_size = self.file_size();
         let page_cache_size = self.page_cache.pages().size();
         if page_cache_size < file_size {
             self.inode_impl.resize(page_cache_size)?;
@@ -1068,7 +1086,7 @@ impl InodeInner {
 
     pub fn rename_entry_at(&mut self, old_name: &str, new_name: &str, offset: usize) -> Result<()> {
         DirEntryWriter::new(&self.page_cache, offset).rename_entry(old_name, new_name)?;
-        let file_size = self.inode_impl.file_size();
+        let file_size = self.file_size();
         let page_cache_size = self.page_cache.pages().size();
         if page_cache_size != file_size {
             self.inode_impl.resize(page_cache_size)?;
@@ -1077,15 +1095,16 @@ impl InodeInner {
     }
 
     pub fn set_parent_ino(&mut self, parent_ino: u32) -> Result<()> {
-        let (offset, mut entry) = self.get_entry("..").unwrap();
+        let mut entry_item = self.find_entry_item("..").unwrap();
-        entry.set_ino(parent_ino);
+        entry_item.set_ino(parent_ino);
-        DirEntryWriter::new(&self.page_cache, offset).write_entry(&entry)?;
+        DirEntryWriter::new(&self.page_cache, entry_item.offset())
+            .write_header_only(entry_item.header())?;
         Ok(())
     }
 
     pub fn sync_data(&self) -> Result<()> {
         // Writes back the data in page cache.
-        let file_size = self.inode_impl.file_size();
+        let file_size = self.file_size();
         self.page_cache.evict_range(0..file_size)?;
         Ok(())
     }