emo/thin-provisioning-tools
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[thin_check (rust)] Fix race in btree walking.

Browse Source 
The seen bitset was locked once to test, and separately to insert.
This commit is contained in:
Joe Thornber 2020-08-10 12:30:12 +01:00
parent 4e4b7ca2b1
commit b915257e10
1 changed files with 26 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/pdata/btree.rs
View File

@ -3,8 +3,8 @@ use fixedbitset::FixedBitSet;
use nom::{number::complete::*, IResult}; use nom::{number::complete::*, IResult};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use crate::io_engine::*;
use crate::checksum; use crate::checksum;
use crate::io_engine::*;
// FIXME: check that keys are in ascending order between nodes. // FIXME: check that keys are in ascending order between nodes.
@ -20,9 +20,7 @@ pub trait Unpack {
pub fn unpack<U: Unpack>(data: &[u8]) -> Result<U> { pub fn unpack<U: Unpack>(data: &[u8]) -> Result<U> {
match U::unpack(data) { match U::unpack(data) {
Err(_e) => { Err(_e) => Err(anyhow!("couldn't parse SMRoot")),
Err(anyhow!("couldn't parse SMRoot"))
},
Ok((_i, v)) => Ok(v), Ok((_i, v)) => Ok(v),
} }
} }
@ -222,16 +220,27 @@ impl BTreeWalker {
} }
} }
fn is_seen(&self, b: u64) -> bool {
let mut seen = self.seen.lock().unwrap();
if !seen[b as usize] {
seen.insert(b as usize);
return false;
}
true
}
fn walk_nodes<NV, V>(&mut self, visitor: &mut NV, bs: &[u64]) -> Result<()> fn walk_nodes<NV, V>(&mut self, visitor: &mut NV, bs: &[u64]) -> Result<()>
where where
NV: NodeVisitor<V>, NV: NodeVisitor<V>,
V: Unpack, V: Unpack,
{ {
let mut blocks = Vec::new(); let mut blocks = Vec::new();
let seen = self.seen.lock().unwrap(); let mut seen = self.seen.lock().unwrap();
for b in bs { for b in bs {
if !seen[*b as usize] { if !seen[*b as usize] {
blocks.push(Block::new(*b)); blocks.push(Block::new(*b));
seen.insert(*b as usize);
} }
} }
drop(seen); drop(seen);
@ -250,10 +259,6 @@ impl BTreeWalker {
NV: NodeVisitor<V>, NV: NodeVisitor<V>,
V: Unpack, V: Unpack,
{ {
let mut seen = self.seen.lock().unwrap();
seen.insert(b.loc as usize);
drop(seen);
let bt = checksum::metadata_block_type(b.get_data()); let bt = checksum::metadata_block_type(b.get_data());
if bt != checksum::BT::NODE { if bt != checksum::BT::NODE {
return Err(anyhow!("checksum failed for node {}, {:?}", b.loc, bt)); return Err(anyhow!("checksum failed for node {}, {:?}", b.loc, bt));
@ -279,7 +284,11 @@ impl BTreeWalker {
NV: NodeVisitor<V>, NV: NodeVisitor<V>,
V: Unpack, V: Unpack,
{ {
self.walk_node(visitor, &root, true) if self.is_seen(root.loc) {
Ok(())
} else {
self.walk_node(visitor, &root, true)
}
} }
pub fn walk<NV, V>(&mut self, visitor: &mut NV, root: u64) -> Result<()> pub fn walk<NV, V>(&mut self, visitor: &mut NV, root: u64) -> Result<()>
@ -287,9 +296,13 @@ impl BTreeWalker {
NV: NodeVisitor<V>, NV: NodeVisitor<V>,
V: Unpack, V: Unpack,
{ {
let mut root = Block::new(root); if self.is_seen(root) {
self.engine.read(&mut root)?; Ok(())
self.walk_node(visitor, &root, true) } else {
let mut root = Block::new(root);
self.engine.read(&mut root)?;
self.walk_node(visitor, &root, true)
}
} }
} }