[device/bcache] some more work on bcache

lib/device/bcache.c

@@ -167,12 +167,19 @@ static struct io_engine *_engine_create(unsigned max_io)
 
 static void _engine_destroy(struct io_engine *e)
 {
+	int r;
+
 	_cb_set_destroy(e->cbs);
-	io_destroy(e->aio_context);
+
+	// io_destroy is really slow
+	r = io_destroy(e->aio_context);
+	if (r)
+		log_sys_warn("io_destroy");
+
 	dm_free(e);
 }
 
-static bool _engine_issue(struct io_engine *e, int fd, enum dir d,
+static bool _engine_issue(struct io_engine *e, enum dir d, int fd,
 			  sector_t sb, sector_t se, void *data, void *context)
 {
 	int r;
@@ -290,7 +297,6 @@ enum block_flags {
 };
 
 struct bcache {
-	int fd;
 	sector_t block_sectors;
 	uint64_t nr_data_blocks;
 	uint64_t nr_cache_blocks;
@@ -488,33 +494,6 @@ static void _relink(struct block *b)
  *
  *--------------------------------------------------------------*/
 
-/*
- * |b->list| should be valid (either pointing to itself, on one of the other
- * lists.
- */
-static bool _issue_low_level(struct block *b, enum dir d)
-{
-	struct bcache *cache = b->cache;
-	sector_t sb = b->index * cache->block_sectors;
-	sector_t se = sb + cache->block_sectors;
-
-	if (_test_flags(b, BF_IO_PENDING))
-		return false;
-
-	_set_flags(b, BF_IO_PENDING);
-	return _engine_issue(cache->engine, cache->fd, d, sb, se, b->data, b);
-}
-
-static inline bool _issue_read(struct block *b)
-{
-	return _issue_low_level(b, DIR_READ);
-}
-
-static inline bool _issue_write(struct block *b)
-{
-	return _issue_low_level(b, DIR_WRITE);
-}
-
 static void _complete_io(void *context, int err)
 {
 	struct block *b = context;
@@ -539,6 +518,39 @@ static void _complete_io(void *context, int err)
 	}
 }
 
+/*
+ * |b->list| should be valid (either pointing to itself, on one of the other
+ * lists.
+ */
+static bool _issue_low_level(struct block *b, enum dir d)
+{
+	struct bcache *cache = b->cache;
+	sector_t sb = b->index * cache->block_sectors;
+	sector_t se = sb + cache->block_sectors;
+
+	if (_test_flags(b, BF_IO_PENDING))
+		return false;
+
+	_set_flags(b, BF_IO_PENDING);
+	if (!_engine_issue(cache->engine, d, b->fd, sb, se, b->data, b)) {
+		_complete_io(b, -EIO);
+		return false;
+	}
+
+	return true;
+
+}
+
+static inline bool _issue_read(struct block *b)
+{
+	return _issue_low_level(b, DIR_READ);
+}
+
+static inline bool _issue_write(struct block *b)
+{
+	return _issue_low_level(b, DIR_WRITE);
+}
+
 static bool _wait_io(struct bcache *cache)
 {
 	return _engine_wait(cache->engine, _complete_io);
@@ -598,7 +610,7 @@ static struct block *_find_unused_clean_block(struct bcache *cache)
 	return NULL;
 }
 
-static struct block *_new_block(struct bcache *cache, block_address index)
+static struct block *_new_block(struct bcache *cache, int fd, block_address index)
 {
 	struct block *b;
 
@@ -616,6 +628,7 @@ static struct block *_new_block(struct bcache *cache, block_address index)
 		dm_list_init(&b->list);
 		dm_list_init(&b->hash);
 		b->flags = 0;
+		b->fd = fd;
 		b->index = index;
 		b->ref_count = 0;
 		b->error = 0;
@@ -685,7 +698,7 @@ static struct block *_lookup_or_read_block(struct bcache *cache,
 	} else {
 		_miss(cache, flags);
 
-		b = _new_block(cache, index);
+		b = _new_block(cache, fd, index);
 		if (b) {
 			if (flags & GF_ZERO)
 				_zero_block(b);
@@ -728,6 +741,21 @@ struct bcache *bcache_create(sector_t block_sectors, unsigned nr_cache_blocks)
 {
 	struct bcache *cache;
 
+	if (!nr_cache_blocks) {
+		log_warn("bcache must have at least one cache block");
+		return NULL;
+	}
+
+	if (!block_sectors) {
+		log_warn("bcache must have a non zero block size");
+		return NULL;
+	}
+
+	if (block_sectors & ((PAGE_SIZE >> SECTOR_SHIFT) - 1)) {
+		log_warn("bcache block size must be a multiple of page size");
+		return NULL;
+	}
+
 	cache = dm_malloc(sizeof(*cache));
 	if (!cache)
 		return NULL;
@@ -787,6 +815,11 @@ void bcache_destroy(struct bcache *cache)
 	dm_free(cache);
 }
 
+unsigned bcache_nr_cache_blocks(struct bcache *cache)
+{
+	return cache->nr_cache_blocks;
+}
+
 void bcache_prefetch(struct bcache *cache, int fd, block_address index)
 {
 	struct block *b = _hash_lookup(cache, fd, index);
@@ -794,7 +827,7 @@ void bcache_prefetch(struct bcache *cache, int fd, block_address index)
 	if (!b) {
 		cache->prefetches++;
 
-		b = _new_block(cache, index);
+		b = _new_block(cache, fd, index);
 		if (b)
 			_issue_read(b);
 	}
@@ -803,7 +836,9 @@ void bcache_prefetch(struct bcache *cache, int fd, block_address index)
 bool bcache_get(struct bcache *cache, int fd, block_address index,
 	        unsigned flags, struct block **result)
 {
-	struct block *b = _lookup_or_read_block(cache, fd, index, flags);
+	struct block *b;
+
+	b = _lookup_or_read_block(cache, fd, index, flags);
 	if (b) {
 		if (!b->ref_count)
 			cache->nr_locked++;

lib/device/bcache.h

@@ -61,7 +61,7 @@ enum bcache_get_flags {
 
 typedef uint64_t block_address;
 
-unsigned bcache_get_max_prefetches(struct bcache *cache);
+unsigned bcache_nr_cache_blocks(struct bcache *cache);
 
 /*
  * Use the prefetch method to take advantage of asynchronous IO.  For example,

test/unit/Makefile.in

@@ -47,7 +47,7 @@ $(TARGETS): $(OBJECTS) $(top_builddir)/libdm/libdevmapper.$(LIB_SUFFIX)
 	$(CC) $(CFLAGS) $(LDFLAGS) $(EXTRA_EXEC_LDFLAGS) -L$(top_builddir)/libdm \
 	      -o $@ $(OBJECTS) $(LDLIBS)
 
-unit: $(TARGETS)
+unit: $(TARGETS) $(top_builddir)/lib/liblvm-internal.a
 	@echo Running unit tests
 	LD_LIBRARY_PATH=$(top_builddir)/libdm ./$(TARGETS)
 endif

test/unit/bcache_t.c

@@ -12,9 +12,21 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#define _GNU_SOURCE
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
 #include "units.h"
 #include "bcache.h"
 
+#define MEG 2048
+#define SECTOR_SHIFT 9
+
+static const char *_test_path = "test.bin";
+
 int bcache_init(void)
 {
 	return 0;
@@ -25,6 +37,42 @@ int bcache_fini(void)
 	return 0;
 }
 
+static int open_file(const char *path)
+{
+	return open(path, O_EXCL | O_RDWR | O_DIRECT, 0666);
+}
+
+static int _prep_file(const char *path)
+{
+	int fd, r;
+
+	fd = open(path, O_CREAT | O_TRUNC | O_EXCL | O_RDWR | O_DIRECT, 0666);
+	if (fd < 0)
+		return -1;
+
+	r = fallocate(fd, FALLOC_FL_ZERO_RANGE, 0, (16 * MEG) << SECTOR_SHIFT);
+	if (r) {
+		close(fd);
+		return -1;
+	}
+
+	close(fd);
+	return 0;
+}
+
+
+static int test_init(void)
+{
+	unlink(_test_path);
+	return _prep_file(_test_path);
+}
+
+static int test_exit(void)
+{
+	unlink(_test_path);
+	return 0;
+}
+
 static void test_create(void)
 {
 	struct bcache *cache = bcache_create(8, 16);
@@ -32,7 +80,106 @@ static void test_create(void)
 	bcache_destroy(cache);
 }
 
+static void test_nr_cache_blocks_must_be_positive(void)
+{
+	struct bcache *cache = bcache_create(8, 0);
+	CU_ASSERT_PTR_NULL(cache);
+}
+
+static void test_block_size_must_be_positive(void)
+{
+	struct bcache *cache = bcache_create(0, 16);
+	CU_ASSERT_PTR_NULL(cache);
+}
+
+static void test_block_size_must_be_multiple_of_page_size(void)
+{
+	unsigned i;
+	struct bcache *cache;
+
+	{
+		static unsigned _bad_examples[] = {3, 9, 13, 1025};
+
+		for (i = 0; i < DM_ARRAY_SIZE(_bad_examples); i++) {
+			cache = bcache_create(_bad_examples[i], 16);
+			CU_ASSERT_PTR_NULL(cache);
+		}
+	}
+
+	{
+		// Only testing a few sizes because io_destroy is seriously
+		// slow.
+		for (i = 1; i < 25; i++) {
+			cache = bcache_create(8 * i, 16);
+			CU_ASSERT_PTR_NOT_NULL(cache);
+			bcache_destroy(cache);
+		}
+	}
+}
+
+static void test_reads_work(void)
+{
+	int fd;
+
+	// FIXME: add fixtures.
+	test_init();
+	fd = open_file("./test.bin");
+	CU_ASSERT(fd >= 0);
+
+	{
+		int i;
+		struct block *b;
+		struct bcache *cache = bcache_create(8, 16);
+
+		CU_ASSERT(bcache_get(cache, fd, 0, 0, &b));
+		for (i = 0; i < 8 << SECTOR_SHIFT; i++)
+			CU_ASSERT(((unsigned char *) b->data)[i] == 0);
+		bcache_put(b);
+
+		bcache_destroy(cache);
+	}
+
+	close(fd);
+
+	test_exit();
+}
+
+static void test_prefetch_works(void)
+{
+	int fd;
+
+	// FIXME: add fixtures.
+	test_init();
+	fd = open_file("./test.bin");
+	CU_ASSERT(fd >= 0);
+
+	{
+		int i;
+		struct block *b;
+		struct bcache *cache = bcache_create(8, 16);
+
+		for (i = 0; i < 16; i++)
+			bcache_prefetch(cache, fd, i);
+
+		for (i = 0; i < 16; i++) {
+			CU_ASSERT(bcache_get(cache, fd, i, 0, &b));
+			bcache_put(b);
+		}
+
+		bcache_destroy(cache);
+	}
+
+	close(fd);
+
+	test_exit();
+}
+
 CU_TestInfo bcache_list[] = {
 	{ (char*)"create", test_create },
+	{ (char*)"nr cache block must be positive", test_nr_cache_blocks_must_be_positive },
+	{ (char*)"block size must be positive", test_block_size_must_be_positive },
+	{ (char*)"block size must be multiple of page size", test_block_size_must_be_multiple_of_page_size },
+	{ (char*)"reads work", test_reads_work },
+	{ (char*)"prefetch works", test_prefetch_works },
 	CU_TEST_INFO_NULL
 };