From 2a427ca925a87d4feb45525df3d3fd584cbfdeb1 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 7 Jan 2013 14:59:41 +0000 Subject: [PATCH] Add persistent_data/lock_tracker for more sanity checking. --- Makefile.in | 2 + persistent-data/block.h | 5 +- persistent-data/block.tcc | 143 ++++++++++++++++++++------------ persistent-data/lock_tracker.cc | 97 ++++++++++++++++++++++ persistent-data/lock_tracker.h | 56 +++++++++++++ 5 files changed, 248 insertions(+), 55 deletions(-) create mode 100644 persistent-data/lock_tracker.cc create mode 100644 persistent-data/lock_tracker.h diff --git a/Makefile.in b/Makefile.in index 792d1c1..62e1a4f 100644 --- a/Makefile.in +++ b/Makefile.in @@ -31,6 +31,7 @@ SOURCE=\ persistent-data/endian_utils.cc \ persistent-data/error_set.cc \ persistent-data/hex_dump.cc \ + persistent-data/lock_tracker.cc \ persistent-data/space_map.cc \ persistent-data/space_map_disk.cc \ persistent-data/space_map_recursive.cc \ @@ -108,6 +109,7 @@ THIN_CHECK_SOURCE=\ persistent-data/endian_utils.cc \ persistent-data/error_set.cc \ persistent-data/hex_dump.cc \ + persistent-data/lock_tracker.cc \ persistent-data/space_map.cc \ persistent-data/space_map_disk.cc \ persistent-data/space_map_recursive.cc \ diff --git a/persistent-data/block.h b/persistent-data/block.h index 62b9ad8..1d47213 100644 --- a/persistent-data/block.h +++ b/persistent-data/block.h @@ -20,6 +20,7 @@ #define BLOCK_H #include "persistent-data/cache.h" +#include "persistent-data/lock_tracker.h" #include #include @@ -246,9 +247,7 @@ namespace persistent_data { mutable base::cache cache_; // FIXME: we need a dirty list as well as a cache - - typedef std::map > held_map; - mutable held_map held_locks_; + mutable lock_tracker tracker_; }; // A little utility to help build validators diff --git a/persistent-data/block.tcc b/persistent-data/block.tcc index 0d48942..be55c23 100644 --- a/persistent-data/block.tcc +++ b/persistent-data/block.tcc @@ -28,6 +28,7 @@ #include #include +// FIXME: remove these from a header! using namespace boost; using namespace persistent_data; using namespace std; @@ -180,6 +181,7 @@ block_manager::read_ref::~read_ref() } else bm_.cache_.put(block_); + bm_.tracker_.unlock(block_->location_); delete holders_; } } @@ -235,7 +237,8 @@ block_manager::block_manager(std::string const &path, unsigned max_concurrent_blocks, bool writeable) : io_(new block_io(path, nr_blocks, writeable)), - cache_(max(64u, max_concurrent_blocks)) + cache_(max(64u, max_concurrent_blocks)), + tracker_(0, nr_blocks) { } @@ -244,17 +247,24 @@ typename block_manager::read_ref block_manager::read_lock(block_address location, typename block_manager::validator::ptr v) const { - check(location); - boost::optional cached_block = cache_.get(location); + tracker_.read_lock(location); + try { + check(location); + boost::optional cached_block = cache_.get(location); - if (cached_block) { - (*cached_block)->check_read_lockable(); - return read_ref(*this, *cached_block); + if (cached_block) { + (*cached_block)->check_read_lockable(); + return read_ref(*this, *cached_block); + } + + block_ptr b(new block(io_, location, BT_NORMAL, v)); + cache_.insert(b); + return read_ref(*this, b); + + } catch (...) { + tracker_.unlock(location); + throw; } - - block_ptr b(new block(io_, location, BT_NORMAL, v)); - cache_.insert(b); - return read_ref(*this, b); } template @@ -262,18 +272,26 @@ typename block_manager::write_ref block_manager::write_lock(block_address location, typename block_manager::validator::ptr v) { - check(location); + tracker_.write_lock(location); + try { + check(location); - boost::optional cached_block = cache_.get(location); + boost::optional cached_block = cache_.get(location); - if (cached_block) { - (*cached_block)->check_write_lockable(); - return write_ref(*this, *cached_block); + if (cached_block) { + (*cached_block)->check_write_lockable(); + return write_ref(*this, *cached_block); + } + + block_ptr b(new block(io_, location, BT_NORMAL, v)); + cache_.insert(b); + return write_ref(*this, b); + + } catch (...) { + tracker_.unlock(location); + throw; } - block_ptr b(new block(io_, location, BT_NORMAL, v)); - cache_.insert(b); - return write_ref(*this, b); } template @@ -281,18 +299,25 @@ typename block_manager::write_ref block_manager::write_lock_zero(block_address location, typename block_manager::validator::ptr v) { - check(location); + tracker_.write_lock(location); + try { + check(location); - boost::optional cached_block = cache_.get(location); - if (cached_block) { - (*cached_block)->check_write_lockable(); - memset((*cached_block)->data_->raw(), 0, BlockSize); - return write_ref(*this, *cached_block); + boost::optional cached_block = cache_.get(location); + if (cached_block) { + (*cached_block)->check_write_lockable(); + memset((*cached_block)->data_->raw(), 0, BlockSize); + return write_ref(*this, *cached_block); + } + + block_ptr b(new block(io_, location, BT_NORMAL, v, true)); + cache_.insert(b); + return write_ref(*this, b); + + } catch (...) { + tracker_.unlock(location); + throw; } - - block_ptr b(new block(io_, location, BT_NORMAL, v, true)); - cache_.insert(b); - return write_ref(*this, b); } template @@ -300,20 +325,27 @@ typename block_manager::write_ref block_manager::superblock(block_address location, typename block_manager::validator::ptr v) { - check(location); + tracker_.write_lock(location); + try { + check(location); - boost::optional cached_block = cache_.get(location); + boost::optional cached_block = cache_.get(location); - if (cached_block) { - (*cached_block)->check_write_lockable(); - (*cached_block)->bt_ = BT_SUPERBLOCK; - (*cached_block)->validator_ = v; - return write_ref(*this, *cached_block); + if (cached_block) { + (*cached_block)->check_write_lockable(); + (*cached_block)->bt_ = BT_SUPERBLOCK; + (*cached_block)->validator_ = v; + return write_ref(*this, *cached_block); + } + + block_ptr b(new block(io_, location, BT_SUPERBLOCK, v)); + cache_.insert(b); + return write_ref(*this, b); + + } catch (...) { + tracker_.unlock(location); + throw; } - - block_ptr b(new block(io_, location, BT_SUPERBLOCK, v)); - cache_.insert(b); - return write_ref(*this, b); } template @@ -321,22 +353,29 @@ typename block_manager::write_ref block_manager::superblock_zero(block_address location, typename block_manager::validator::ptr v) { - check(location); + tracker_.write_lock(location); + try { + check(location); - boost::optional cached_block = cache_.get(location); + boost::optional cached_block = cache_.get(location); - if (cached_block) { - (*cached_block)->check_write_lockable(); - memset((*cached_block)->data_->raw(), 0, BlockSize); // FIXME: add a zero method to buffer - (*cached_block)->validator_ = v; - return write_ref(*this, *cached_block); + if (cached_block) { + (*cached_block)->check_write_lockable(); + memset((*cached_block)->data_->raw(), 0, BlockSize); // FIXME: add a zero method to buffer + (*cached_block)->validator_ = v; + return write_ref(*this, *cached_block); + } + + block_ptr b(new block(io_, location, BT_SUPERBLOCK, + mk_validator(new noop_validator), true)); + b->validator_ = v; + cache_.insert(b); + return write_ref(*this, b); + + } catch (...) { + tracker_.unlock(location); + throw; } - - block_ptr b(new block(io_, location, BT_SUPERBLOCK, - mk_validator(new noop_validator), true)); - b->validator_ = v; - cache_.insert(b); - return write_ref(*this, b); } template diff --git a/persistent-data/lock_tracker.cc b/persistent-data/lock_tracker.cc new file mode 100644 index 0000000..b7c3f48 --- /dev/null +++ b/persistent-data/lock_tracker.cc @@ -0,0 +1,97 @@ +// Copyright (C) 2012 Red Hat, Inc. All rights reserved. +// +// This file is part of the thin-provisioning-tools source. +// +// thin-provisioning-tools is free software: you can redistribute it +// and/or modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation, either version 3 of +// the License, or (at your option) any later version. +// +// thin-provisioning-tools is distributed in the hope that it will be +// useful, but WITHOUT ANY WARRANTY; without even the implied warranty +// of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with thin-provisioning-tools. If not, see +// . + +#include "lock_tracker.h" + +#include + +using namespace persistent_data; +using namespace std; + +//---------------------------------------------------------------- + +lock_tracker::lock_tracker(uint64_t low, uint64_t high) + : low_(low), + high_(high) +{ +} + +void +lock_tracker::read_lock(uint64_t key) +{ + check_key(key); + + LockMap::const_iterator it = locks_.find(key); + if (found(it)) { + if (it->second < 0) + throw runtime_error("already write locked"); + + locks_.insert(make_pair(key, it->second + 1)); + + } else + locks_.insert(make_pair(key, 1)); +} + +void +lock_tracker::write_lock(uint64_t key) +{ + check_key(key); + + LockMap::const_iterator it = locks_.find(key); + if (found(it)) + throw runtime_error("already locked"); + + locks_.insert(make_pair(key, -1)); +} + +void +lock_tracker::unlock(uint64_t key) +{ + check_key(key); + + LockMap::const_iterator it = locks_.find(key); + if (!found(it)) + throw runtime_error("not locked"); + + if (it->second > 1) + locks_.insert(make_pair(key, it->second - 1)); + else + locks_.erase(key); +} + +bool +lock_tracker::found(LockMap::const_iterator it) const +{ + return it != locks_.end(); +} + +bool +lock_tracker::valid_key(uint64_t key) const +{ + return (key >= low_ && key <= high_); +} + +void +lock_tracker::check_key(uint64_t key) const +{ + if (!valid_key(key)) + throw runtime_error("invalid key"); +} + +//---------------------------------------------------------------- + diff --git a/persistent-data/lock_tracker.h b/persistent-data/lock_tracker.h new file mode 100644 index 0000000..8541d83 --- /dev/null +++ b/persistent-data/lock_tracker.h @@ -0,0 +1,56 @@ +// Copyright (C) 2011 Red Hat, Inc. All rights reserved. +// +// This file is part of the thin-provisioning-tools source. +// +// thin-provisioning-tools is free software: you can redistribute it +// and/or modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation, either version 3 of +// the License, or (at your option) any later version. +// +// thin-provisioning-tools is distributed in the hope that it will be +// useful, but WITHOUT ANY WARRANTY; without even the implied warranty +// of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with thin-provisioning-tools. If not, see +// . + +#ifndef LOCK_TRACKER_H +#define LOCK_TRACKER_H + +#include +#include +#include + + +//---------------------------------------------------------------- + +namespace persistent_data { + class lock_tracker : private boost::noncopyable { + public: + lock_tracker(uint64_t low, uint64_t high); + + void read_lock(uint64_t key); + void write_lock(uint64_t key); + void unlock(uint64_t key); + + private: + typedef std::map LockMap; + + bool found(LockMap::const_iterator it) const; + + bool valid_key(uint64_t key) const; + void check_key(uint64_t key) const; + + // Positive for read lock, negative for write lock + LockMap locks_; + + uint64_t low_; + uint64_t high_; + }; +} + +//---------------------------------------------------------------- + +#endif