diff --git a/kernel/crates/another_ext4/ext4_test/src/bin/recovery_fault_injection.rs b/kernel/crates/another_ext4/ext4_test/src/bin/recovery_fault_injection.rs index 2970ee0ae..03d1850ee 100644 --- a/kernel/crates/another_ext4/ext4_test/src/bin/recovery_fault_injection.rs +++ b/kernel/crates/another_ext4/ext4_test/src/bin/recovery_fault_injection.rs @@ -3040,6 +3040,13 @@ fn main() { if let Ok(case) = std::env::var("DRAGONOS_EXT4_RECOVERY_CASE") { let persistence = PersistenceModel::WriteBack; match case.as_str() { + "orphan-index" => { + for model in [PersistenceModel::WriteBack, PersistenceModel::WriteThrough] { + run_journal_reclaim_matrix(model); + run_journal_linked_tail_final_unlink_matrix(model); + run_journal_linked_tail_rename_matrix(model); + } + } "reserved-payload-retry" => run_reserved_delalloc_payload_retry_test(persistence), "production-append" => run_production_delalloc_append_block_test(persistence), "production-split" => run_production_delalloc_extent_split_test(persistence), diff --git a/kernel/crates/another_ext4/src/ext4/alloc.rs b/kernel/crates/another_ext4/src/ext4/alloc.rs index 32e4c46c0..91e9ecd9e 100644 --- a/kernel/crates/another_ext4/src/ext4/alloc.rs +++ b/kernel/crates/another_ext4/src/ext4/alloc.rs @@ -2051,9 +2051,9 @@ impl Ext4 { } // Each iteration starts from the checkpointed inode-table entry. The // on-disk extent root is therefore the restart cursor after any crash. - // Chain membership was fully validated once above. The metadata write - // barrier keeps the chain stable, avoiding O(extents * orphan_count) - // repeated walks; final orphan_del performs its own bounded walk. + // Membership comes from the validated, transaction-maintained index. + // The metadata write barrier keeps the chain stable throughout reclaim; + // final orphan_del rechecks the target and indexed predecessor images. loop { let mut inode = self.validate_reclaim_inode(inode_id, generation)?; if !inode.inode.uses_extents() { diff --git a/kernel/crates/another_ext4/src/ext4/extent.rs b/kernel/crates/another_ext4/src/ext4/extent.rs index f42a12b2a..64c516390 100644 --- a/kernel/crates/another_ext4/src/ext4/extent.rs +++ b/kernel/crates/another_ext4/src/ext4/extent.rs @@ -2385,6 +2385,8 @@ mod tests { Ext4 { block_device, metadata_cache: crate::ext4::MetadataBlockCache::new(16), + orphan_index: spin::Mutex::new(crate::ext4::orphan::LegacyOrphanIndex::default()), + orphan_index_valid: Arc::new(core::sync::atomic::AtomicBool::new(false)), cached_super_block: spin::Mutex::new(sb), cached_block_groups: Vec::new(), system_metadata_ranges: Vec::new(), @@ -2434,6 +2436,8 @@ mod tests { Ext4 { block_device, metadata_cache: crate::ext4::MetadataBlockCache::new(16), + orphan_index: spin::Mutex::new(crate::ext4::orphan::LegacyOrphanIndex::default()), + orphan_index_valid: Arc::new(core::sync::atomic::AtomicBool::new(false)), cached_super_block: spin::Mutex::new(sb), cached_block_groups: Vec::new(), system_metadata_ranges: Vec::new(), diff --git a/kernel/crates/another_ext4/src/ext4/journal.rs b/kernel/crates/another_ext4/src/ext4/journal.rs index 756f02e51..206a3e9e8 100644 --- a/kernel/crates/another_ext4/src/ext4/journal.rs +++ b/kernel/crates/another_ext4/src/ext4/journal.rs @@ -410,6 +410,8 @@ impl Ext4 { { return Err(Ext4Error::new(ErrCode::EIO)); } + self.orphan_index_valid + .store(false, core::sync::atomic::Ordering::Release); ext4_sb = recovered_sb; *self.cached_super_block.lock() = recovered_sb; self.cached_block_groups = recovered_groups; diff --git a/kernel/crates/another_ext4/src/ext4/journal_transaction.rs b/kernel/crates/another_ext4/src/ext4/journal_transaction.rs index 275461176..7cbf8d3c9 100644 --- a/kernel/crates/another_ext4/src/ext4/journal_transaction.rs +++ b/kernel/crates/another_ext4/src/ext4/journal_transaction.rs @@ -343,6 +343,7 @@ pub struct Transaction<'a> { retired: Vec, preserve_originals: bool, owns_writer: bool, + orphan_index: Option>, } impl Transaction<'_> { @@ -358,8 +359,19 @@ impl Transaction<'_> { retired: Vec::new(), preserve_originals, owns_writer: true, + orphan_index: None, } } + /// The caller owns the exclusive metadata gate until this transaction is + /// consumed. Index changes are provisional until logical publication. + pub(super) fn track_orphan_index(&mut self, valid: Arc) { + if let Some(previous) = &self.orphan_index { + debug_assert!(Arc::ptr_eq(previous, &valid)); + } else { + self.orphan_index = Some(valid); + } + } + /// Replace the final image for `home`. Re-staging the same home block does /// not consume another credit and subsequent reads observe the replacement. pub fn stage(&mut self, home: PBlockId, image: Box<[u8; BLOCK_SIZE]>) -> Result<()> { @@ -514,6 +526,9 @@ impl Transaction<'_> { return Err(Ext4Error::new(ErrCode::EINVAL)); }; let result = core.publish(&mut self.staged, &mut self.retired, publisher); + if result.is_ok() { + self.orphan_index = None; + } self.release_writer(); result } @@ -620,6 +635,7 @@ impl Transaction<'_> { return self.fail(error, CommitFailure::CommitUncertain, true); } publisher.publish_home_current(&self.staged, &self.retired); + self.orphan_index = None; self.release_writer(); Ok(()) } @@ -669,6 +685,7 @@ impl Transaction<'_> { } } publisher.publish_home_current(&self.staged, &self.retired); + self.orphan_index = None; self.release_writer(); Ok(()) } @@ -693,6 +710,9 @@ impl Transaction<'_> { let result = core.commit_images(device, &images, || { publisher.publish_home_current(&self.staged, &self.retired) }); + if result.is_ok() { + self.orphan_index = None; + } self.release_writer(); result } @@ -1013,6 +1033,9 @@ impl JournalTransactionCore { impl Drop for Transaction<'_> { fn drop(&mut self) { + if let Some(valid) = self.orphan_index.take() { + valid.store(false, Ordering::Release); + } self.release_writer(); } } @@ -1399,6 +1422,77 @@ mod tests { } } + #[test] + fn orphan_index_is_invalidated_by_abort_drop_and_commit_failure() { + for action in 0..3 { + let valid = Arc::new(AtomicBool::new(true)); + let device = MemoryDevice::new(); + let publisher = Publisher(AtomicUsize::new(0)); + let core = DirectTransactionCore::new(128).unwrap(); + let mut operation = core.start(1).unwrap(); + operation.track_orphan_index(valid.clone()); + operation.stage(2, Box::new([2; BLOCK_SIZE])).unwrap(); + match action { + 0 => operation.abort(), + 1 => drop(operation), + _ => { + device.fail_at.store(0, Ordering::SeqCst); + assert!(operation.commit(&device, &publisher).is_err()); + } + } + assert!(!valid.load(Ordering::Acquire)); + } + } + + #[test] + fn orphan_index_survives_successful_logical_publication() { + let device = MemoryDevice::new(); + let publisher = Publisher(AtomicUsize::new(0)); + let direct = DirectTransactionCore::new(128).unwrap(); + let valid = Arc::new(AtomicBool::new(true)); + let mut operation = direct.start(1).unwrap(); + operation.track_orphan_index(valid.clone()); + operation.stage(2, Box::new([2; BLOCK_SIZE])).unwrap(); + operation.commit(&device, &publisher).unwrap(); + assert!(valid.load(Ordering::Acquire)); + + let journal = JournalTransactionCore::new(context_with_ring(64, 1)).unwrap(); + let mut operation = staged_journal_transaction(&journal, 1); + operation.track_orphan_index(valid.clone()); + operation.commit(&device, &publisher).unwrap(); + assert!(valid.load(Ordering::Acquire)); + + let batch = JournalBatchCore::new(context_with_ring(64, 1), 32).unwrap(); + let mut operation = batch.start(1).unwrap(); + operation.track_orphan_index(valid.clone()); + operation.stage(2, Box::new([3; BLOCK_SIZE])).unwrap(); + operation.publish(&publisher).unwrap(); + assert!(valid.load(Ordering::Acquire)); + // A newer operation's abort invalidates the current logical index; + // checkpointing the older accepted batch must not revive it. + let mut aborted = batch.start(1).unwrap(); + aborted.track_orphan_index(valid.clone()); + aborted.abort(); + batch.request_seal(); + batch.commit_pending(&device).unwrap(); + assert!(!valid.load(Ordering::Acquire)); + } + + #[test] + fn orphan_index_is_invalidated_at_every_journal_failure_boundary() { + for failure in 0..14 { + let device = MemoryDevice::new(); + device.fail_at.store(failure, Ordering::SeqCst); + let publisher = Publisher(AtomicUsize::new(0)); + let journal = JournalTransactionCore::new(context_with_ring(64, 1)).unwrap(); + let valid = Arc::new(AtomicBool::new(true)); + let mut operation = staged_journal_transaction(&journal, 1); + operation.track_orphan_index(valid.clone()); + let result = operation.commit(&device, &publisher); + assert_eq!(valid.load(Ordering::Acquire), result.is_ok()); + } + } + #[test] fn direct_commit_writes_home_blocks_in_order_then_publishes() { let device = MemoryDevice::new(); diff --git a/kernel/crates/another_ext4/src/ext4/low_level.rs b/kernel/crates/another_ext4/src/ext4/low_level.rs index 9835e59c2..47781aaf9 100644 --- a/kernel/crates/another_ext4/src/ext4/low_level.rs +++ b/kernel/crates/another_ext4/src/ext4/low_level.rs @@ -4605,6 +4605,8 @@ mod tests { Ext4 { block_device, metadata_cache: crate::ext4::MetadataBlockCache::new(16), + orphan_index: spin::Mutex::new(crate::ext4::orphan::LegacyOrphanIndex::default()), + orphan_index_valid: Arc::new(core::sync::atomic::AtomicBool::new(false)), cached_super_block: spin::Mutex::new(sb), cached_block_groups: Vec::new(), system_metadata_ranges: Vec::new(), diff --git a/kernel/crates/another_ext4/src/ext4/mod.rs b/kernel/crates/another_ext4/src/ext4/mod.rs index 9314cc5a1..e017e08d4 100644 --- a/kernel/crates/another_ext4/src/ext4/mod.rs +++ b/kernel/crates/another_ext4/src/ext4/mod.rs @@ -349,6 +349,8 @@ pub struct Ext4 { /// Bounded raw metadata acceleration. Journal overlays remain the /// authoritative accepted view; this cache is always discardable. metadata_cache: MetadataBlockCache, + orphan_index: spin::Mutex, + orphan_index_valid: Arc, /// Cached superblock to avoid repeated disk reads. /// The superblock is loaded once at mount time and updated /// in memory whenever it is written to disk. @@ -818,6 +820,8 @@ impl Ext4 { Ok(Self { block_device, metadata_cache: MetadataBlockCache::new(0), + orphan_index: spin::Mutex::new(orphan::LegacyOrphanIndex::default()), + orphan_index_valid: Arc::new(AtomicBool::new(false)), cached_super_block: spin::Mutex::new(sb), cached_block_groups, system_metadata_ranges, diff --git a/kernel/crates/another_ext4/src/ext4/orphan.rs b/kernel/crates/another_ext4/src/ext4/orphan.rs index b0cc457ad..012b3d8b7 100644 --- a/kernel/crates/another_ext4/src/ext4/orphan.rs +++ b/kernel/crates/another_ext4/src/ext4/orphan.rs @@ -15,6 +15,74 @@ pub(super) struct LegacyOrphanChain { pub(super) inodes: Vec, } +/// Membership and predecessor links for a completely validated logical chain. +/// The metadata mutation gate excludes writers while this index is used. +/// Transaction-private updates are invalidated on abort, never rolled forward +/// by an older batch checkpoint. +#[derive(Default)] +pub(super) struct LegacyOrphanIndex { + head: InodeId, + entries: BTreeMap, +} + +impl LegacyOrphanIndex { + fn from_chain(chain: &[InodeId]) -> Self { + let mut result = Self { + head: chain.first().copied().unwrap_or(0), + ..Self::default() + }; + for (position, id) in chain.iter().copied().enumerate() { + let previous = position.checked_sub(1).map(|i| chain[i]).unwrap_or(0); + let next = chain.get(position + 1).copied().unwrap_or(0); + result.entries.insert(id, (previous, next)); + } + result + } + + fn predecessor(&self, id: InodeId) -> Result { + self.entries + .get(&id) + .map(|entry| entry.0) + .ok_or_else(corruption) + } + + fn add(&mut self, id: InodeId, old_head: InodeId) -> Result<()> { + if id == 0 || self.head != old_head || self.entries.contains_key(&id) { + return Err(corruption()); + } + if old_head != 0 && self.entries.get(&old_head).map(|entry| entry.0) != Some(0) { + return Err(corruption()); + } + self.entries.insert(id, (0, old_head)); + if let Some(entry) = self.entries.get_mut(&old_head) { + entry.0 = id; + } + self.head = id; + Ok(()) + } + + fn remove(&mut self, id: InodeId, next: InodeId) -> Result<()> { + let (previous, expected_next) = *self.entries.get(&id).ok_or_else(corruption)?; + if next != expected_next + || (previous == 0 && self.head != id) + || (previous != 0 && self.entries.get(&previous).map(|entry| entry.1) != Some(id)) + || (next != 0 && self.entries.get(&next).map(|entry| entry.0) != Some(id)) + { + return Err(corruption()); + } + if let Some(entry) = self.entries.get_mut(&previous) { + entry.1 = next; + } else { + self.head = next; + } + if let Some(entry) = self.entries.get_mut(&next) { + entry.0 = previous; + } + self.entries.remove(&id); + Ok(()) + } +} + /// The durable role of an inode in the legacy orphan chain. /// /// A linked tail is not interchangeable with a final-unlink orphan. The @@ -140,17 +208,38 @@ impl Ext4 { Err(corruption()) } - /// Verify that `inode_id` is a member of the complete, valid legacy list. - /// - /// Reclaim calls this while holding the target inode's mutation shard. A - /// complete bounded walk is intentional: accepting a locally plausible - /// node from a corrupt list could permanently lose the remainder at final - /// deletion. + /// Query a completely validated logical list. Callers hold a metadata + /// mutation/read gate (or the unpublished mount-recovery exclusion). + /// The first query after construction or an aborted update validates the + /// complete chain; subsequent queries use transaction-maintained links. pub(super) fn legacy_orphan_contains(&self, inode_id: InodeId) -> Result { - Ok(self - .validate_legacy_orphan_chain()? - .inodes - .contains(&inode_id)) + self.ensure_orphan_index()?; + Ok(self.orphan_index.lock().entries.contains_key(&inode_id)) + } + + fn ensure_orphan_index(&self) -> Result<()> { + use core::sync::atomic::Ordering; + if self.orphan_index_valid.load(Ordering::Acquire) { + return Ok(()); + } + // Do not hold a spinlock while validating metadata through block I/O. + // The caller's metadata gate excludes every chain mutation. Compatible + // read callers can build the same validated snapshot concurrently. + let chain = self.validate_legacy_orphan_chain()?; + *self.orphan_index.lock() = LegacyOrphanIndex::from_chain(&chain.inodes); + self.orphan_index_valid.store(true, Ordering::Release); + Ok(()) + } + + fn index_orphan_add( + &self, + transaction: &mut super::journal_transaction::Transaction<'_>, + inode: InodeId, + old_head: InodeId, + ) -> Result<()> { + self.ensure_orphan_index()?; + transaction.track_orphan_index(self.orphan_index_valid.clone()); + self.orphan_index.lock().add(inode, old_head) } /// Insert a zero-link inode at the head of the legacy orphan list in the @@ -172,6 +261,7 @@ impl Ext4 { if old_head == inode.id || (old_head != 0 && !valid_orphan_number(self, old_head)) { return Err(corruption()); } + self.index_orphan_add(transaction, inode.id, old_head)?; inode.inode.set_next_orphan(old_head); self.transaction_stage_inode_with_csum(transaction, inode)?; sb.set_last_orphan(inode.id); @@ -202,6 +292,7 @@ impl Ext4 { if old_head == inode.id || (old_head != 0 && !valid_orphan_number(self, old_head)) { return Err(corruption()); } + self.index_orphan_add(transaction, inode.id, old_head)?; inode.inode.set_next_orphan(old_head); self.transaction_stage_inode_with_csum(transaction, inode)?; sb.set_last_orphan(inode.id); @@ -279,8 +370,9 @@ impl Ext4 { /// Remove an inode from the legacy orphan chain in the caller's final /// reclaim transaction. Both head and non-head deletion are supported. - /// The walk is bounded by `s_inodes_count` and validates every visited - /// inode before changing either the predecessor or superblock image. + /// The validated index finds the predecessor without a full chain walk. + /// Allocation, checksums, generation and links are checked against the + /// authoritative local images before staging predecessor/head changes. pub(super) fn transaction_orphan_del( &self, transaction: &mut super::journal_transaction::Transaction<'_>, @@ -293,46 +385,43 @@ impl Ext4 { return Err(corruption()); } - let mut current = sb.last_orphan(); - let mut predecessor: Option = None; - let mut visited = BTreeSet::new(); - while current != 0 { - if !valid_orphan_number(self, current) - || visited.len() >= sb.inode_count() as usize - || !visited.insert(current) - || !self.inode_is_allocated(current)? - { + self.ensure_orphan_index()?; + let predecessor = self.orphan_index.lock().predecessor(target)?; + // The index identifies the predecessor; authoritative local images + // still establish allocation, checksum, generation and exact links. + if !self.inode_is_allocated(target)? { + return Err(corruption()); + } + let current = self.read_inode_uncached(target)?; + if !inode_checksum_valid(sb, ¤t) + || current.inode.file_type() == FileType::Unknown + || current.inode.generation() != inode.inode.generation() + || current.inode.next_orphan() != target_next + { + return Err(corruption()); + } + if predecessor != 0 { + if !self.inode_is_allocated(predecessor)? { return Err(corruption()); } - let current_inode = self.read_inode_uncached(current)?; - if !inode_checksum_valid(sb, ¤t_inode) - || current_inode.inode.file_type() == FileType::Unknown + let mut pred = self.read_inode_uncached(predecessor)?; + if !inode_checksum_valid(sb, &pred) + || pred.inode.file_type() == FileType::Unknown + || pred.inode.next_orphan() != target { return Err(corruption()); } - let next = current_inode.inode.next_orphan(); - if next != 0 && !valid_orphan_number(self, next) { + pred.inode.set_next_orphan(target_next); + self.transaction_stage_inode_with_csum(transaction, &mut pred)?; + } else { + if sb.last_orphan() != target { return Err(corruption()); } - if current == target { - if current_inode.inode.generation() != inode.inode.generation() - || next != target_next - { - return Err(corruption()); - } - if let Some(mut pred) = predecessor { - pred.inode.set_next_orphan(target_next); - self.transaction_stage_inode_with_csum(transaction, &mut pred)?; - } else { - sb.set_last_orphan(target_next); - self.transaction_stage_super_block(transaction, sb)?; - } - return Ok(()); - } - predecessor = Some(current_inode); - current = next; + sb.set_last_orphan(target_next); + self.transaction_stage_super_block(transaction, sb)?; } - Err(corruption()) + transaction.track_orphan_index(self.orphan_index_valid.clone()); + self.orphan_index.lock().remove(target, target_next) } /// Reject formats whose orphan state this implementation cannot update. @@ -452,6 +541,34 @@ impl LegacyOrphanReader for Ext4 { mod tests { use super::*; + #[test] + fn validated_index_tracks_head_middle_tail_and_reuse() { + let mut index = LegacyOrphanIndex::from_chain(&[11, 12, 13]); + assert_eq!(index.predecessor(11).unwrap(), 0); + assert_eq!(index.predecessor(13).unwrap(), 12); + index.remove(12, 13).unwrap(); + assert_eq!(index.predecessor(13).unwrap(), 11); + index.remove(11, 13).unwrap(); + assert_eq!(index.predecessor(13).unwrap(), 0); + index.add(12, 13).unwrap(); + index.remove(13, 0).unwrap(); + index.remove(12, 0).unwrap(); + assert!(index.entries.is_empty()); + assert_eq!(index.head, 0); + } + + #[test] + fn validated_index_rejects_inconsistent_transitions() { + let mut index = LegacyOrphanIndex::from_chain(&[11, 12, 13]); + assert!(index.add(12, 11).is_err()); + assert!(index.add(14, 12).is_err()); + assert!(index.remove(12, 0).is_err()); + assert!(index.remove(14, 0).is_err()); + assert_eq!(index.head, 11); + assert_eq!(index.entries.len(), 3); + assert_eq!(index.predecessor(13).unwrap(), 12); + } + struct MockReader { nodes: BTreeMap, }