diff --git a/bcachefs/src/alloc_bitmap.rs b/bcachefs/src/alloc_bitmap.rs index c6a5db186f9..af76e98260c 100644 --- a/bcachefs/src/alloc_bitmap.rs +++ b/bcachefs/src/alloc_bitmap.rs @@ -1,5 +1,9 @@ +use alloc::collections::BTreeMap; +use alloc::vec::Vec; + use crate::block_io::{BlockBuf, BlockNum, BlockIO, BlockIOExt, BLOCK_SIZE}; use crate::fs::FsError; +use crate::superblock::Superblock; const BITS_PER_BLOCK: u64 = (BLOCK_SIZE * 8) as u64; @@ -22,15 +26,200 @@ pub struct Run { /// /// The bitmap is stored on disk starting at `bitmap_start` and spanning /// `bitmap_blocks` blocks. Each bit represents one block: 1 = used, 0 = free. +/// +/// **With `shadow` on, no node the last committed tree reaches is written +/// over, and no block it reaches is handed out again, before the next commit +/// lands** (`Mounted::sync`): a btree node is written in place only when the +/// operation in progress took its block ([`Self::in_place`]), and a block +/// given up is free at once only if no commit has named it since it was taken +/// (`fresh`); any other waits in `pending` for the commit. An operation +/// ([`Self::begin`]) gives up its blocks only when it succeeds, and refused +/// gives back every one it took. +/// mkfs runs with `shadow` off: nothing it writes is committed until it ends. pub struct BitmapAllocator { pub bitmap_start: BlockNum, pub bitmap_blocks: u64, pub total_blocks: u64, pub free_blocks: u64, pub next_alloc: u64, // cursor — scan starts here, wraps once + pub shadow: bool, + /// Taken since the last commit's superblock was handed to the device. + fresh: Runs, + /// Given up, and reached by a tree a mount may still find. + pending: Vec<(u64, u32)>, + op: Option, +} + +/// Blocks only an operation that [`Reserve::Draw`]s may take while `shadow` +/// is on, so a full volume still copies the nodes a delete or a shrink +/// changes, as many as sixteen between two commits. +pub const NODE_RESERVE: u64 = 16; + +/// Whether an operation may take the blocks [`NODE_RESERVE`] keeps: only one +/// that leaves the tree no larger, so a commit gives back all it drew. +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum Reserve { + Keep, + Draw, +} + +/// The operation in progress: what it took, and what it gives up if it succeeds. +struct Op { + reserve: Reserve, + taken: Runs, + given: Vec<(u64, u32)>, +} + +/// Disjoint runs of blocks, by first block, to one past the last. +#[derive(Default)] +struct Runs(BTreeMap); + +impl Runs { + fn insert(&mut self, start: u64, len: u64) { + self.0.insert(start, start + len); + } + + fn contains(&self, block: u64) -> bool { + self.0.range(..=block).next_back().is_some_and(|(_, &end)| block < end) + } + + /// Take `block` out, answering whether it was in. + fn remove(&mut self, block: u64) -> bool { + let Some((&start, &end)) = self.0.range(..=block).next_back() else { return false }; + if block >= end { + return false; + } + self.0.remove(&start); + if start < block { + self.0.insert(start, block); + } + if block + 1 < end { + self.0.insert(block + 1, end); + } + true + } } impl BitmapAllocator { + /// The allocator of a mounted volume, its free count the last commit's. + pub fn open(sb: &Superblock) -> Self { + Self { + bitmap_start: sb.bitmap_start, + bitmap_blocks: sb.bitmap_blocks, + total_blocks: sb.block_count, + free_blocks: sb.free_blocks, + next_alloc: sb.next_alloc, + shadow: true, + fresh: Runs::default(), + pending: Vec::new(), + op: None, + } + } + + /// Read the free count off the bitmap: a stop after the last commit + /// leaves it holding fewer than the superblock counts. + pub fn count_free(&mut self, io: &dyn BlockIO) -> Result<(), FsError> { + let mut free_blocks = 0; + let mut buf = BlockBuf::zeroed(); + for i in 0..self.total_blocks.div_ceil(BITS_PER_BLOCK) { + io.read(BlockNum::new(self.bitmap_start.raw() + i), &mut buf)?; + let bits = (self.total_blocks - i * BITS_PER_BLOCK).min(BITS_PER_BLOCK); + let (whole, rest) = buf.0[..bits.div_ceil(8) as usize].split_at((bits / 8) as usize); + free_blocks += whole.iter().map(|b| b.count_zeros() as u64).sum::(); + free_blocks += rest.first().map_or(0, |b| (!b & ((1u8 << (bits % 8)) - 1)).count_ones() as u64); + } + self.free_blocks = free_blocks; + Ok(()) + } + + /// Begin one operation on the tree, which [`Self::succeed`] or + /// [`Self::fail`] ends. + pub fn begin(&mut self, reserve: Reserve) { + let op = Op { reserve, taken: Runs::default(), given: Vec::new() }; + assert!(self.op.replace(op).is_none(), "an operation began inside another"); + } + + /// What an allocation may take now: every free block, but for those + /// [`NODE_RESERVE`] keeps from all but an operation that draws on it. + fn spare(&self) -> u64 { + match !self.shadow || self.op.as_ref().is_some_and(|op| op.reserve == Reserve::Draw) { + true => self.free_blocks, + false => self.free_blocks.saturating_sub(NODE_RESERVE), + } + } + + /// Whether a node at `block` may be written where it is. + pub fn in_place(&self, block: BlockNum) -> bool { + !self.shadow || self.op.as_ref().is_some_and(|op| op.taken.contains(block.raw())) + } + + /// The operation took effect: what it gave up is given up now. A bit the + /// device would not clear is a leaked block, and no reason to call an + /// operation that took effect refused. + pub fn succeed(&mut self, io: &dyn BlockIO) { + let op = self.op.take().expect("an operation in progress"); + for (start, count) in op.given { + let _ = self.give(io, start, count); + } + } + + /// The operation was refused: every block it took is free again, and what + /// it would have given up stays its owner's. A bit the device would not + /// clear is a leaked block, and the refusal already in hand is the answer. + pub fn fail(&mut self, io: &dyn BlockIO) { + let op = self.op.take().expect("an operation in progress"); + for (start, end) in op.taken.0 { + for block in start..end { + self.fresh.remove(block); + let _ = self.set_free(io, BlockNum::new(block)); + } + } + } + + /// A superblock naming the tree as it stands may land from here on, so + /// nothing taken so far is free at once when given up. + pub fn seal(&mut self) { + self.fresh = Runs::default(); + } + + /// Blocks given up that a commit landing makes free. + pub fn pending(&self) -> u64 { + self.pending.iter().map(|&(_, count)| count as u64).sum() + } + + /// A commit landed: the blocks only an older tree reached are free. + pub fn committed(&mut self, io: &dyn BlockIO) -> Result<(), FsError> { + while let Some((start, count)) = self.pending.pop() { + for i in 0..count { + if let Err(e) = self.set_free(io, BlockNum::new(start + i as u64)) { + self.pending.push((start + i as u64, count - i)); + return Err(e); + } + } + } + Ok(()) + } + + /// Give up `count` blocks from `start`: free now if no commit has named + /// them, and otherwise once the next one lands. A block whose bit the + /// device would not clear is leaked, and the rest are given up still. + fn give(&mut self, io: &dyn BlockIO, start: u64, count: u32) -> Result<(), FsError> { + let mut refused = Ok(()); + for block in start..start + count as u64 { + if !self.shadow || self.fresh.remove(block) { + if let Err(e) = self.set_free(io, BlockNum::new(block)) { + refused = refused.and(Err(e)); + } + } else { + match self.pending.last_mut() { + Some((at, n)) if *at + *n as u64 == block => *n += 1, + _ => self.pending.push((block, 1)), + } + } + } + refused + } + /// Where a block's bit lives, and which bit of that byte it is. fn bit_of(&self, block: BlockNum) -> (BlockNum, usize, u8) { let byte_idx = block.raw() / 8; @@ -96,8 +285,12 @@ impl BitmapAllocator { /// The run is never empty and may be shorter than asked for, so every /// caller has to loop or has to be wrong. pub fn alloc_up_to(&mut self, io: &dyn BlockIO, from: u64, wanted: u32) -> Result { + let spare = self.spare(); + if spare == 0 { + return Err(FsError::NoSpace { requested: wanted, available: 0 }); + } // A zero-length run would let a caller's loop spin without progress. - let wanted = wanted.max(1); + let wanted = wanted.max(1).min(u32::try_from(spare).unwrap_or(u32::MAX)); let (start, len) = self.longest_free_run(io, from % self.total_blocks, wanted)?; self.reserve(io, start, len.min(wanted)) } @@ -105,6 +298,10 @@ impl BitmapAllocator { /// Reserve all of `count` or nothing, for callers that cannot place a /// short run. Nothing is marked used unless the whole run is there. pub fn alloc_exact(&mut self, io: &dyn BlockIO, count: u32) -> Result { + let spare = self.spare(); + if spare < count as u64 { + return Err(FsError::NoSpace { requested: count, available: spare }); + } let (start, len) = self.longest_free_run(io, self.next_alloc, count)?; if len < count { return Err(FsError::NoSpace { @@ -123,6 +320,10 @@ impl BitmapAllocator { fn reserve(&mut self, io: &dyn BlockIO, start: u64, len: u32) -> Result { let start_block = BlockNum::new(start); self.set_range_used(io, start_block, len as u64)?; + self.fresh.insert(start, len as u64); + if let Some(op) = &mut self.op { + op.taken.insert(start, len as u64); + } self.free_blocks -= len as u64; self.next_alloc = start + len as u64; if self.next_alloc >= self.total_blocks { @@ -216,17 +417,21 @@ impl BitmapAllocator { Ok((start, best_count)) } - /// Free a contiguous range of blocks. + /// Give up a contiguous range of blocks: inside an operation, once it + /// succeeds. pub fn free_range( &mut self, io: &dyn BlockIO, start: BlockNum, count: u32, ) -> Result<(), FsError> { - for i in 0..count as u64 { - self.set_free(io, BlockNum::new(start.raw() + i))?; + match &mut self.op { + Some(op) => { + op.given.push((start.raw(), count)); + Ok(()) + } + None => self.give(io, start.raw(), count), } - Ok(()) } /// Initialize bitmap on disk: zero all bitmap blocks, then mark metadata blocks as used. @@ -248,6 +453,10 @@ impl BitmapAllocator { total_blocks, free_blocks: total_blocks - metadata_blocks, next_alloc: metadata_blocks, + shadow: false, + fresh: Runs::default(), + pending: Vec::new(), + op: None, }; // Metadata blocks (superblock, bitmap, journal area), and the backup diff --git a/bcachefs/src/btree.rs b/bcachefs/src/btree.rs index 59e6ba3d8cc..5a616feb14d 100644 --- a/bcachefs/src/btree.rs +++ b/bcachefs/src/btree.rs @@ -365,15 +365,24 @@ fn write_entry(b: &mut [u8; BLOCK_SIZE], offset: usize, key: &Key, value: &[u8]) /// child's subtree, so the answer is the last child whose key is `<= key`, /// defaulting to the first — which covers everything below the second key. fn find_child(children: &[Child], key: &Key) -> Option { - let mut chosen = children.first()?.block; - for child in children { - if child.key <= *key { - chosen = child.block; - } else { - break; - } + children.get(child_index(children, key)).map(|c| c.block) +} + +/// [`find_child`]'s answer as an index into `children`. +fn child_index(children: &[Child], key: &Key) -> usize { + children.iter().take_while(|c| c.key <= *key).count().saturating_sub(1) +} + +/// The block a node the operation in progress changed is written to: its +/// own where [`BitmapAllocator::in_place`] allows, and otherwise a new one, +/// the old given up. +fn own(io: &dyn BlockIO, alloc: &mut BitmapAllocator, block: BlockNum) -> Result { + if alloc.in_place(block) { + return Ok(block); } - Some(chosen) + let moved = alloc.alloc_block(io)?; + alloc.free_range(io, block, 1)?; + Ok(moved) } /// Search the B+ tree for an exact key match. Returns the leaf entry's value. @@ -434,27 +443,48 @@ pub fn search_by_hash( } } -/// Delete an exact key from the B+ tree. Returns the old value if found. +/// Delete an exact key from the B+ tree: the root after it and the old value, +/// or `None` with nothing written when no entry has the key. /// Does not merge underflowing nodes — just removes the entry from the leaf. -pub fn delete(io: &dyn BlockIO, root: BlockNum, key: &Key) -> Result>, FsError> { - let mut block = root; - let mut depth = Depth::ROOT; +pub fn delete( + io: &dyn BlockIO, + alloc: &mut BitmapAllocator, + root: BlockNum, + key: &Key, +) -> Result)>, FsError> { + delete_recursive(io, alloc, root, Depth::ROOT, key) +} - loop { - match Node::read(io, block)? { - Node::Leaf(mut entries) => { - let Some(pos) = entries.iter().position(|e| e.key == *key) else { - return Ok(None); - }; - let old = entries.remove(pos); - Node::Leaf(entries).write(io, block)?; - return Ok(Some(old.value)); - } - Node::Interior { children, .. } => { - let next = find_child(&children, key).ok_or(FsError::CorruptedNode(block))?; - depth = depth.descend(block)?; - block = next; +fn delete_recursive( + io: &dyn BlockIO, + alloc: &mut BitmapAllocator, + block: BlockNum, + depth: Depth, + key: &Key, +) -> Result)>, FsError> { + match Node::read(io, block)? { + Node::Leaf(mut entries) => { + let Some(pos) = entries.iter().position(|e| e.key == *key) else { + return Ok(None); + }; + let old = entries.remove(pos); + let at = own(io, alloc, block)?; + Node::Leaf(entries).write(io, at)?; + Ok(Some((at, old.value))) + } + Node::Interior { level, mut children } => { + let idx = child_index(&children, key); + let child = children.get(idx).ok_or(FsError::CorruptedNode(block))?.block; + let Some((moved, old)) = delete_recursive(io, alloc, child, depth.descend(block)?, key)? else { + return Ok(None); + }; + if moved == child { + return Ok(Some((block, old))); } + children[idx].block = moved; + let at = own(io, alloc, block)?; + Node::Interior { level, children }.write(io, at)?; + Ok(Some((at, old))) } } } @@ -499,7 +529,8 @@ fn walk_recursive( /// Insert a key-value pair into the B+ tree. /// -/// Returns the root block, which changes when the old root was split. +/// Returns the root block, which changes when the old root was split or +/// written somewhere new. pub fn insert( io: &dyn BlockIO, alloc: &mut BitmapAllocator, @@ -508,29 +539,29 @@ pub fn insert( ) -> Result { check_entry_fits(&entry)?; - match insert_recursive(io, alloc, root, Depth::ROOT, entry)? { - InsertResult::Done => Ok(root), - InsertResult::Split(siblings) => { - let level = Node::read(io, root)? - .level() - .checked_add(1) - .ok_or(FsError::CorruptedNode(root))?; - let old_min_key = min_key(io, root, Depth::ROOT)?; - let new_root_block = alloc.alloc_block(io)?; - - let mut children = alloc::vec![Child { key: old_min_key, block: root }]; - children.extend(siblings); - Node::Interior { level, children }.write(io, new_root_block)?; - - Ok(new_root_block) - } + let Placed { at, split } = insert_recursive(io, alloc, root, Depth::ROOT, entry)?; + if split.is_empty() { + return Ok(at); } + let level = Node::read(io, at)? + .level() + .checked_add(1) + .ok_or(FsError::CorruptedNode(at))?; + let old_min_key = min_key(io, at, Depth::ROOT)?; + let new_root_block = alloc.alloc_block(io)?; + + let mut children = alloc::vec![Child { key: old_min_key, block: at }]; + children.extend(split); + Node::Interior { level, children }.write(io, new_root_block)?; + + Ok(new_root_block) } -enum InsertResult { - Done, - /// The node split: these follow it, in key order. - Split(Vec), +/// Where an insert left a node: its block, and the siblings it split off, in +/// key order. +struct Placed { + at: BlockNum, + split: Vec, } fn insert_recursive( @@ -539,7 +570,7 @@ fn insert_recursive( block: BlockNum, depth: Depth, entry: Entry, -) -> Result { +) -> Result { match Node::read(io, block)? { Node::Leaf(mut entries) => { match entries.binary_search_by(|e| e.key.cmp(&entry.key)) { @@ -548,32 +579,24 @@ fn insert_recursive( } write_or_split(io, alloc, block, Node::Leaf(entries)) } - Node::Interior { level, children } => { - let mut idx = 0; - for (i, child) in children.iter().enumerate() { - if child.key <= entry.key { - idx = i; - } else { - break; - } - } + Node::Interior { level, mut children } => { + let idx = child_index(&children, &entry.key); let child_block = children.get(idx).ok_or(FsError::CorruptedNode(block))?.block; let deeper = depth.descend(block)?; - match insert_recursive(io, alloc, child_block, deeper, entry)? { - InsertResult::Done => Ok(InsertResult::Done), - InsertResult::Split(siblings) => { - let mut children = children; - for sibling in siblings { - let pos = match children.binary_search_by(|c| c.key.cmp(&sibling.key)) { - Ok(i) => i + 1, - Err(i) => i, - }; - children.insert(pos, sibling); - } - write_or_split(io, alloc, block, Node::Interior { level, children }) - } + let Placed { at, split } = insert_recursive(io, alloc, child_block, deeper, entry)?; + if at == child_block && split.is_empty() { + return Ok(Placed { at: block, split }); + } + children[idx].block = at; + for sibling in split { + let pos = match children.binary_search_by(|c| c.key.cmp(&sibling.key)) { + Ok(i) => i + 1, + Err(i) => i, + }; + children.insert(pos, sibling); } + write_or_split(io, alloc, block, Node::Interior { level, children }) } } } @@ -583,20 +606,23 @@ fn write_or_split( alloc: &mut BitmapAllocator, block: BlockNum, node: Node, -) -> Result { +) -> Result { + let at = own(io, alloc, block)?; if NODE_HEADER_SIZE + node.payload_size() <= BLOCK_SIZE { - node.write(io, block)?; - return Ok(InsertResult::Done); + node.write(io, at)?; + return Ok(Placed { at, split: Vec::new() }); } - split_node(io, alloc, block, node) + Ok(Placed { at, split: split_node(io, alloc, at, node)? }) } +/// Split `node` across `block` and new siblings, answering the siblings. The +/// blocks it took are the operation's, which gives them back if it fails. fn split_node( io: &dyn BlockIO, alloc: &mut BitmapAllocator, block: BlockNum, node: Node, -) -> Result { +) -> Result, FsError> { match node { Node::Leaf(entries) => { // One entry is not a split problem. Halving by *count* used to @@ -610,28 +636,15 @@ fn split_node( let mut nodes = pack(entries); let first = nodes.remove(0); - let mut blocks = Vec::with_capacity(nodes.len()); - for _ in &nodes { - match alloc.alloc_block(io) { - Ok(sibling) => blocks.push(sibling), - Err(e) => { - for taken in blocks { - alloc.free_range(io, taken, 1)?; - } - return Err(e); - } - } - } - // The siblings first: a failure before `block` is replaced leaves - // the tree as it was and blocks nothing names. let mut children = Vec::with_capacity(nodes.len()); - for (node, sibling) in nodes.into_iter().zip(blocks) { + for node in nodes { + let sibling = alloc.alloc_block(io)?; children.push(Child { key: node[0].key, block: sibling }); Node::Leaf(node).write(io, sibling)?; } Node::Leaf(first).write(io, block)?; - Ok(InsertResult::Split(children)) + Ok(children) } Node::Interior { level, mut children } => { if children.len() < 2 { @@ -649,7 +662,7 @@ fn split_node( Node::Interior { level, children }.write(io, block)?; Node::Interior { level, children: right }.write(io, right_block)?; - Ok(InsertResult::Split(alloc::vec![Child { key: split_key, block: right_block }])) + Ok(alloc::vec![Child { key: split_key, block: right_block }]) } } } @@ -806,9 +819,8 @@ mod tests { ); } - /// A split that finds no block for a later sibling gives back the ones it - /// took for the earlier: nothing names them yet, so nothing else frees - /// them. + /// A split that finds no block for a later sibling fails its operation, + /// which gives back the block it took for the earlier: nothing names it. #[test] fn a_split_short_of_a_sibling_gives_back_the_ones_it_took() { let io = crate::block_io::VecBlockIO::new(16); @@ -821,10 +833,13 @@ mod tests { let mut middle: Vec = (0..40).map(|_| entry(72)).collect(); middle.insert(20, entry(MAX_ENTRY_SIZE - KEY_HEADER_SIZE)); assert_eq!(pack(middle.clone()).len(), 3, "two siblings, and a block for one"); + alloc.begin(crate::alloc_bitmap::Reserve::Keep); assert!(matches!( split_node(&io, &mut alloc, leaf, Node::Leaf(middle)), Err(FsError::NoSpace { .. }), )); + assert_eq!(alloc.free_blocks, 0, "the first sibling took the last block"); + alloc.fail(&io); assert_eq!(alloc.free_blocks, 1, "the first sibling's block went back"); } diff --git a/bcachefs/src/fs.rs b/bcachefs/src/fs.rs index e83a49d17e3..28355c73d84 100644 --- a/bcachefs/src/fs.rs +++ b/bcachefs/src/fs.rs @@ -4,7 +4,7 @@ use alloc::vec::Vec; use core::marker::PhantomData; use core::ops::ControlFlow; -use crate::alloc_bitmap::BitmapAllocator; +use crate::alloc_bitmap::{BitmapAllocator, Reserve}; use crate::block_io::{BlockBuf, BlockNum, BlockIO, BlockIOExt, DeviceError, BLOCK_SIZE}; use crate::btree::{self, Entry, Key, KeyType, Node}; use crate::superblock::{FsUuid, Superblock}; @@ -139,12 +139,10 @@ impl FsError { pub struct ReadOnly; pub struct ReadWrite; -/// A formatted but not yet mounted filesystem. Used for building images (mkfs). -pub struct Formatted { - io: IO, - sb: Superblock, - alloc: BitmapAllocator, -} +/// A formatted but not yet mounted filesystem. Used for building images +/// (mkfs): written in place, since nothing it holds is committed before +/// [`Formatted::into_io`]. +pub struct Formatted(Mounted); /// A mounted filesystem. Mode is ReadOnly or ReadWrite. pub struct Mounted { @@ -412,9 +410,8 @@ fn decode_leaf_value(value: &[u8], volume_blocks: u64) -> Result 0 { - let run = match alloc.alloc_up_to(io, alloc.next_alloc, remaining) { - Ok(run) => run, - Err(err) => return Err(give_back(io, alloc, &extents, err)), - }; + let run = alloc.alloc_up_to(io, alloc.next_alloc, remaining)?; push_extent(&mut extents, run.start.raw(), run.len); let mut buf = BlockBuf::zeroed(); @@ -444,9 +438,7 @@ fn write_data( let len = chunk_end - data_offset; buf.0[..len].copy_from_slice(&data[data_offset..chunk_end]); } - if let Err(err) = io.write(BlockNum::new(run.start.raw() + i), &buf) { - return Err(give_back(io, alloc, &extents, err)); - } + io.write(BlockNum::new(run.start.raw() + i), &buf)?; data_offset += BLOCK_SIZE; } @@ -456,24 +448,6 @@ fn write_data( Ok(extents) } -/// Hand back the runs a failed [`write_data`] had already reserved, and return -/// the failure that stopped it. -/// -/// Best effort by construction: this runs because something has already gone -/// wrong, and a bitmap write that also fails has no better answer to give than -/// the error already in hand. -fn give_back( - io: &dyn BlockIO, - alloc: &mut BitmapAllocator, - extents: &[Extent], - err: FsError, -) -> FsError { - for ext in extents { - let _ = alloc.free_range(io, BlockNum::new(ext.start_block), ext.block_count); - } - err -} - /// Read file data from a list of extents. /// /// **`size` sizes the `Vec` and `size` is a number off the disk**, so it is @@ -575,106 +549,71 @@ impl Formatted { sb.write(&io)?; - Ok(Self { io, sb, alloc }) + Ok(Self(Mounted { io, sb, alloc, _mode: PhantomData })) } /// Name this filesystem, so a role's kernel argument can select it. /// /// A separate act from formatting: a volume nothing names is legal, and - /// nothing here invents a name for one. Persisted by [`Self::sync`], which - /// [`Self::into_io`] runs. + /// nothing here invents a name for one. Persisted by [`Self::into_io`]. pub fn set_uuid(&mut self, uuid: FsUuid) { - self.sb.uuid = uuid; + self.0.sb.uuid = uuid; } /// Create a file on the formatted filesystem (used during mkfs). pub fn create(&mut self, name: &str, data: &[u8], mtime: u64) -> Result<(), FsError> { - if name.is_empty() || name.len() > MAX_NAME_LEN { - return Err(FsError::NameTooLong { len: name.len(), max: MAX_NAME_LEN }); - } - - let extents = write_data(&self.io, &mut self.alloc, data)?; - let value = encode_leaf_value(KeyType::File, name, data.len() as u64, mtime, &extents); - let key = make_key(&self.sb.hash_seed, name, KeyType::File); - let entry = Entry { key, value }; - - self.sb.root_node = btree::insert(&self.io, &mut self.alloc, self.sb.root_node, entry)?; - - Ok(()) + self.0.create(name, data, mtime) } /// Create a symlink on the formatted filesystem. pub fn create_symlink(&mut self, name: &str, target: &str, mtime: u64) -> Result<(), FsError> { - if name.is_empty() || name.len() > MAX_NAME_LEN { - return Err(FsError::NameTooLong { len: name.len(), max: MAX_NAME_LEN }); - } - - let target_bytes = target.as_bytes(); - let extents = write_data(&self.io, &mut self.alloc, target_bytes)?; - let value = encode_leaf_value(KeyType::Symlink, name, target_bytes.len() as u64, mtime, &extents); - let key = make_key(&self.sb.hash_seed, name, KeyType::Symlink); - let entry = Entry { key, value }; - - self.sb.root_node = btree::insert(&self.io, &mut self.alloc, self.sb.root_node, entry)?; - - Ok(()) - } - - /// Finalize the filesystem: write superblock with clean flag. - pub fn sync(&mut self) -> Result<(), FsError> { - self.sb.free_blocks = self.alloc.free_blocks; - self.sb.next_alloc = self.alloc.next_alloc; - self.sb.set_clean(true); - self.sb.write(&self.io)?; - self.io.flush() + self.0.put(name, KeyType::Symlink, target.as_bytes(), mtime) } /// Mount this formatted filesystem for read-write access. pub fn mount(self) -> Mounted { - Mounted { - io: self.io, - sb: self.sb, - alloc: self.alloc, - _mode: PhantomData, - } + let mut fs = self.0; + fs.alloc.shadow = true; + fs } /// Mount this formatted filesystem for read-only access. pub fn mount_readonly(self) -> Mounted { - Mounted { - io: self.io, - sb: self.sb, - alloc: self.alloc, - _mode: PhantomData, - } + let Mounted { io, sb, alloc, .. } = self.0; + Mounted { io, sb, alloc, _mode: PhantomData } } /// Consume and return the underlying IO (for extracting the image bytes). pub fn into_io(mut self) -> Result { - self.sync()?; - Ok(self.io) + self.0.sync()?; + Ok(self.0.io) } } // --- Mounted (read operations, available for both ReadOnly and ReadWrite) --- +impl Mounted { + /// Open an existing filesystem from disk, to read. + pub fn open(io: IO) -> Result { + Self::read_superblock(io) + } +} + +impl Mounted { + /// Open an existing filesystem from disk, to change; a mount that only + /// reads never asks how much is free. + pub fn open(io: IO) -> Result { + let mut fs = Self::read_superblock(io)?; + fs.alloc.count_free(&fs.io)?; + Ok(fs) + } +} + impl Mounted { - /// Open an existing filesystem from disk. - pub fn open(io: IO) -> Result, FsError> { + fn read_superblock(io: IO) -> Result { let sb = Superblock::read(&io)?; - let alloc = BitmapAllocator { - bitmap_start: sb.bitmap_start, - bitmap_blocks: sb.bitmap_blocks, - total_blocks: sb.block_count, - free_blocks: sb.free_blocks, - next_alloc: sb.next_alloc, - }; - Ok(Mounted { - io, - sb, - alloc, - _mode: PhantomData, - }) + let alloc = BitmapAllocator::open(&sb); + Ok(Mounted { io, sb, alloc, _mode: PhantomData }) } /// The device this filesystem was opened over. @@ -803,11 +742,9 @@ impl Mounted { /// Convert back to Formatted state (for testing — insert more files after reading). pub fn into_formatted(self) -> Formatted { - Formatted { - io: self.io, - sb: self.sb, - alloc: self.alloc, - } + let Self { io, sb, mut alloc, .. } = self; + alloc.shadow = false; + Formatted(Mounted { io, sb, alloc, _mode: PhantomData }) } /// Return the extents and file size for a file. @@ -825,6 +762,28 @@ impl Mounted { // --- ReadWrite-only operations --- +/// **Every change is one operation and every sync one commit.** An operation +/// ([`Mounted::atomic`]) takes effect whole or, refused anywhere, leaves the +/// tree and the allocator as they were; it never writes over a node the last +/// commit's tree reaches, but to new blocks up to a new root. A commit +/// ([`Mounted::sync`]) is both superblock copies naming that root landing on +/// the device after everything the root reaches. A superblock's bytes lie in +/// its first 512-byte sector, which a device writes whole, so killed anywhere +/// each copy names the last commit's tree whole or the new one whole. +/// +/// That is true of names, and of each entry's length and extents; not of a +/// file's bytes. A page written over where the file already holds it +/// ([`Mounted::resolve_or_alloc_block`]) is written in place, so a kill can +/// leave a file's committed blocks holding pages from both sides of a write +/// (`issues/a-write-over-a-files-committed-page-is-not-shadowed.md`). What a +/// kill costs besides is the blocks taken since the last commit and those only +/// the old tree reached: marked used, and named by no tree, until a sweep +/// (`issues/a-crash-leaks-the-blocks-of-its-last-commit.md`). +/// +/// A delete, and an update whose entry grows no longer, may take the blocks +/// [`crate::alloc_bitmap::NODE_RESERVE`] keeps from every other change, so a +/// full volume still shrinks, by as many committed nodes as the reserve holds +/// between two commits (`issues/a-full-data-volume-frees-space-only-at-a-commit.md`). impl Mounted { /// Create a file, replacing whatever answered to `name`. pub fn create(&mut self, name: &str, data: &[u8], mtime: u64) -> Result<(), FsError> { @@ -836,6 +795,28 @@ impl Mounted { self.put(name, KeyType::Symlink, target.as_bytes(), 0) } + /// Run `change` as one operation: whole, or refused with the root and + /// every block as they were. + fn atomic( + &mut self, + reserve: Reserve, + change: impl FnOnce(&mut Self) -> Result, + ) -> Result { + let root = self.sb.root_node; + self.alloc.begin(reserve); + match change(self) { + Ok(done) => { + self.alloc.succeed(&self.io); + Ok(done) + } + Err(e) => { + self.sb.root_node = root; + self.alloc.fail(&self.io); + Err(e) + } + } + } + /// Put `name` on the volume, displacing whatever answered to it. /// /// The new entry goes in before the old one comes out, for the reason @@ -860,21 +841,28 @@ impl Mounted { return Err(FsError::NameTooLong { len: name.len(), max: MAX_NAME_LEN }); } - let displaced = match self.find_by_name(name)? { - Some((key, value)) => Some((key, self.decode(&value)?.extents().to_vec())), - None => None, - }; + self.atomic(Reserve::Keep, |fs| { + let displaced = match fs.find_by_name(name)? { + Some((key, value)) => Some((key, fs.decode(&value)?.extents().to_vec())), + None => None, + }; - let extents = write_data(&self.io, &mut self.alloc, data)?; - let value = encode_leaf_value(key_type, name, data.len() as u64, mtime, &extents); - let key = make_key(&self.sb.hash_seed, name, key_type); - self.sb.root_node = btree::insert( - &self.io, &mut self.alloc, - self.sb.root_node, - Entry { key, value }, - )?; + let extents = write_data(&fs.io, &mut fs.alloc, data)?; + let value = encode_leaf_value(key_type, name, data.len() as u64, mtime, &extents); + let key = make_key(&fs.sb.hash_seed, name, key_type); + fs.sb.root_node = btree::insert(&fs.io, &mut fs.alloc, fs.sb.root_node, Entry { key, value })?; - self.retire_displaced(displaced, key) + fs.retire_displaced(displaced, key) + }) + } + + /// Remove `key`'s entry, if the tree has one. + fn remove(&mut self, key: &Key) -> Result { + let Some((root, _)) = btree::delete(&self.io, &mut self.alloc, self.sb.root_node, key)? else { + return Ok(false); + }; + self.sb.root_node = root; + Ok(true) } /// Remove the entry the insert of `new_key` did not replace, and free the @@ -890,26 +878,9 @@ impl Mounted { ) -> Result<(), FsError> { let Some((old_key, old_extents)) = displaced else { return Ok(()) }; if old_key != new_key { - btree::delete(&self.io, self.sb.root_node, &old_key)?; - } - for ext in &old_extents { - self.alloc.free_range(&self.io, BlockNum::new(ext.start_block), ext.block_count)?; + self.remove(&old_key)?; } - Ok(()) - } - - /// Delete a file or symlink by name. Returns true if found and deleted. - pub fn delete(&mut self, name: &str) -> Result { - self.delete_by_name(name) - } - - /// Sync filesystem state to disk. - pub fn sync(&mut self) -> Result<(), FsError> { - self.sb.free_blocks = self.alloc.free_blocks; - self.sb.next_alloc = self.alloc.next_alloc; - self.sb.set_clean(true); - self.sb.write(&self.io)?; - self.io.flush() + self.free_extents(&old_extents) } /// Delete a file/symlink by name, freeing its data blocks. Returns true if found. @@ -921,32 +892,55 @@ impl Mounted { /// caller nothing had happened. It also answers it once for both key /// types, where the old shape fell through from File to Symlink after a /// non-matching removal and could take two entries out in one call. - fn delete_by_name(&mut self, name: &str) -> Result { - let Some((key, value)) = self.find_by_name(name)? else { return Ok(false) }; - let extents = self.decode(&value)?.extents().to_vec(); - - // `find_by_name` reached this key by the descent `btree::delete` is - // about to repeat, so an empty removal is not "no such file" — it is a - // tree that answers two ways. - if btree::delete(&self.io, self.sb.root_node, &key)?.is_none() { - return Err(FsError::CorruptedNode(self.sb.root_node)); - } - for ext in &extents { - self.alloc.free_range(&self.io, BlockNum::new(ext.start_block), ext.block_count)?; - } - Ok(true) + pub fn delete(&mut self, name: &str) -> Result { + self.atomic(Reserve::Draw, |fs| { + let Some((key, value)) = fs.find_by_name(name)? else { return Ok(false) }; + let extents = fs.decode(&value)?.extents().to_vec(); + + // `find_by_name` reached this key by the descent `btree::delete` is + // about to repeat, so an empty removal is not "no such file" — it is a + // tree that answers two ways. + if !fs.remove(&key)? { + return Err(FsError::CorruptedNode(fs.sb.root_node)); + } + fs.free_extents(&extents)?; + Ok(true) + }) + } + + /// Commit the tree as it stands: see this block's header. + pub fn sync(&mut self) -> Result<(), FsError> { + self.io.flush()?; + // What the volume holds free once this commit's own frees are made. + self.sb.free_blocks = self.alloc.free_blocks + self.alloc.pending(); + self.sb.next_alloc = self.alloc.next_alloc; + self.sb.set_clean(true); + // From the first superblock write on, this commit may be what a mount + // finds, even where the write is refused. + self.alloc.seal(); + self.sb.write(&self.io)?; + self.io.flush()?; + self.alloc.committed(&self.io) } /// Rename a file or symlink. - /// - /// The new entry goes in before the old one comes out, so a crash between - /// the two leaves the file under both names rather than under neither. What - /// that ordering costs is that the insert *is* the removal of whatever - /// `new_name` named — same name and same type is the same key, and - /// `btree::insert` replaces on an equal key — so the displaced entry has to - /// be read out of the tree before the insert. Asking for it afterwards, by - /// name, answers with the file that was just renamed and frees its extents. pub fn rename(&mut self, old_name: &str, new_name: &str) -> Result<(), FsError> { + self.rename_all(&[(old_name, new_name)]) + } + + /// Rename every `(old, new)` pair in turn, as one operation: a directory + /// is the names beneath it, and moves whole or not at all. + pub fn rename_all(&mut self, renames: &[(&str, &str)]) -> Result<(), FsError> { + self.atomic(Reserve::Keep, |fs| renames.iter().try_for_each(|&(old, new)| fs.rename_one(old, new))) + } + + /// The new entry goes in before the old one comes out. What that ordering + /// costs is that the insert *is* the removal of whatever `new_name` named — + /// same name and same type is the same key, and `btree::insert` replaces on + /// an equal key — so the displaced entry has to be read out of the tree + /// before the insert. Asking for it afterwards, by name, answers with the + /// file that was just renamed and frees its extents. + fn rename_one(&mut self, old_name: &str, new_name: &str) -> Result<(), FsError> { // Every other name-taking entry point bounds its name; this one did // not, and `user_ptr::MAX_USER_STR` lets 64 KiB of it through. if new_name.is_empty() || new_name.len() > MAX_NAME_LEN { @@ -982,7 +976,7 @@ impl Mounted { // extent list. Nothing to delete when the two names share a key — the // entry under it is the one the insert just wrote. if new_key != old_key { - btree::delete(&self.io, self.sb.root_node, &old_key)?; + self.remove(&old_key)?; } Ok(()) @@ -996,29 +990,27 @@ impl Mounted { size: u64, mtime: u64, ) -> Result<(), FsError> { - let (old_key, old_value) = self.find_by_name(name)? - .ok_or(FsError::NotFound)?; + let (old_key, old_value) = self.find_by_name(name)?.ok_or(FsError::NotFound)?; let leaf = self.decode(&old_value)?; - let new_value = encode_leaf_value(old_key.key_type, leaf.name(), size, mtime, new_extents); + // An entry no longer than the one it replaces splits no node. + let reserve = match new_value.len() <= old_value.len() { + true => Reserve::Draw, + false => Reserve::Keep, + }; let new_entry = Entry { key: old_key, value: new_value }; - // No delete first. The key is unchanged and `btree::insert` replaces on - // an equal key, so the delete bought nothing and cost the file: a - // pre-check for `EntryTooLarge` does not cover `insert`'s other - // rejection, a split with no free block to split into, and that one - // left the entry deleted and never put back. Blocks the caller drops - // from the extent list are the caller's to free, through - // [`Self::free_extents`], after this records the shortened list. - self.sb.root_node = btree::insert( - &self.io, &mut self.alloc, - self.sb.root_node, - new_entry, - )?; - Ok(()) + self.atomic(reserve, |fs| { + // No delete first: the key is unchanged and `btree::insert` replaces + // on an equal key. Blocks the caller drops from the extent list are + // the caller's to free, through [`Self::free_extents`], after this + // records the shortened list. + fs.sb.root_node = btree::insert(&fs.io, &mut fs.alloc, fs.sb.root_node, new_entry)?; + Ok(()) + }) } - /// Return `extents`' blocks to the allocator. Record first, free second: + /// Give `extents`' blocks back to the allocator. Record first, free second: /// the caller shortens the entry's list before calling this, so a failure /// between the two leaks blocks rather than leaving an entry naming freed /// ones. diff --git a/bcachefs/tests/crash.rs b/bcachefs/tests/crash.rs new file mode 100644 index 00000000000..357794bc8f1 --- /dev/null +++ b/bcachefs/tests/crash.rs @@ -0,0 +1,269 @@ +//! A directory rename, changes after it and the sync that commits them, +//! stopped at every block write: the crash-point model of the commit. +//! +//! **A device keeps what a flush covered and any part of what it did not, a +//! 512-byte sector at a time.** The writes since the last flush may land in +//! any order, so each stop is tried with every write of the open epoch before +//! it landed and with none of them; and the stop's write itself whole, or +//! torn at a sector boundary: its first sectors new and the rest old, or its +//! last. What the device then holds must mount, keep every name beside the +//! directory, and name the directory whole under exactly one of its two +//! names; and so must it with block 0 lost, from the backup superblock. + +use std::cell::RefCell; +use std::collections::BTreeMap; + +use bcachefs::{BlockBuf, BlockIO, BlockNum, DeviceError, Formatted, FsError, Mounted, ReadOnly, ReadWrite, Superblock, TransferError, VecBlockIO}; + +const BLOCKS: u64 = 512; +const BLOCK: usize = 4096; +const SECTOR: usize = 512; +const STAGED: &str = "staged"; +const INSTALLED: &str = "apps/pkg"; + +/// What the device was asked, in order: a block written, or a flush. +enum Event { + Write(u64, Box<[u8; BLOCK]>), + Flush, +} + +/// The device as the filesystem sees it, every write in place at once, with +/// the order it was asked in kept beside it; and the write numbered `refuse`, +/// refused. +struct Logged { + image: RefCell>, + log: RefCell>, + writes: RefCell, + refuse: Option, +} + +impl Logged { + fn new(image: &[u8], refuse: Option) -> Self { + Self { image: RefCell::new(image.to_vec()), log: RefCell::default(), writes: RefCell::default(), refuse } + } +} + +struct Refused; +impl TransferError for Refused { + fn refused_before_attempt(&self) -> bool { + false + } +} + +impl BlockIO for Logged { + fn read_block(&self, block: BlockNum, buf: &mut BlockBuf) -> Result<(), DeviceError> { + let at = block.raw() as usize * BLOCK; + buf.0.copy_from_slice(&self.image.borrow()[at..at + BLOCK]); + Ok(()) + } + + fn write_block(&self, block: BlockNum, buf: &BlockBuf) -> Result<(), DeviceError> { + let n = self.writes.replace_with(|n| *n + 1); + if Some(n) == self.refuse { + return Err(DeviceError::classify(&Refused)); + } + self.log.borrow_mut().push(Event::Write(block.raw(), Box::new(buf.0))); + let at = block.raw() as usize * BLOCK; + self.image.borrow_mut()[at..at + BLOCK].copy_from_slice(&buf.0); + Ok(()) + } + + fn block_count(&self) -> u64 { + BLOCKS + } + + fn sync(&self) -> Result<(), DeviceError> { + self.log.borrow_mut().push(Event::Flush); + Ok(()) + } +} + +fn file(i: usize) -> String { + if i.is_multiple_of(4) { format!("{STAGED}/sub/f{i}") } else { format!("{STAGED}/f{i}") } +} + +/// A committed volume: names beside the directory, and the directory with +/// files enough to span leaves, a subdirectory and an empty one. +fn staged() -> Vec { + let mut fs = Formatted::format(VecBlockIO::new(BLOCKS)).expect("format"); + for i in 0..80 { + fs.create(&format!("beside{i}"), format!("beside {i}").as_bytes(), 1).expect("beside"); + } + fs.create(&format!("{STAGED}/"), b"", 1).expect("the directory's own entry"); + fs.create(&format!("{STAGED}/empty/"), b"", 1).expect("an empty directory"); + for i in 0..60 { + fs.create(&file(i), format!("file {i} of the package").as_bytes(), 1).expect("staged"); + } + fs.into_io().expect("commit").into_vec() +} + +/// Every name `keep` accepts, with its bytes, or what would not read. +fn names(fs: &Mounted, keep: &dyn Fn(&str) -> bool) -> Result>, String> { + let listed = fs.list(usize::MAX, keep).map_err(|e| format!("the list: {e:?}"))?; + listed + .into_iter() + .map(|(name, _)| fs.read_file(&name).map(|bytes| (name.clone(), bytes)).map_err(|e| format!("{name}: {e:?}"))) + .collect() +} + +/// The directory's own entry and every name beneath it, by what follows `dir`. +fn tree(fs: &Mounted, dir: &str) -> Result>, String> { + let beneath = format!("{dir}/"); + let all = names(fs, &|n| n.starts_with(&beneath))?; + Ok(all.into_iter().map(|(name, bytes)| (name[dir.len()..].to_string(), bytes)).collect()) +} + +fn beside(fs: &Mounted) -> Result>, String> { + names(fs, &|n| n.starts_with("beside")) +} + +/// The rename: every name beneath the directory, and its own. +fn rename(fs: &mut Mounted) -> Result<(), FsError> { + let moves: Vec<(String, String)> = tree(fs, STAGED) + .expect("the directory reads") + .into_keys() + .map(|rest| (format!("{STAGED}{rest}"), format!("{INSTALLED}{rest}"))) + .collect(); + let pairs: Vec<(&str, &str)> = moves.iter().map(|(a, b)| (a.as_str(), b.as_str())).collect(); + fs.rename_all(&pairs) +} + +/// What a volume must hold: the directory's names and those beside it. +struct Want { + tree: BTreeMap>, + beside: BTreeMap>, +} + +impl Want { + fn of(image: &[u8]) -> Self { + let fs = reopened(image.to_vec()).expect("the staged volume mounts"); + Self { tree: tree(&fs, STAGED).unwrap(), beside: beside(&fs).unwrap() } + } + + /// Where the directory is: `Ok(true)` whole under its new name, + /// `Ok(false)` whole under its old, and otherwise what was found. + fn whole_under_one(&self, fs: &Mounted) -> Result { + let kept = beside(fs)?; + if kept != self.beside { + return Err(format!("{} of {} names beside the directory", kept.len(), self.beside.len())); + } + let (old, new) = (tree(fs, STAGED)?, tree(fs, INSTALLED)?); + match (old.is_empty(), new.is_empty()) { + (false, true) if old == self.tree => Ok(false), + (true, false) if new == self.tree => Ok(true), + _ => Err(format!("{} names under {STAGED} and {} under {INSTALLED}", old.len(), new.len())), + } + } + + fn held(&self, image: Vec) -> Result { + reopened(image).and_then(|fs| self.whole_under_one(&fs)) + } +} + +fn reopened(image: Vec) -> Result, String> { + Mounted::<_, ReadOnly>::open(VecBlockIO::from_vec(image)).map_err(|e| format!("unmountable: {e:?}")) +} + +#[test] +fn a_directory_rename_is_whole_under_one_name_wherever_the_device_stops() { + let image = staged(); + let want = Want::of(&image); + assert!(want.tree.len() > 60, "the directory holds {} names", want.tree.len()); + + let mut fs = Mounted::<_, ReadWrite>::open(Logged::new(&image, None)).expect("mount"); + rename(&mut fs).expect("the rename"); + // Changes after it, which take the blocks the rename gave up if they + // are handed out before the commit lands. + for i in 0..40 { + fs.create(&format!("after{i}"), &[7; BLOCK], 2).expect("a change after it"); + } + fs.sync().expect("the commit"); + assert_eq!(want.held(fs.io().image.borrow().clone()), Ok(true)); + let log = fs.io().log.replace(Vec::new()); + + let writes: Vec = (0..log.len()).filter(|&i| matches!(log[i], Event::Write(..))).collect(); + assert!(writes.len() > 10, "the run wrote {} blocks", writes.len()); + let sb = Superblock::read(&VecBlockIO::from_vec(image.clone())).expect("the staged superblock"); + let in_place: Vec = [0, BLOCKS - 1] + .into_iter() + .chain(sb.bitmap_start.raw()..sb.bitmap_start.raw() + sb.bitmap_blocks) + .collect(); + let sectors = BLOCK / SECTOR; + let parts: Vec<(String, std::ops::Range)> = std::iter::once(("whole".to_string(), 0..sectors)) + .chain((1..sectors).flat_map(|k| [(format!("its first {k} sectors"), 0..k), (format!("its last {k} sectors"), sectors - k..sectors)])) + .collect(); + let (mut torn, mut tried) = (Vec::new(), 0); + for (k, &stop) in writes.iter().enumerate() { + let epoch = log[..stop].iter().rposition(|e| matches!(e, Event::Flush)).map_or(0, |f| f + 1); + let Event::Write(block, data) = &log[stop] else { unreachable!("a stop is a write") }; + for (shape, before) in [("the epoch's writes before it landed", epoch..stop), ("alone", stop..stop)] { + let mut landed = image.clone(); + for event in log[..epoch].iter().chain(&log[before]) { + if let Event::Write(block, data) = event { + let at = *block as usize * BLOCK; + landed[at..at + BLOCK].copy_from_slice(&data[..]); + } + } + let at = *block as usize * BLOCK; + let old = landed[at..at + BLOCK].to_vec(); + for (part, range) in &parts { + let mut held = landed.clone(); + held[at + range.start * SECTOR..at + range.end * SECTOR] + .copy_from_slice(&data[range.start * SECTOR..range.end * SECTOR]); + // A tear only of a block the layout writes in place, and only + // where it leaves bytes neither the old nor the new did. + let new = &held[at..at + BLOCK]; + if range.len() < sectors && (!in_place.contains(block) || new == &old[..] || new == &data[..]) { + continue; + } + let mut lost = held.clone(); + lost[..BLOCK].fill(0); + for (copy, image) in [("", held), (", block 0 lost", lost)] { + tried += 1; + if let Err(why) = want.held(image) { + torn.push(format!("stopped at write {k} of {} (block {block}), {shape}, {part}{copy}: {why}", writes.len())); + } + } + } + } + } + assert!(torn.is_empty(), "{} of {tried} stops tear the volume:\n{}", torn.len(), torn.join("\n")); +} + +/// The same with the one write numbered `n` refused, the filesystem alive +/// after it: the rename's answer is what it serves, the device holds the +/// directory whole under one name past the refused sync, and the next sync +/// makes the answer what the device holds. +#[test] +fn a_directory_rename_is_whole_under_one_name_whichever_write_is_refused() { + let image = staged(); + let want = Want::of(&image); + let writes = { + let mut fs = Mounted::<_, ReadWrite>::open(Logged::new(&image, None)).expect("mount"); + rename(&mut fs).expect("the rename"); + fs.sync().expect("the commit"); + let n = *fs.io().writes.borrow(); + n + }; + + let mut torn = Vec::new(); + for refuse in 0..writes { + let mut fs = Mounted::<_, ReadWrite>::open(Logged::new(&image, Some(refuse))).expect("mount"); + let renamed = rename(&mut fs).is_ok(); + let served = want.whole_under_one(&fs); + let first = fs.sync(); + // Changes past a refused sync, which take the blocks of the tree it + // may have committed if they are handed out before the next lands. + let changed = (0..40).try_for_each(|i| fs.create(&format!("between{i}"), &[7; 9000], 2)); + let between = want.held(fs.io().image.borrow().clone()); + let second = fs.sync(); + let held = want.held(fs.io().image.borrow().clone()); + if served != Ok(renamed) || changed.is_err() || between.is_err() || second.is_err() || held != Ok(renamed) { + torn.push(format!( + "write {refuse} of {writes} refused: renamed {renamed}, served {served:?}, \ + the sync {first:?}, the changes past it {changed:?}, left {between:?}, the next {second:?} left {held:?}" + )); + } + } + assert!(torn.is_empty(), "{} of {writes} refusals tear the directory:\n{}", torn.len(), torn.join("\n")); +} diff --git a/bcachefs/tests/integration.rs b/bcachefs/tests/integration.rs index 838c7071256..cc4fb2273c1 100644 --- a/bcachefs/tests/integration.rs +++ b/bcachefs/tests/integration.rs @@ -707,7 +707,7 @@ fn a_rename_carries_the_file_to_the_new_name() { fn a_renamed_file_keeps_every_extent_it_had() { // A file in one run survives a rename that reallocates as readily as one // that carries the extents. Discontiguous runs do not. - let (mut fs, _) = one_block_holes(64); + let (mut fs, _) = one_block_holes(128); let data: Vec = (0..4 * 4096 + 11).map(|i| (i % 251) as u8).collect(); fs.create("frag.bin", &data, 3).expect("create a fragmented file"); let (before, _) = fs.file_extents("frag.bin").expect("file_extents").expect("extents"); @@ -837,40 +837,39 @@ fn entries_of_mixed_size_survive_node_splits() { // --- A short allocation is not the allocation that was asked for --- +/// The blocks every change but a shrink leaves free: the crate's `NODE_RESERVE`. +const NODE_RESERVE: u32 = 16; + /// A volume whose free space is nothing but one-block holes, so the allocator -/// can only ever report a run shorter than a multi-block request. +/// can only ever report a run shorter than a multi-block request: every block +/// a file's data may take, taken in one run, every other one given back, and +/// the run the reserve kept taken after. /// -/// Returns the surviving files' single data blocks alongside it: a block the -/// allocator did *not* hand out is the ground truth for "this write landed on -/// somebody else's file". -fn one_block_holes(blocks: u64) -> (Mounted, Vec<(String, u64)>) { +/// Returns the blocks still taken alongside it: a block the allocator did +/// *not* hand out is the ground truth for "this write landed on somebody +/// else's file". +fn one_block_holes(blocks: u64) -> (Mounted, Vec) { let mut fs = Formatted::format(VecBlockIO::new(blocks)).expect("format").mount(); - let mut made = Vec::new(); - for i in 0..blocks { - let name = format!("f{i:03}"); - if fs.create(&name, &vec![0xAAu8; 4096], 0).is_err() { - break; - } - made.push(name); + let mut run = Vec::new(); + let mut page = 0; + while fs.resolve_or_alloc_block(&mut run, page).is_ok() { + page += 1; } - assert!(made.len() > 8, "volume too small to fragment: {} files", made.len()); - for (i, name) in made.iter().enumerate() { - if i % 2 == 0 { - assert!(fs.delete(name).expect("delete"), "delete {name}"); - } - } - let survivors = made - .iter() - .enumerate() - .filter(|(i, _)| i % 2 == 1) - .map(|(_, n)| { - let (extents, _) = fs.file_extents(n).expect("file_extents").expect("a survivor kept its extents"); - assert_eq!(extents.len(), 1); - assert_eq!(extents[0].block_count, 1); - (n.clone(), extents[0].start_block) - }) - .collect(); - (fs, survivors) + let taken: Vec = run.iter().flat_map(|e| e.start_block..e.start_block + e.block_count as u64).collect(); + assert!(taken.len() > 8, "volume too small to fragment: {} blocks", taken.len()); + let holes: Vec = + taken.iter().step_by(2).map(|&b| Extent { start_block: b, block_count: 1, _reserved: 0 }).collect(); + fs.free_extents(&holes).expect("give every other block back"); + let mut tail = vec![Extent { start_block: *taken.last().unwrap(), block_count: 1, _reserved: 0 }]; + fs.resolve_or_alloc_block(&mut tail, NODE_RESERVE).expect("the run left for nodes"); + + let sb = bcachefs::Superblock::read(fs.io()).expect("the superblock"); + let mut bitmap = bcachefs::BlockBuf::zeroed(); + bcachefs::BlockIO::read_block(fs.io(), sb.bitmap_start, &mut bitmap).expect("the bitmap"); + let free = |b: u64| bitmap.0[b as usize / 8] & (1 << (b % 8)) == 0; + let run = (0..blocks - 1).find(|&b| free(b) && free(b + 1)); + assert_eq!(run, None, "a free run longer than one block: this proves nothing"); + (fs, taken.into_iter().skip(1).step_by(2).collect()) } #[test] @@ -891,11 +890,7 @@ fn a_sparse_write_resolves_inside_the_blocks_it_reserved() { reserved.contains(&block), "page 3 resolved to block {block}, outside the extents it recorded: {extents:?}", ); - assert!( - !survivors.iter().any(|(_, b)| *b == block), - "page 3 resolved to block {block}, which belongs to {:?}", - survivors.iter().find(|(_, b)| *b == block).map(|(n, _)| n), - ); + assert!(!survivors.contains(&block), "page 3 resolved to block {block}, which was already taken"); } #[test] @@ -923,9 +918,11 @@ fn every_page_of_a_fragmented_file_owns_a_distinct_block() { // --- Write-path ordering: an operation that fails must not have destroyed // what it was asked to replace, nor kept what it took. --- -/// Blocks a 64-block volume has to give: everything but the superblock, the -/// bitmap, the root node and the backup superblock. -const FREE_BLOCKS_64: usize = 60; +/// One-block files a fresh 64-block volume takes: its 60 free blocks, less +/// the 16 a create leaves free, less the formatted root, which the first +/// change copies and frees only at a commit, less the leaf's copy the last +/// create holds beside the one it replaces. +const ONE_BLOCK_FILES_64: usize = 60 - NODE_RESERVE as usize - 2; fn small_volume() -> Mounted { Formatted::format(VecBlockIO::new(64)).expect("format").mount() @@ -934,7 +931,7 @@ fn small_volume() -> Mounted { /// How many one-block files this volume still has room for. fn one_block_files_that_fit(fs: &mut Mounted) -> usize { let mut fitted = 0; - for i in 0..FREE_BLOCKS_64 * 2 { + for i in 0..ONE_BLOCK_FILES_64 * 2 { let name = format!("p{:03}", i); if fs.create(&name, b"x", 0).is_err() { break; @@ -971,7 +968,7 @@ fn a_write_that_runs_out_of_space_gives_back_what_it_took() { let mut fresh = small_volume(); let untouched = one_block_files_that_fit(&mut fresh); assert_eq!( - untouched, FREE_BLOCKS_64, + untouched, ONE_BLOCK_FILES_64, "the baseline is wrong, so the comparison below proves nothing", ); @@ -989,12 +986,16 @@ fn a_write_that_runs_out_of_space_gives_back_what_it_took() { #[test] fn a_metadata_update_that_cannot_be_reinserted_leaves_the_entry_alone() { let mut fs = small_volume(); - // Every free block spent, and the root leaf filled to within one entry's - // growth of a split — so the reinsert has to split and the split has no - // block to split into. - for i in 0..FREE_BLOCKS_64 { + // Every block a file's data may take spent, then the rest on the nodes + // of names with no data, until one more needs a block the volume does + // not have — so the reinsert has no block to write to. + for i in 0..ONE_BLOCK_FILES_64 { fs.create(&format!("f{:02}", i), b"x", 100 + i as u64).expect("fill"); } + let mut names = 0; + while fs.create(&format!("empty{names}"), b"", 0).is_ok() { + names += 1; + } let (extents, _) = fs.file_extents("f00").expect("file_extents").expect("f00 is on the volume"); let grown: Vec = (0..16).map(|_| extents[0]).collect(); @@ -1014,6 +1015,103 @@ fn a_metadata_update_that_cannot_be_reinserted_leaves_the_entry_alone() { assert_eq!(fs.file_mtime("f00").expect("mtime").unwrap_or(0), 100, "the failed update left its mtime behind"); } +/// Fill `fs` with names enough to span leaves, then with one-block files, +/// then with names with none until a create right after a commit is refused: +/// how many files it took. +fn filled_to_the_last_name(fs: &mut Mounted) -> usize { + for i in 0..150 { + fs.create(&format!("spread{i:03}"), b"", 0).expect("a name with no data"); + if i % 50 == 49 { + fs.sync().expect("a commit"); + } + } + let mut files = 0; + while fs.create(&format!("f{files:02}"), b"x", 0).is_ok() { + files += 1; + } + let mut names = 0; + loop { + fs.sync().expect("a commit"); + let before = names; + while fs.create(&format!("empty{names}"), b"", 0).is_ok() { + names += 1; + } + if names == before { + break; + } + } + assert!(files > 4 && names > 0, "{files} files and {names} empty names filled the volume"); + assert!(fs.create("after", b"x", 0).is_err(), "the volume is full"); + files +} + +/// Shrink every one of `files` to nothing, then delete each, on a volume +/// filled to its last name; then, after a commit, take a file again. +fn shrinks_and_deletes_and_takes_again(fs: &mut Mounted, files: usize) { + for i in 0..files { + let name = format!("f{i:02}"); + let (extents, _) = fs.file_extents(&name).expect("file_extents").expect("the file is on the volume"); + fs.update_metadata(&name, &[], 0, 1).expect("a shrink of a full volume's file"); + fs.free_extents(&extents).expect("the shrink's free"); + } + for i in 0..files { + assert!(matches!(fs.delete(&format!("f{i:02}")), Ok(true)), "the delete of f{i:02}"); + } + fs.sync().expect("a commit"); + fs.create("after", b"x", 0).expect("a file in what the deletes gave back"); +} + +/// A volume filled to its last name still shrinks and deletes every file, and +/// after the next commit takes a file again. +#[test] +fn a_volume_full_of_empty_names_still_deletes_and_frees() { + let mut fs = Formatted::format(VecBlockIO::new(128)).expect("format").mount(); + let files = filled_to_the_last_name(&mut fs); + shrinks_and_deletes_and_takes_again(&mut fs, files); +} + +/// Data grown outside any operation takes no block of the reserve, even +/// where every free block lies in one run after the file's end, so the entry +/// that records the run does not grow and draws on the reserve itself. +#[test] +fn a_file_grown_past_what_the_volume_spares_leaves_the_reserve() { + let mut mkfs = Formatted::format(VecBlockIO::new(128)).expect("format"); + mkfs.create("f00", b"x", 0).expect("a one-block file"); + let mut fs = mkfs.mount(); + let (mut extents, _) = fs.file_extents("f00").expect("file_extents").expect("f00 is on the volume"); + + let grown = fs.resolve_or_alloc_block(&mut extents, 128); + assert!(matches!(grown, Err(FsError::NoSpace { .. })), "a page past every block of the volume: {grown:?}"); + assert_eq!(extents.len(), 1, "the run is not contiguous with the file, so this proves nothing: {extents:?}"); + let blocks = extents[0].block_count as u64; + fs.update_metadata("f00", &extents, blocks * 4096, 1).expect("the run recorded"); + fs.sync().expect("a commit"); + + let free = bcachefs::Superblock::read(fs.io()).expect("the superblock").free_blocks; + assert!(free >= NODE_RESERVE as u64, "the commit holds {free} blocks free, fewer than the reserve"); + shrinks_and_deletes_and_takes_again(&mut fs, 1); +} + +/// A volume stopped between two commits holds more blocks used than its +/// superblock counts free; a read-write mount counts its bitmap, so what the +/// stop leaked is no block a shrink or a delete is promised. +#[test] +fn a_mount_after_a_stop_counts_the_blocks_its_bitmap_holds() { + let mut fs = Formatted::format(VecBlockIO::new(128)).expect("format").mount(); + fs.sync().expect("a commit"); + fs.create("lost", &[1; 20 * 4096], 0).expect("taken and never committed"); + let image = (0..128).flat_map(|b| { + let mut buf = bcachefs::BlockBuf::zeroed(); + bcachefs::BlockIO::read_block(fs.io(), BlockNum::new(b), &mut buf).expect("read"); + buf.as_bytes().to_vec() + }); + let mut fs = Mounted::<_, ReadWrite>::open(VecBlockIO::from_vec(image.collect())).expect("the stopped volume mounts"); + assert!(matches!(fs.read_file("lost"), Err(FsError::NotFound)), "the stop came before the commit"); + + let files = filled_to_the_last_name(&mut fs); + shrinks_and_deletes_and_takes_again(&mut fs, files); +} + // --- The device error channel: a block the device would not give back is not // a block of zeros. --- diff --git a/issues/a-btree-split-leaves-nodes-holding-a-few-entries.md b/issues/a-btree-split-leaves-nodes-holding-a-few-entries.md new file mode 100644 index 00000000000..1add39df1d9 --- /dev/null +++ b/issues/a-btree-split-leaves-nodes-holding-a-few-entries.md @@ -0,0 +1,25 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# A btree split leaves nodes holding a few entries + +`bcachefs/src/btree.rs`'s `split_node` packs a leaf that overflows into as +few nodes as hold it (`pack`), each filled in key order: a full node and a +node holding what is left over, often one entry. Keys are hashes of names, so +the next insert lands in the full node more often than not and splits it +again. The tree grows by about a node every few entries where a node holds +dozens. + +Measured with `main`'s crate at 198a9d38e (a scratch binary, not committed): +300 names with no data created on a 1024-block volume, one sync after each. +The first 72 fit one leaf, 4 blocks used; the 228 after them took 93 more +blocks, about 2.5 entries a node. Every DATA create, every `mkdir` and every +`/state/` pays it, and ROOT's mkfs as well. + +## Exit condition + +A host test that creates 300 names one at a time finds the volume's nodes, +leaves and interior, holding on average at least half what a node can. diff --git a/issues/a-crash-leaks-the-blocks-of-its-last-commit.md b/issues/a-crash-leaks-the-blocks-of-its-last-commit.md new file mode 100644 index 00000000000..20f7bd96675 --- /dev/null +++ b/issues/a-crash-leaks-the-blocks-of-its-last-commit.md @@ -0,0 +1,30 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# A crash leaks the blocks taken since the last commit + +The DATA volume commits by shadow paging (`bcachefs/src/fs.rs`, the read-write +block's header): no node the last commit's tree reaches is written over, and +the allocator's bitmap is written in place. So a server killed or a machine +stopped between two commits leaves every block taken since the last one marked +used in the bitmap and named by no tree, and the blocks only the older tree +reached, which a commit that landed was about to free, the same. A block whose +bit the device refused to clear when it was given up (`BitmapAllocator::give`, +`succeed`, `fail`) stays marked used the same way, with nothing said. Nothing +finds them again: a read-write mount counts the bitmap as the device holds it +(`BitmapAllocator::count_free`), so the volume is smaller by every such block. + +Measured with `bcachefs/tests/crash.rs`'s run (a 512-block volume, a +directory of 62 names renamed, 40 one-block files written, then a commit) +stopped after every write before the first superblock write: the volume mounts +as it was, with 284 blocks marked used where it had 190. + +## Exit condition + +A mount after any stop, and a volume after any refused write, holds a bitmap +whose used blocks are exactly those the committed tree reaches and the +superblock and bitmap's own, measured by `bcachefs/tests/crash.rs`'s stops and +refusals. diff --git a/issues/a-directory-rename-on-data-is-not-atomic.md b/issues/a-directory-rename-on-data-is-not-atomic.md deleted file mode 100644 index 457e55f9211..00000000000 --- a/issues/a-directory-rename-on-data-is-not-atomic.md +++ /dev/null @@ -1,32 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-09 ---- - -# A directory rename on DATA is not atomic - -Fileserver's. `userland/fileserver/src/data.rs`'s `rename` of a directory -renames each entry under it one at a time, because the format keys every file -by its whole path and has no rename of a prefix; its own comment says a kill -in the middle leaves the directory in two halves. A refused write does the -same without a kill: a host test of `DataVolume` (scratch, not committed) made -`home/staged/a` (5 bytes) and `home/staged/z` (240 pages on a fragmented -volume), and renamed `home/staged` to a 300-byte name. `rename` answered -`ResourceExhausted` (the format's `EntryTooLarge { size: 4192, max: 4064 }` -for `z`), and the volume then held `/a` and `home/staged/z`: half the -directory under each name, and the call reported as failed. The format keeps -no journal either (`issues/bcachefs-crate-is-not-bcachefs.md`), so even one -entry's rename is only as whole as the sync that writes it. - -**What it blocks.** The package track's stage-then-commit -(`issues/a-package-is-a-directory-under-apps-and-the-installer-is-a-program.md`): -`/system/bin/pkg` stages `/apps/` privately and commits it in one step, -which a rename that can leave half a package under `/apps` is not. - -## Exit condition - -A directory rename on DATA leaves the directory whole under exactly one of its -names whatever write is refused and wherever the server is killed, measured by -a test that refuses every write of the rename in turn and kills the server at -every one. diff --git a/issues/a-full-data-volume-frees-space-only-at-a-commit.md b/issues/a-full-data-volume-frees-space-only-at-a-commit.md new file mode 100644 index 00000000000..5af330624a9 --- /dev/null +++ b/issues/a-full-data-volume-frees-space-only-at-a-commit.md @@ -0,0 +1,35 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# A full DATA volume frees space only at a commit + +The DATA volume's commit is shadow paging (`bcachefs/src/fs.rs`), so a block +the last committed tree reaches is not free until the next commit lands +(`BitmapAllocator`'s `pending`): an unlink or a truncate of a file a sync +already wrote gives its blocks back only at the next sync, and every change +copies the nodes it touches to new blocks first. On a volume its files filled, +a write that follows an unlink is refused `ResourceExhausted` until fileserver's +next sync, at most `WRITEBACK` later. + +`NODE_RESERVE` (16 blocks) is kept from every change but a delete and an +update whose entry grows no longer, which leave the tree no larger, so a +commit gives back whatever they drew. Its limits: the deletes and shrinks +between two commits on a full volume copy at most 16 committed nodes between +them, and a run that touches more is refused until the next sync; and a tree +deeper than 16 levels could not copy one delete's path at all. Nothing reads +DATA's depth: 16 is a bound picked, not one measured against it. + +Measured on a 1024-block `DataVolume` filled with 841 one-page files and +synced (a scratch host test, not committed), before the reserve was kept from +names with no data: a one-page write after an unlink was refused +`ResourceExhausted` and accepted after a sync; then 13 unlinks of every seventh +file were accepted and the 14th refused `ResourceExhausted`. + +## Exit condition + +On a volume its files filled, an unlink followed by a write of the blocks it +freed is accepted with no sync between them, and so is a delete of every file +on it, measured by a host test of `fileserver::data`. diff --git a/issues/a-package-is-a-directory-under-apps-and-the-installer-is-a-program.md b/issues/a-package-is-a-directory-under-apps-and-the-installer-is-a-program.md index f402d90e001..e889cd1fddb 100644 --- a/issues/a-package-is-a-directory-under-apps-and-the-installer-is-a-program.md +++ b/issues/a-package-is-a-directory-under-apps-and-the-installer-is-a-program.md @@ -97,8 +97,7 @@ The storage track's users and mount-protocol stages do not block this one. 4. The signed repository. Landed: the verifier and the publisher, on the host. Owed: `pkg install ` from a mirror list whose one kind is a local directory, the pinned root and its floors under `/system/etc/pkg/`, - the machine's under `/state/pkg`, and the commit by rename, which waits on - `issues/a-directory-rename-on-data-is-not-atomic.md`. + the machine's under `/state/pkg`, and the commit by rename. 5. The users track's per-user `/home` (`issues/a-user-is-a-home-tree-and-a-login-row.md`) decides where a package's own data goes. Until then it goes in its own folder of diff --git a/issues/a-write-over-a-files-committed-page-is-not-shadowed.md b/issues/a-write-over-a-files-committed-page-is-not-shadowed.md new file mode 100644 index 00000000000..d46282fd152 --- /dev/null +++ b/issues/a-write-over-a-files-committed-page-is-not-shadowed.md @@ -0,0 +1,26 @@ +--- +status: open +kind: defect +opened: 2026-10-09 +--- + +# A write over a file's committed page is not shadowed + +DATA's commit (`bcachefs/src/fs.rs`, the read-write block's header) holds +every name, and each entry's length and extents, to one sync or the other, +and not a file's bytes. Fileserver's `write` (`userland/fileserver/src/data.rs`) +resolves a page the file's extents already reach to the block they name +(`Mounted::resolve_or_alloc_block`) and writes the page there, through the +cache, which may write the block out before the next sync. So a kill can leave +a file's committed blocks holding some pages of a write and the rest from +before it, under whichever length the committed entry names. + +Read from the code, not measured: `bcachefs/tests/crash.rs` and fileserver's +kill tests overwrite no page a sync already committed. + +## Exit condition + +A write over a page a sync committed lands in a block no committed entry +names, and a host test that overwrites committed pages and stops the device at +every write of that write and its sync finds each file's bytes as one sync or +the other. diff --git a/userland/fileserver/src/data.rs b/userland/fileserver/src/data.rs index 1fdbf82260c..3fe12928e23 100644 --- a/userland/fileserver/src/data.rs +++ b/userland/fileserver/src/data.rs @@ -20,11 +20,13 @@ //! shrinks is recorded before the blocks past it are freed, so a failure //! between the two leaks blocks rather than leaving an entry naming freed ones. //! -//! **What a kill costs.** The format updates its btree in place and keeps no -//! journal (`issues/bcachefs-crate-is-not-bcachefs.md`): what the disk -//! holds is what the last sync wrote, and a server that dies inside a sync can -//! leave a node half of that sync's. Nothing but a sync writes a dirty block, -//! unless the cache is holding more than it keeps. +//! **What a kill costs.** A sync is the format's commit (`bcachefs`'s +//! `Mounted::sync`): a server killed anywhere leaves every name, and every +//! file's length and extents, as the last sync that finished or as the one it +//! was inside, whole either way; a file's bytes are not so held, since a page +//! written over is written in place. A directory's rename is one operation of +//! the format's (`Mounted::rename_all`): it moves whole, or is refused with +//! nothing moved. //! //! Every name a client chose is bounded by the format ([`FsError::NameTooLong`]) //! before it reaches the tree. @@ -687,38 +689,42 @@ impl Volume for DataVolume { if to.starts_with(&format!("{from}/")) { return Err(SyscallError::InvalidArgument); } - // One entry at a time: the format has no rename of a prefix, - // so a kill in the middle leaves the directory in two halves, - // every entry under exactly one of its names. + // Every entry beneath, and the directory's own, in one + // operation of the format's: the directory moves whole or not at all. let prefix = format!("{from}/"); let moving: Vec<(String, Kind)> = self .names - .range(prefix.clone()..) - .take_while(|(n, _)| n.starts_with(&prefix)) + .get_key_value(from) + .into_iter() + .chain(self.names.range(prefix.clone()..).take_while(|(n, _)| n.starts_with(&prefix))) .map(|(n, k)| (n.clone(), *k)) .collect(); - for (name, kind) in &moving { - let moved = format!("{to}/{}", &name[prefix.len()..]); + for (name, _) in &moving { if let Some(node) = self.by_path.get(name).copied() { self.persist(node)?; } - let (on_disk_from, on_disk_to) = match kind { - Kind::Dir => (format!("{name}/"), format!("{moved}/")), - _ => (name.clone(), moved.clone()), - }; - mapped("rename", name, self.fs.rename(&on_disk_from, &on_disk_to))?; - self.names.remove(name); - self.names.insert(moved.clone(), *kind); - if let Some(node) = self.by_path.remove(name) { + } + let renames: Vec<(String, String)> = moving + .iter() + .map(|(name, kind)| { + let moved = format!("{to}{}", &name[from.len()..]); + match kind { + Kind::Dir => (format!("{name}/"), format!("{moved}/")), + _ => (name.clone(), moved), + } + }) + .collect(); + let pairs: Vec<(&str, &str)> = renames.iter().map(|(a, b)| (a.as_str(), b.as_str())).collect(); + mapped("rename", from, self.fs.rename_all(&pairs))?; + for (name, kind) in moving { + let moved = format!("{to}{}", &name[from.len()..]); + self.names.remove(&name); + self.names.insert(moved.clone(), kind); + if let Some(node) = self.by_path.remove(&name) { self.open.get_mut(&node).expect("indexed").path = moved.clone(); self.by_path.insert(moved, node); } } - if self.names.get(from) == Some(&Kind::Dir) { - mapped("rename", from, self.fs.rename(&format!("{from}/"), &format!("{to}/")))?; - self.names.remove(from); - self.names.insert(to.to_string(), Kind::Dir); - } Ok(()) } } @@ -903,22 +909,23 @@ mod tests { again } - /// A full volume of one-page files, every other one then deleted: no two - /// free blocks are adjacent, so every run a file is given is one block. + /// A volume whose free blocks are all apart, so every run a file is given + /// is one block: every block a file's data may take, taken in one run, + /// every other one given back, and the run the reserve kept + /// (`bcachefs`'s `NODE_RESERVE`, 16) taken after. fn fragmented() -> DataVolume { let mut v = DataVolume::format(Ram::new(1024), &["home"], clock).unwrap(); - let mut made = 0; - while let Ok(n) = v.open(&format!("home/fill/{made}"), CREATE) { - let written = v.write(n, 0, &[9; BLOCK]); - v.close(n).unwrap(); - if written.is_err() { - break; - } - made += 1; - } - for i in (0..made).step_by(2) { - v.unlink(&format!("home/fill/{i}")).unwrap(); + let mut run = Vec::new(); + let mut page = 0; + while v.fs.resolve_or_alloc_block(&mut run, page).is_ok() { + page += 1; } + let taken: Vec = run.iter().flat_map(|e| e.start_block..e.start_block + e.block_count as u64).collect(); + let holes: Vec = + taken.iter().step_by(2).map(|&b| Extent { start_block: b, block_count: 1, _reserved: 0 }).collect(); + v.fs.free_extents(&holes).unwrap(); + let mut tail = vec![Extent { start_block: *taken.last().unwrap(), block_count: 1, _reserved: 0 }]; + v.fs.resolve_or_alloc_block(&mut tail, 16).unwrap(); v } @@ -1092,4 +1099,239 @@ mod tests { v.close(to).unwrap(); assert_eq!(v.lstat(&to_path).unwrap().size, 22, "`to`'s length reached the volume"); } + + /// A disk whose blocks are a map the test can copy, that keeps the first + /// `keep` block writes and loses every later one — the server killed there + /// — or refuses the one numbered `refuse` and keeps the rest. A request of + /// several blocks counts each, so a kill can land inside one. + type Image = BTreeMap>; + + struct Stops { + blocks: u64, + /// Shared, so a test reads what the disk holds while the server runs. + image: Rc>, + writes: usize, + keep: usize, + refuse: Option, + } + + impl Stops { + fn new(blocks: u64) -> Self { + Self { blocks, image: Rc::default(), writes: 0, keep: usize::MAX, refuse: None } + } + + fn from(image: &Image, blocks: u64, keep: usize, refuse: Option) -> Self { + Self { blocks, image: Rc::new(image.clone().into()), writes: 0, keep, refuse } + } + } + + impl Disk for Stops { + fn blocks(&self) -> u64 { + self.blocks + } + + fn read(&mut self, first: u64, out: &mut [u8]) -> Result<(), DiskError> { + for (i, chunk) in out.chunks_exact_mut(BLOCK).enumerate() { + match self.image.borrow().get(&(first + i as u64)) { + Some(block) => chunk.copy_from_slice(&block[..]), + None => chunk.fill(0), + } + } + Ok(()) + } + + fn write(&mut self, first: u64, data: &[u8]) -> Result<(), DiskError> { + for (i, chunk) in data.chunks_exact(BLOCK).enumerate() { + let n = self.writes; + self.writes += 1; + if Some(n) == self.refuse { + return Err(DiskError::Device); + } + if n < self.keep { + self.image.borrow_mut().insert(first + i as u64, Box::new(chunk.try_into().expect("a block"))); + } + } + Ok(()) + } + + fn flush(&mut self) -> Result<(), DiskError> { + Ok(()) + } + } + + fn take(v: DataVolume) -> Stops { + let DataVolume { fs, cache, .. } = v; + drop(fs); + Rc::try_unwrap(cache).ok().expect("one owner").into_disk() + } + + /// Every file under `dir` with its bytes, by its path beneath `dir`; a + /// directory with nothing under it is listed with a trailing `/`. + fn tree(v: &mut DataVolume, dir: &str) -> Option>> { + let listed = v.list(dir).ok()?; + let mut out = BTreeMap::new(); + if listed.is_empty() { + out.insert("/".to_string(), Vec::new()); + } + for (name, meta) in listed { + let path = join(dir, &name); + match meta.kind { + Kind::Dir => { + for (below, bytes) in tree(v, &path)? { + let below = if below == "/" { format!("{name}/") } else { format!("{name}/{below}") }; + out.insert(below, bytes); + } + } + _ => { + let n = v.open(&path, PLAIN).ok()?; + let mut bytes = vec![0u8; meta.size as usize]; + let read = v.read(n, 0, &mut Buf(&mut bytes)); + v.close(n).ok()?; + (read.ok()? == bytes.len()).then_some(())?; + out.insert(name, bytes); + } + } + } + Some(out) + } + + const STAGED: &str = "home/staged"; + const INSTALLED: &str = "apps/pkg"; + + /// Where the directory is: `Ok(true)` whole under its new name and absent + /// under its old, `Ok(false)` the reverse, and otherwise what was found. + fn whole_under_one(v: &mut DataVolume, want: &BTreeMap>) -> Result { + let (old, new) = (tree(v, STAGED), tree(v, INSTALLED)); + match (&old, &new) { + (Some(old), None) if old == want => Ok(false), + (None, Some(new)) if new == want => Ok(true), + _ => Err(format!( + "{} names under {STAGED} and {} under {INSTALLED}", + old.as_ref().map_or(0, |t| t.len()), + new.as_ref().map_or(0, |t| t.len()) + )), + } + } + + /// A staged package: files enough to fill several leaves, a subdirectory, + /// an empty one, and names beside it the rename must not move; synced. + fn staged() -> (Image, BTreeMap>) { + let mut v = DataVolume::format(Stops::new(1024), &["home", "apps"], clock).unwrap(); + for i in 0..80 { + let n = v.open(&format!("home/beside{i}"), CREATE).unwrap(); + v.write(n, 0, format!("beside {i}").as_bytes()).unwrap(); + v.close(n).unwrap(); + } + v.mkdir(STAGED).unwrap(); + v.mkdir(&format!("{STAGED}/empty")).unwrap(); + for i in 0..60 { + let path = if i % 4 == 0 { format!("{STAGED}/sub/f{i}") } else { format!("{STAGED}/f{i}") }; + let n = v.open(&path, CREATE).unwrap(); + v.write(n, 0, format!("file {i} of the package").as_bytes()).unwrap(); + v.close(n).unwrap(); + } + assert_eq!(v.sync(), Ok(Vec::new())); + let want = tree(&mut v, STAGED).unwrap(); + (take(v).image.take(), want) + } + + fn mounted(disk: Stops) -> Result, String> { + match DataVolume::probe(disk, &["home", "apps"], clock) { + Probed::Mounted(v) => Ok(v), + Probed::Unmountable(why) => Err(format!("unmountable: {why}")), + Probed::Foreign => Err("foreign".into()), + } + } + + /// The rename and the sync that makes it durable, against a disk that + /// stops at every block write the two make in turn: what the disk then + /// holds mounts, and names the directory whole under exactly one name. + #[test] + fn a_directory_rename_is_whole_under_one_name_wherever_the_server_is_killed() { + let (image, want) = staged(); + let blocks = 1024; + let mut v = mounted(Stops::from(&image, blocks, usize::MAX, None)).unwrap(); + v.rename(STAGED, INSTALLED).unwrap(); + assert_eq!(v.sync(), Ok(Vec::new())); + assert_eq!(whole_under_one(&mut v, &want), Ok(true)); + let writes = take(v).writes; + assert!(writes > 4, "the rename and its sync wrote {writes} blocks"); + + let mut torn = Vec::new(); + for keep in 0..=writes { + let mut v = mounted(Stops::from(&image, blocks, keep, None)).unwrap(); + let _ = v.rename(STAGED, INSTALLED); + let _ = v.sync(); + let verdict = mounted(take(v)).and_then(|mut again| whole_under_one(&mut again, &want)); + if let Err(why) = verdict { + torn.push(format!("killed after {keep} of {writes} writes: {why}")); + } + } + assert!(torn.is_empty(), "{} of {} kill points tear the directory:\n{}", torn.len(), writes + 1, torn.join("\n")); + } + + /// The same, with the one write numbered `n` refused and the server + /// alive: what it answers and what it then serves agree, the disk holds + /// the directory whole under one name past the refused sync, and the next + /// sync makes the answer what the disk holds. + #[test] + fn a_directory_rename_is_whole_under_one_name_whichever_write_is_refused() { + let (image, want) = staged(); + let blocks = 1024; + let mut v = mounted(Stops::from(&image, blocks, usize::MAX, None)).unwrap(); + v.rename(STAGED, INSTALLED).unwrap(); + assert_eq!(v.sync(), Ok(Vec::new())); + let writes = take(v).writes; + let held = |on_disk: &Image| { + mounted(Stops::from(on_disk, blocks, usize::MAX, None)).and_then(|mut v| whole_under_one(&mut v, &want)) + }; + + let mut torn = Vec::new(); + for refuse in 0..writes { + let disk = Stops::from(&image, blocks, usize::MAX, Some(refuse)); + let on_disk = Rc::clone(&disk.image); + let mut v = mounted(disk).unwrap(); + let renamed = v.rename(STAGED, INSTALLED).is_ok(); + let first = v.sync(); + let between = held(&on_disk.borrow()); + let served = whole_under_one(&mut v, &want); + let second = v.sync(); + let after = held(&on_disk.borrow()); + if served != Ok(renamed) || between.is_err() || second != Ok(Vec::new()) || after != Ok(renamed) { + torn.push(format!( + "write {refuse} of {writes} refused: answered {renamed}, served {served:?}, \ + the sync {first:?} left {between:?}, the next {second:?} left {after:?}" + )); + } + } + assert!(torn.is_empty(), "{} of {writes} refusals tear the directory:\n{}", torn.len(), torn.join("\n")); + } + + /// A rename the format refuses part-way — `z`'s extents fit its own name + /// and not the long one — leaves the directory whole under its old name. + #[test] + fn a_directory_rename_the_format_refuses_part_way_moves_nothing() { + let mut v = fragmented(); + let a = v.open("home/staged/a", CREATE).unwrap(); + v.write(a, 0, b"small").unwrap(); + v.close(a).unwrap(); + let z = v.open("home/staged/z", CREATE).unwrap(); + for page in 0..240u64 { + v.write(z, page * BLOCK as u64, &[1; BLOCK]).unwrap(); + } + v.close(z).unwrap(); + let to = format!("home/{}", "t".repeat(300)); + + assert_eq!(v.rename("home/staged", &to), Err(SyscallError::ResourceExhausted)); + + let names = |v: &mut DataVolume, dir: &str| -> Vec { + v.list(dir).map(|l| l.into_iter().map(|(n, _)| n).collect()).unwrap_or_default() + }; + assert_eq!(names(&mut v, "home/staged"), ["a", "z"]); + assert_eq!(v.lstat(&to), Err(SyscallError::NotFound)); + assert_eq!(v.sync(), Ok(Vec::new())); + let mut v = remount(v); + assert_eq!(names(&mut v, "home/staged"), ["a", "z"]); + assert_eq!(v.lstat(&to), Err(SyscallError::NotFound)); + } }