[cache_check (rust)] Add more checks

- check version-2 dirty bitset size and consistency
- check discard bitset size

Cargo.lock

@@ -810,7 +810,6 @@ dependencies = [
  "thiserror",
  "threadpool",
  "tui",
- "typenum",
 ]
 
 [[package]]
@@ -864,12 +863,6 @@ dependencies = [
  "unicode-width",
 ]
 
-[[package]]
-name = "typenum"
-version = "1.12.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "373c8a200f9e67a0c95e62a4f52fbf80c23b4381c05a17845531982fa99e6b33"
-
 [[package]]
 name = "unicode-segmentation"
 version = "1.6.0"

src/cache/check.rs

@@ -1,5 +1,6 @@
 use anyhow::anyhow;
-use std::collections::*;
+use fixedbitset::FixedBitSet;
+use std::collections::BTreeSet;
 use std::path::Path;
 use std::sync::{Arc, Mutex};
 
@@ -8,6 +9,7 @@ use crate::cache::mapping::*;
 use crate::cache::superblock::*;
 use crate::io_engine::{AsyncIoEngine, IoEngine, SyncIoEngine};
 use crate::pdata::array_walker::*;
+use crate::pdata::bitset_walker::*;
 
 //------------------------------------------
 
@@ -15,96 +17,138 @@ const MAX_CONCURRENT_IO: u32 = 1024;
 
 //------------------------------------------
 
