Beginning to work

block-cache/block_cache.cc

@@ -10,6 +10,7 @@
 #include <unistd.h>
 
 #include <iostream>
+#include <stdexcept>
 
 //----------------------------------------------------------------
 
@@ -176,7 +177,6 @@ namespace bcache {
 	int
 	block_cache::issue_read(block &b)
 	{
-		std::cerr << "issuing read - that's a shame: " << b.index_ << "\n";
 		assert(!test_flags(b, IO_PENDING));
 		return issue_low_level(b, IO_CMD_PREAD, "read");
 	}
@@ -359,7 +359,7 @@ namespace bcache {
 		if (!b) {
 			if (list_empty(&clean_)) {
 				if (list_empty(&io_pending_))
-					writeback(9000);
+					writeback(16);
 				wait_io();
 			}
 
@@ -566,7 +566,6 @@ namespace bcache {
 	{
 		block *b, *tmp;
 
-		std::cerr << "in flush\n";
 		list_for_each_entry_safe (b, tmp, &dirty_, list_) {
 			if (b->ref_count_ || test_flags(*b, IO_PENDING))
 				// The superblock may well be still locked.
@@ -575,9 +574,7 @@ namespace bcache {
 			issue_write(*b);
 		}
 
-		std::cerr << "issued all writes\n";
 		wait_all();
-		std::cerr << "wait all returned\n";
 
 		return list_empty(&errored_) ? 0 : -EIO;
 	}

block-cache/block_cache.h

@@ -1,7 +1,6 @@
 #ifndef BLOCK_CACHE_H
 #define BLOCK_CACHE_H
 
-#include "block-cache/buffer.h"
 #include "block-cache/list.h"
 
 #include <boost/shared_ptr.hpp>

block-cache/buffer.h

@@ -1,94 +0,0 @@
-// Copyright (C) 2013 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
-// <http://www.gnu.org/licenses/>.
-
-#ifndef BUFFER_H
-#define BUFFER_H
-
-#include <stdint.h>
-#include <malloc.h>
-
-#include <boost/optional.hpp>
-#include <boost/shared_ptr.hpp>
-#include <boost/static_assert.hpp>
-
-#include <stdexcept>
-
-//----------------------------------------------------------------
-
-namespace bcache {
-	uint32_t const DEFAULT_BUFFER_SIZE = 4096;
-
-	template <uint32_t Size = DEFAULT_BUFFER_SIZE>
-	class buffer {
-	public:
-		buffer(void *data, bool writeable = true)
-			: data_(static_cast<unsigned char *>(data)),
-			  writeable_(writeable) {
-		}
-
-		typedef boost::shared_ptr<buffer> ptr;
-		typedef boost::shared_ptr<buffer const> const_ptr;
-
-		size_t size() const {
-			return Size;
-		}
-
-		unsigned char &operator[](unsigned index) {
-			check_writeable();
-			check_index(index);
-
-			return data_[index];
-		}
-
-		unsigned char const &operator[](unsigned index) const {
-			check_index(index);
-
-			return data_[index];
-		}
-
-		unsigned char *raw() {
-			return data_;
-		}
-
-		unsigned char const *raw() const {
-			return data_;
-		}
-
-		void set_data(void *data) {
-			data_ = static_cast<unsigned char *>(data);
-		}
-
-	private:
-		static void check_index(unsigned index) {
-			if (index >= Size)
-				throw std::range_error("buffer index out of bounds");
-		}
-
-		void check_writeable() const {
-			if (!writeable_)
-				throw std::runtime_error("buffer isn't writeable");
-		}
-
-		unsigned char *data_;
-		bool writeable_;
-	};
-}
-
-//----------------------------------------------------------------
-
-#endif

persistent-data/block.h

@@ -20,7 +20,6 @@
 #define BLOCK_H
 
 #include "block-cache/block_cache.h"
-#include "block-cache/buffer.h"
 
 #include <stdint.h>
 #include <map>
@@ -61,61 +60,28 @@ namespace persistent_data {
 			BT_NORMAL
 		};
 
-		// FIXME: eventually this will disappear to be replaced with block_cache::block
+		typedef void (*put_behaviour_fn)(block_cache &, block_cache::block &);
-		struct block : private boost::noncopyable {
-			typedef boost::shared_ptr<block> ptr;
-
-			block(block_cache &bc,
-			      block_address location,
-			      block_type bt,
-			      typename bcache::validator::ptr v,
-			      bool zero = false);
-			~block();
-
-			void check_read_lockable() const {
-				// FIXME: finish
-			}
-
-			void check_write_lockable() const {
-				// FIXME: finish
-			}
-
-			block_type get_type() const;
-			uint64_t get_location() const;
-
-			void const *get_data() const;
-			void *get_data();
-
-			void mark_dirty();
-			void unlock();
-
-		private:
-			void check_not_unlocked() const;
-
-			block_cache &bc_;
-			block_cache::block *internal_;
-			block_type bt_;
-			bool dirty_;
-			bool unlocked_;
-		};
 
 		class read_ref {
 		public:
 			static uint32_t const BLOCK_SIZE = BlockSize;
 
-			read_ref(block_manager<BlockSize> const &bm,
+			read_ref(block_cache &bc,
-				 typename block::ptr b);
+				 block_cache::block &b,
+				 put_behaviour_fn fn);
+
 			read_ref(read_ref const &rhs);
 			virtual ~read_ref();
 
 			read_ref const &operator =(read_ref const &rhs);
 
 			block_address get_location() const;
-			void const * data() const;
+			void const *data() const;
 
 		protected:
-			block_manager<BlockSize> const *bm_;
+			block_cache &bc_;
-			typename block::ptr block_;
+			block_cache::block &b_;
+			put_behaviour_fn fn_;
 			unsigned *holders_;
 		};
 
@@ -123,13 +89,24 @@ namespace persistent_data {
 		// locked.
 		class write_ref : public read_ref {
 		public:
-			write_ref(block_manager<BlockSize> const &bm,
+			write_ref(block_cache &bc,
-				  typename block::ptr b);
+				  block_cache::block &b,
+				  put_behaviour_fn fn);
 
 			using read_ref::data;
 			void *data();
 		};
 
+		class super_ref : public write_ref {
+		public:
+			super_ref(block_cache &bc,
+				  block_cache::block &b,
+				  put_behaviour_fn fn);
+
+			using read_ref::data;
+			using write_ref::data;
+		};
+
 		// Locking methods
 		read_ref
 		read_lock(block_address location,
@@ -174,11 +151,9 @@ namespace persistent_data {
 
 	private:
 		void check(block_address b) const;
-		void write_block(typename block::ptr b) const;
 
 		int fd_;
 
-		// FIXME: the mutable is a fudge to allow flush() to be const, which I'm not sure is necc.
 		mutable block_cache bc_;
 	};
 

persistent-data/block.tcc

@@ -105,102 +105,39 @@ namespace {
 };
 
 namespace persistent_data {
+
+	inline void read_put(block_cache &bc, block_cache::block &b) {
+		bc.put(b, 0);
+	}
+
+	inline void write_put(block_cache &bc, block_cache::block &b) {
+		bc.put(b, block_cache::PF_DIRTY);
+	}
+
+	inline void super_put(block_cache &bc, block_cache::block &b) {
+		bc.flush();
+		bc.put(b, block_cache::PF_DIRTY);
+		bc.flush();
+	}
+
 	template <uint32_t BlockSize>
-	block_manager<BlockSize>::block::block(block_cache &bc,
+	block_manager<BlockSize>::read_ref::read_ref(block_cache &bc,
-					       block_address location,
+						     block_cache::block &b,
-					       block_type bt,
+						     put_behaviour_fn fn)
-					       typename validator::ptr v,
-					       bool zero)
 		: bc_(bc),
-		  bt_(bt),
+		  b_(b),
-		  dirty_(false),
+		  fn_(fn),
-		  unlocked_(false)
+		  holders_(new unsigned)
-	{
-		if (zero) {
-			internal_ = &bc.get(location, block_cache::GF_ZERO | block_cache::GF_CAN_BLOCK, v);
-			dirty_ = true;
-		} else {
-			internal_ = &bc.get(location, block_cache::GF_CAN_BLOCK, v);
-		}
-	}
-
-	template <uint32_t BlockSize>
-	block_manager<BlockSize>::block::~block()
-	{
-		if (!unlocked_)
-			unlock();
-	}
-
-	template <uint32_t BlockSize>
-	void
-	block_manager<BlockSize>::block::unlock()
-	{
-		bc_.put(*internal_, dirty_ ? block_cache::PF_DIRTY : 0);
-		unlocked_ = true;
-	}
-
-	template <uint32_t BlockSize>
-	typename block_manager<BlockSize>::block_type
-	block_manager<BlockSize>::block::get_type() const
-	{
-		return bt_;
-	}
-
-	template <uint32_t BlockSize>
-	uint64_t
-	block_manager<BlockSize>::block::get_location() const
-	{
-		check_not_unlocked();
-		return internal_->get_index();
-	}
-
-	template <uint32_t BlockSize>
-	void const *
-	block_manager<BlockSize>::block::get_data() const
-	{
-		return internal_->get_data();
-	}
-
-	template <uint32_t BlockSize>
-	void *
-	block_manager<BlockSize>::block::get_data()
-	{
-		return internal_->get_data();
-	}
-
-	template <uint32_t BlockSize>
-	void
-	block_manager<BlockSize>::block::mark_dirty()
-	{
-		check_not_unlocked();
-		dirty_ = true;
-	}
-
-	template <uint32_t BlockSize>
-	void
-	block_manager<BlockSize>::block::check_not_unlocked() const
-	{
-		if (unlocked_)
-			throw std::runtime_error("block prematurely unlocked");
-	}
-
-	//----------------------------------------------------------------
-
-	template <uint32_t BlockSize>
-	block_manager<BlockSize>::read_ref::read_ref(block_manager<BlockSize> const &bm,
-						     typename block::ptr b)
-		: bm_(&bm),
-		block_(b),
-		holders_(new unsigned)
 	{
 		*holders_ = 1;
 	}
 
 	template <uint32_t BlockSize>
 	block_manager<BlockSize>::read_ref::read_ref(read_ref const &rhs)
-		: bm_(rhs.bm_),
+		: bc_(rhs.bc_),
-		block_(rhs.block_),
+		  b_(rhs.b_),
-		holders_(rhs.holders_)
+		  fn_(rhs.fn_),
+		  holders_(rhs.holders_)
 	{
 		(*holders_)++;
 	}
@@ -209,13 +146,7 @@ namespace persistent_data {
 	block_manager<BlockSize>::read_ref::~read_ref()
 	{
 		if (!--(*holders_)) {
-			if (block_->get_type() == BT_SUPERBLOCK) {
+			fn_(bc_, b_);
-				bm_->flush();
-				block_->unlock();
-				bm_->flush();
-			} else
-				block_->unlock();
-
 			delete holders_;
 		}
 	}
@@ -225,8 +156,9 @@ namespace persistent_data {
 	block_manager<BlockSize>::read_ref::operator =(read_ref const &rhs)
 	{
 		if (this != &rhs) {
-			block_ = rhs.block_;
+			bc_ = rhs.bc_;
-			bm_ = rhs.bm_;
+			b_ = rhs.b_;
+			fn_ = rhs.fn_;
 			holders_ = rhs.holders_;
 			(*holders_)++;
 		}
@@ -238,31 +170,40 @@ namespace persistent_data {
 	block_address
 	block_manager<BlockSize>::read_ref::get_location() const
 	{
-		return block_->get_location();
+		return b_.get_index();
 	}
 
 	template <uint32_t BlockSize>
 	void const *
 	block_manager<BlockSize>::read_ref::data() const
 	{
-		return block_->get_data();
+		return b_.get_data();
 	}
 
 	//--------------------------------
 
 	template <uint32_t BlockSize>
-	block_manager<BlockSize>::write_ref::write_ref(block_manager<BlockSize> const &bm,
+	block_manager<BlockSize>::write_ref::write_ref(block_cache &bc,
-						       typename block::ptr b)
+						       block_cache::block &b,
-		: read_ref(bm, b)
+						       put_behaviour_fn fn)
+		: read_ref(bc, b, fn)
 	{
-		b->mark_dirty();
 	}
 
 	template <uint32_t BlockSize>
 	void *
 	block_manager<BlockSize>::write_ref::data()
 	{
-		return read_ref::block_->get_data();
+		return read_ref::b_.get_data();
+	}
+
+	//--------------------------------
+
+	template <uint32_t BlockSize>
+	block_manager<BlockSize>::super_ref::super_ref(block_cache &bc,
+						       block_cache::block &b,
+						       put_behaviour_fn fn)
+		: write_ref(bc, b, fn) {
 	}
 
 	//----------------------------------------------------------------
@@ -282,8 +223,8 @@ namespace persistent_data {
 	block_manager<BlockSize>::read_lock(block_address location,
 					    typename bcache::validator::ptr v) const
 	{
-		typename block::ptr b(new block(bc_, location, BT_NORMAL, v, false));
+		block_cache::block &b = bc_.get(location, block_cache::GF_CAN_BLOCK, v);
-		return read_ref(*this, b);
+		return read_ref(bc_, b, read_put);
 	}
 
 	template <uint32_t BlockSize>
@@ -291,8 +232,8 @@ namespace persistent_data {
 	block_manager<BlockSize>::write_lock(block_address location,
 					     typename bcache::validator::ptr v)
 	{
-		typename block::ptr b(new block(bc_, location, BT_NORMAL, v, false));
+		block_cache::block &b = bc_.get(location, block_cache::GF_CAN_BLOCK, v);
-		return write_ref(*this, b);
+		return write_ref(bc_, b, write_put);
 	}
 
 	template <uint32_t BlockSize>
@@ -300,8 +241,8 @@ namespace persistent_data {
 	block_manager<BlockSize>::write_lock_zero(block_address location,
 						  typename bcache::validator::ptr v)
 	{
-		typename block::ptr b(new block(bc_, location, BT_NORMAL, v, true));
+		block_cache::block &b = bc_.get(location, block_cache::GF_CAN_BLOCK | block_cache::GF_ZERO, v);
-		return write_ref(*this, b);
+		return write_ref(bc_, b, write_put);
 	}
 
 	template <uint32_t BlockSize>
@@ -309,8 +250,8 @@ namespace persistent_data {
 	block_manager<BlockSize>::superblock(block_address location,
 					     typename bcache::validator::ptr v)
 	{
-		typename block::ptr b(new block(bc_, location, BT_SUPERBLOCK, v, false));
+		block_cache::block &b = bc_.get(location, block_cache::GF_CAN_BLOCK, v);
-		return write_ref(*this, b);
+		return super_ref(bc_, b, super_put);
 	}
 
 	template <uint32_t BlockSize>
@@ -318,8 +259,8 @@ namespace persistent_data {
 	block_manager<BlockSize>::superblock_zero(block_address location,
 						  typename bcache::validator::ptr v)
 	{
-		typename block::ptr b(new block(bc_, location, BT_SUPERBLOCK, v, true));
+		block_cache::block &b = bc_.get(location, block_cache::GF_CAN_BLOCK | block_cache::GF_ZERO, v);
-		return write_ref(*this, b);
+		return super_ref(bc_, b, super_put);
 	}
 
 	template <uint32_t BlockSize>
@@ -329,13 +270,6 @@ namespace persistent_data {
 		return bc_.get_nr_blocks();
 	}
 
-	template <uint32_t BlockSize>
-	void
-	block_manager<BlockSize>::write_block(typename block::ptr b) const
-	{
-		b->flush();
-	}
-
 	template <uint32_t BlockSize>
 	void
 	block_manager<BlockSize>::flush() const