From 876dd2427fa2ceacf976dd899878f3cf28df96f7 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 16 Nov 2011 12:53:37 +0000 Subject: [PATCH] change sm->new_block() to return an optional --- space_map.h | 10 +++++++++- space_map_core.h | 10 +++++++--- space_map_disk.cc | 22 ++++++++++++++-------- space_map_recursive.cc | 15 +++++++++++---- thin_pool.cc | 6 +++++- transaction_manager.cc | 18 +++++++++++++----- 6 files changed, 59 insertions(+), 22 deletions(-) diff --git a/space_map.h b/space_map.h index f39e83a..bdd8914 100644 --- a/space_map.h +++ b/space_map.h @@ -5,6 +5,7 @@ #include "block_counter.h" #include +#include //---------------------------------------------------------------- @@ -25,7 +26,14 @@ namespace persistent_data { virtual void inc(block_address b) = 0; virtual void dec(block_address b) = 0; - virtual block_address new_block() = 0; + + // FIXME: change these to return an optional, failure is + // not that rare if we're restricting the area that's + // searched. + typedef boost::optional maybe_block; + + virtual maybe_block new_block() = 0; + virtual maybe_block new_block(block_address begin, block_address end) = 0; virtual bool count_possibly_greater_than_one(block_address b) const = 0; diff --git a/space_map_core.h b/space_map_core.h index de527bc..bba5464 100644 --- a/space_map_core.h +++ b/space_map_core.h @@ -52,15 +52,19 @@ namespace persistent_data { nr_free_++; } - block_address new_block() { - for (block_address i = 0; i < counts_.size(); i++) + maybe_block new_block() { + return new_block(0, counts_.size()); + } + + maybe_block new_block(block_address begin, block_address end) { + for (block_address i = begin; i < min(end, counts_.size()); i++) if (counts_[i] == 0) { counts_[i] = 1; nr_free_--; return i; } - throw std::runtime_error("no space"); + return maybe_block(); } bool count_possibly_greater_than_one(block_address b) const { diff --git a/space_map_disk.cc b/space_map_disk.cc index 41d0ae9..9c4ebb6 100644 --- a/space_map_disk.cc +++ b/space_map_disk.cc @@ -290,16 +290,22 @@ namespace { set_count(b, old - 1); } - block_address new_block() { - // FIXME: silly to always start searching from the - // beginning. - block_address nr_indexes = div_up(nr_blocks_, ENTRIES_PER_BLOCK); - for (block_address index = 0; index < nr_indexes; index++) { + maybe_block new_block() { + // FIXME: keep track of the lowest free block so we + // can start searching from a suitable place. + return new_block(0, nr_blocks_); + } + + maybe_block new_block(block_address begin, block_address end) { + block_address begin_index = begin / ENTRIES_PER_BLOCK; + block_address end_index = div_up(end, ENTRIES_PER_BLOCK); + + for (block_address index = begin_index; index < end_index; index++) { index_entry ie = indexes_->find_ie(index); bitmap bm(tm_, ie); - optional maybe_b = bm.find_free(0, (index == nr_indexes - 1) ? - nr_blocks_ % ENTRIES_PER_BLOCK : ENTRIES_PER_BLOCK); + optional maybe_b = bm.find_free((index == begin_index) ? (begin % ENTRIES_PER_BLOCK) : 0, + (index == end_index - 1) ? (end % ENTRIES_PER_BLOCK) : ENTRIES_PER_BLOCK); if (maybe_b) { block_address b = *maybe_b; indexes_->save_ie(index, bm.get_ie()); @@ -310,7 +316,7 @@ namespace { } } - throw runtime_error("out of space"); + return maybe_block(); } bool count_possibly_greater_than_one(block_address b) const { diff --git a/space_map_recursive.cc b/space_map_recursive.cc index a68597d..8433471 100644 --- a/space_map_recursive.cc +++ b/space_map_recursive.cc @@ -85,14 +85,21 @@ namespace { } } - virtual block_address new_block() { - // new_block can recurse, because we know it's - // looking up entries in the _previous_ - // transaction. + // new_block must not recurse. + virtual boost::optional + new_block() { + cant_recurse("new_block"); recursing_lock lock(*this); return sm_->new_block(); } + virtual boost::optional + new_block(block_address begin, block_address end) { + cant_recurse("new_block(range)"); + recursing_lock lock(*this); + return sm_->new_block(begin, end); + } + virtual bool count_possibly_greater_than_one(block_address b) const { if (depth_) return true; diff --git a/thin_pool.cc b/thin_pool.cc index fb6ef4c..d68d797 100644 --- a/thin_pool.cc +++ b/thin_pool.cc @@ -167,7 +167,11 @@ thin_pool::get_held_root() const block_address thin_pool::alloc_data_block() { - return md_->data_sm_->new_block(); + optional mb = md_->data_sm_->new_block(); + if (!mb) + throw runtime_error("couldn't allocate new block"); + + return *mb; } void diff --git a/transaction_manager.cc b/transaction_manager.cc index 261ad31..b93232c 100644 --- a/transaction_manager.cc +++ b/transaction_manager.cc @@ -30,10 +30,13 @@ transaction_manager::begin(block_address superblock, validator v) transaction_manager::write_ref transaction_manager::new_block(validator v) { - block_address b = sm_->new_block(); - sm_decrementer decrementer(sm_, b); - write_ref wr = bm_->write_lock_zero(b, v); - add_shadow(b); + optional mb = sm_->new_block(); + if (!mb) + throw runtime_error("couldn't allocate new block"); + + sm_decrementer decrementer(sm_, *mb); + write_ref wr = bm_->write_lock_zero(*mb, v); + add_shadow(*mb); decrementer.dont_bother(); return wr; } @@ -46,7 +49,12 @@ transaction_manager::shadow(block_address orig, validator v) return make_pair(bm_->write_lock(orig, v), false); read_ref src = bm_->read_lock(orig, v); - write_ref dest = bm_->write_lock_zero(sm_->new_block(), v); + + optional mb = sm_->new_block(); + if (!mb) + throw runtime_error("couldn't allocate new block"); + + write_ref dest = bm_->write_lock_zero(*mb, v); ::memcpy(dest.data(), src.data(), MD_BLOCK_SIZE); ref_t count = sm_->get_count(orig);