-struct CheckMappingVisitor {
+mod format1 {
-    metadata_version: u32,
+    use super::*;
-    allowed_flags: u32,
-    nr_origin_blocks: u64,
-    seen_oblocks: Arc<Mutex<BTreeSet<u64>>>,
-    //dirty_iterator: Option<BitsetIterator>,
-}
 
-impl CheckMappingVisitor {
+    pub struct MappingChecker {
-    fn new(metadata_version: u32, nr_origin_blocks: Option<u64>, dirty_root: Option<u64>) -> CheckMappingVisitor {
+        nr_origin_blocks: u64,
-        let mut flags: u32 = MappingFlags::Valid as u32;
+        seen_oblocks: Mutex<BTreeSet<u64>>,
-        //let dirty_iterator;
-        if metadata_version == 1 {
-            flags |= MappingFlags::Dirty as u32;
-            //dirty_iterator = None;
-        } else {
-            let _b = dirty_root.expect("dirty bitset unavailable");
-            //dirty_iterator = Some(BitsetIterator::new(b));
-        }
-
-        CheckMappingVisitor {
-            metadata_version,
-            allowed_flags: flags,
-            nr_origin_blocks: if let Some(n) = nr_origin_blocks {n} else {MAX_ORIGIN_BLOCKS},
-            seen_oblocks: Arc::new(Mutex::new(BTreeSet::new())),
-            //dirty_iterator,
-        }
     }
 
-    // TODO: move to ctor of Mapping?
+    impl MappingChecker {
-    fn check_flags(&self, m: &Mapping) -> anyhow::Result<()> {
+        pub fn new(nr_origin_blocks: Option<u64>) -> MappingChecker {
-        if (m.flags & !self.allowed_flags) != 0 {
+            MappingChecker {
-            return Err(anyhow!("unknown flags in mapping"));
+                nr_origin_blocks: if let Some(n) = nr_origin_blocks {n} else {MAX_ORIGIN_BLOCKS},
-        }
+                seen_oblocks: Mutex::new(BTreeSet::new()),
-
-        if !m.is_valid() {
-            if self.metadata_version == 1 {
-                if m.is_dirty() {
-                    return Err(anyhow!("dirty bit found on an unmapped block"));
-                }
-            }/*else if dirty_iterator.expect("dirty bitset unavailable").next() {
-                return Err(anyhow!("dirty bit found on an unmapped block"));
-            }*/
-        }
-
-        Ok(())
-    }
-
-    fn check_oblock(&self, m: &Mapping) -> anyhow::Result<()> {
-        if !m.is_valid() {
-            if m.oblock > 0 {
-                return Err(anyhow!("invalid mapped block"));
             }
-            return Ok(());
         }
 
-        if m.oblock >= self.nr_origin_blocks {
+        fn check_flags(&self, m: &Mapping) -> anyhow::Result<()> {
-            return Err(anyhow!("mapping beyond end of the origin device"));
+            if (m.flags & !(MappingFlags::Valid as u32 | MappingFlags::Dirty as u32)) != 0 {
+                return Err(anyhow!("unknown flags in mapping: {}", m.flags));
+            }
+            if !m.is_valid() && m.is_dirty() {
+                return Err(anyhow!("dirty bit found on an unmapped block"));
+            }
+            Ok(())
         }
 
-        let mut seen_oblocks = self.seen_oblocks.lock().unwrap();
+        fn check_oblock(&self, m: &Mapping) -> anyhow::Result<()> {
-        if seen_oblocks.contains(&m.oblock) {
+            if !m.is_valid() {
-            return Err(anyhow!("origin block already mapped"));
+                if m.oblock > 0 {
-        }
+                    return Err(anyhow!("invalid mapped block"));
-        seen_oblocks.insert(m.oblock);
+                }
+                return Ok(());
+            }
+            if m.oblock >= self.nr_origin_blocks {
+                return Err(anyhow!("mapping beyond end of the origin device"));
+            }
 
-        Ok(())
+            let mut seen_oblocks = self.seen_oblocks.lock().unwrap();
+            if seen_oblocks.contains(&m.oblock) {
+                return Err(anyhow!("origin block already mapped"));
+            }
+            seen_oblocks.insert(m.oblock);
+
+            Ok(())
+        }
+    }
+
+    impl ArrayVisitor<Mapping> for MappingChecker {
+        fn visit(&self, _index: u64, m: Mapping) -> anyhow::Result<()> {
+            self.check_flags(&m)?;
+            self.check_oblock(&m)?;
+
+            Ok(())
+        }
     }
 }
 
-impl ArrayVisitor<Mapping> for CheckMappingVisitor {
+mod format2 {
-    fn visit(&self, _index: u64, m: Mapping) -> anyhow::Result<()> {
+    use super::*;
-        self.check_flags(&m)?;
-        self.check_oblock(&m)?;
 
-        Ok(())
+    pub struct MappingChecker {
+        nr_origin_blocks: u64,
+        inner: Mutex<Inner>,
+    }
+
+    struct Inner {
+        seen_oblocks: BTreeSet<u64>,
+        dirty_bits: FixedBitSet,
+    }
+
+    impl MappingChecker {
+        pub fn new(nr_origin_blocks: Option<u64>, dirty_bits: FixedBitSet) -> MappingChecker {
+            MappingChecker {
+                nr_origin_blocks: if let Some(n) = nr_origin_blocks {n} else {MAX_ORIGIN_BLOCKS},
+                inner: Mutex::new(Inner {
+                    seen_oblocks: BTreeSet::new(),
+                    dirty_bits,
+                }),
+            }
+        }
+
+        fn check_flags(&self, m: &Mapping, dirty_bit: bool) -> anyhow::Result<()> {
+            if (m.flags & !(MappingFlags::Valid as u32)) != 0 {
+                return Err(anyhow!("unknown flags in mapping: {}", m.flags));
+            }
+            if !m.is_valid() && dirty_bit {
+                return Err(anyhow!("dirty bit found on an unmapped block"));
+            }
+            Ok(())
+        }
+
+        fn check_oblock(&self, m: &Mapping, seen_oblocks: &mut BTreeSet<u64>) -> anyhow::Result<()> {
+            if !m.is_valid() {
+                if m.oblock > 0 {
+                    return Err(anyhow!("invalid mapped block"));
+                }
+                return Ok(());
+            }
+            if m.oblock >= self.nr_origin_blocks {
+                return Err(anyhow!("mapping beyond end of the origin device"));
+            }
+            if seen_oblocks.contains(&m.oblock) {
+                return Err(anyhow!("origin block already mapped"));
+            }
+            seen_oblocks.insert(m.oblock);
+
+            Ok(())
+        }
+    }
+
+    impl ArrayVisitor<Mapping> for MappingChecker {
+        fn visit(&self, index: u64, m: Mapping) -> anyhow::Result<()> {
+            let mut inner = self.inner.lock().unwrap();
+            self.check_flags(&m, inner.dirty_bits.contains(index as usize))?;
+            self.check_oblock(&m, &mut inner.seen_oblocks)?;
+
+            Ok(())
+        }
     }
 }
 
 //------------------------------------------
 
-struct CheckHintVisitor;
+struct HintChecker;
 
-impl CheckHintVisitor {
+impl HintChecker {
-    fn new() -> CheckHintVisitor {
+    fn new() -> HintChecker {
-        CheckHintVisitor
+        HintChecker
     }
 }
 
-impl ArrayVisitor<Hint> for CheckHintVisitor {
+impl ArrayVisitor<Hint> for HintChecker {
     fn visit(&self, _index: u64, _hint: Hint) -> anyhow::Result<()> {
         // TODO: check hints
         Ok(())
@@ -171,11 +215,22 @@ pub fn check(opts: CacheCheckOptions) -> anyhow::Result<()> {
     // TODO: factor out into check_mappings()
     if !opts.skip_mappings {
         let w = ArrayWalker::new(engine.clone(), false);
-        let mut c = CheckMappingVisitor::new(sb.version, nr_origin_blocks, sb.dirty_root);
+        match sb.version {
-        w.walk(&mut c, sb.mapping_root)?;
+            1 => {
-
+                let mut c = format1::MappingChecker::new(nr_origin_blocks);
-        if sb.version >= 2 {
+                w.walk(&mut c, sb.mapping_root)?;
-            // TODO: check dirty bitset
+            }
+            2 => {
+                // FIXME: possibly size truncation on 32-bit targets
+                let mut dirty_bits = FixedBitSet::with_capacity(sb.cache_blocks as usize);
+                // TODO: allow ignore_none_fatal
+                read_bitset(engine.clone(), sb.dirty_root.unwrap(), false, &mut dirty_bits)?;
+                let mut c = format2::MappingChecker::new(nr_origin_blocks, dirty_bits);
+                w.walk(&mut c, sb.mapping_root)?;
+            }
+            v => {
+                return Err(anyhow!("unsupported metadata version {}", v));
+            }
         }
     }
 
@@ -184,12 +239,15 @@ pub fn check(opts: CacheCheckOptions) -> anyhow::Result<()> {
             return Err(anyhow!("cache_check only supports policy hint size of 4"));
         }
         let w = ArrayWalker::new(engine.clone(), false);
-        let mut c = CheckHintVisitor::new();
+        let mut c = HintChecker::new();
         w.walk(&mut c, sb.hint_root)?;
     }
 
-    if !opts.skip_discards {
+    // The discard bitset might not be available if the cache has never been suspended,
-        // TODO: check discard bitset
+    // e.g., a crash of freshly created cache.
+    if !opts.skip_discards && sb.discard_root != 0 {
+        let mut discard_bits = FixedBitSet::with_capacity(sb.discard_nr_blocks as usize);
+        read_bitset(engine.clone(), sb.discard_root, false, &mut discard_bits)?;
     }
 
     Ok(())

src/pdata/bitset_walker.rs

@@ -0,0 +1,51 @@
+use anyhow::{anyhow, Result};
+use fixedbitset::FixedBitSet;
+use std::sync::{Arc, Mutex};
+
+use crate::io_engine::IoEngine;
+use crate::pdata::array_walker::{ArrayVisitor, ArrayWalker};
+use crate::pdata::btree::{self};
+
+struct BitsetVisitor<'a> {
+    nr_entries: u64,
+    bits: Mutex<&'a mut FixedBitSet>,
+}
+
+impl<'a> BitsetVisitor<'a> {
+    pub fn new(bitset: &'a mut FixedBitSet) -> Self {
+        BitsetVisitor {
+            nr_entries: bitset.len() as u64,
+            bits: Mutex::new(bitset),
+        }
+    }
+}
+
+impl<'a> ArrayVisitor<u64> for BitsetVisitor<'a> {
+    fn visit(&self, index: u64, bits: u64) -> Result<()> {
+        let begin: u64 = index << 6;
+        if begin > self.nr_entries {
+            return Err(anyhow!("bitset size exceeds expectation"));
+        }
+
+        let end: u64 = std::cmp::min(begin + 64, self.nr_entries);
+        let mut mask = 1;
+        for i in begin..end {
+            self.bits.lock().unwrap().set(i as usize, bits & mask != 0);
+            mask <<= 1;
+        }
+        Ok(())
+    }
+}
+
+// TODO: remap errors
+// TODO: multi-threaded is possible
+pub fn read_bitset(
+    engine: Arc<dyn IoEngine + Send + Sync>,
+    root: u64,
+    ignore_none_fatal: bool,
+    bitset: &mut FixedBitSet,
+)-> btree::Result<()> {
+    let w = ArrayWalker::new(engine.clone(), ignore_none_fatal);
+    let mut v = BitsetVisitor::new(bitset);
+    w.walk(&mut v, root)
+}

src/pdata/mod.rs

@@ -1,5 +1,6 @@
 pub mod array_block;
 pub mod array_walker;
+pub mod bitset_walker;
 pub mod btree;
 pub mod btree_builder;
 pub mod btree_merge;