From a99d6896a8d026490d90aad52884d96b06fd0196 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 29 Jul 2014 13:41:45 +0100 Subject: [PATCH] Fix a bug in the block cache read path. --- block-cache/block_cache.cc | 50 ++++++++++++++++++++++++-------------- block-cache/block_cache.h | 7 +++--- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/block-cache/block_cache.cc b/block-cache/block_cache.cc index 98aabf4..7d5203c 100644 --- a/block-cache/block_cache.cc +++ b/block-cache/block_cache.cc @@ -146,7 +146,7 @@ namespace bcache { * lists. */ // FIXME: add batch issue - int + void block_cache::issue_low_level(block &b, enum io_iocb_cmd opcode, const char *desc) { int r; @@ -168,25 +168,26 @@ namespace bcache { info("could not submit IOs, with %s op\n", desc); complete_io(b, EIO); - return -EIO; - } - return 0; + std::ostringstream out; + out << "couldn't issue io (" << desc << ") for block " << b.index_; + throw std::runtime_error(out.str()); + } } - int + void block_cache::issue_read(block &b) { assert(!b.test_flags(BF_IO_PENDING)); - return issue_low_level(b, IO_CMD_PREAD, "read"); + issue_low_level(b, IO_CMD_PREAD, "read"); } - int + void block_cache::issue_write(block &b) { assert(!b.test_flags(BF_IO_PENDING)); b.v_->prepare(b.data_, b.index_); - return issue_low_level(b, IO_CMD_PWRITE, "write"); + issue_low_level(b, IO_CMD_PWRITE, "write"); } void @@ -267,7 +268,6 @@ namespace bcache { unsigned block_cache::writeback(unsigned count) { - int r; block *b, *tmp; unsigned actual = 0, dirty_length = 0; @@ -282,9 +282,8 @@ namespace bcache { if (b->ref_count_) continue; - r = issue_write(*b); - if (!r) - actual++; + issue_write(*b); + actual++; } info("writeback: requested %u, actual %u, dirty length %u\n", count, actual, dirty_length); @@ -361,6 +360,23 @@ namespace bcache { cb->u.c.nbytes = block_size_bytes; } + block_cache::block * + block_cache::find_unused_clean_block() + { + struct block *b, *tmp; + + list_for_each_entry_safe (b, tmp, &clean_, list_) { + if (b->ref_count_) + continue; + + hash_remove(*b); + list_del(&b->list_); + return b; + } + + return NULL; + } + block_cache::block * block_cache::new_block(block_address index) { @@ -374,11 +390,7 @@ namespace bcache { wait_io(); } - if (!list_empty(&clean_)) { - b = list_first_entry(&clean_, block, list_); - hash_remove(*b); - list_del(&b->list_); - } + b = find_unused_clean_block(); } if (b) { @@ -510,8 +522,10 @@ namespace bcache { else { if (b->v_.get() && b->v_.get() != v.get() && - b->test_flags(BF_DIRTY)) + b->test_flags(BF_DIRTY)) { b->v_->prepare(b->data_, b->index_); + v->check(b->data_, b->index_); + } } b->v_ = v; diff --git a/block-cache/block_cache.h b/block-cache/block_cache.h index 3ba8fd4..24f7949 100644 --- a/block-cache/block_cache.h +++ b/block-cache/block_cache.h @@ -139,9 +139,9 @@ namespace bcache { int init_free_list(unsigned count); block *__alloc_block(); void complete_io(block &b, int result); - int issue_low_level(block &b, enum io_iocb_cmd opcode, const char *desc); - int issue_read(block &b); - int issue_write(block &b); + void issue_low_level(block &b, enum io_iocb_cmd opcode, const char *desc); + void issue_read(block &b); + void issue_write(block &b); void wait_io(); list_head *__categorise(block &b); void hit(block &b); @@ -154,6 +154,7 @@ namespace bcache { void hash_insert(block &b); void hash_remove(block &b); void setup_control_block(block &b); + block *find_unused_clean_block(); block *new_block(block_address index); void mark_dirty(block &b); unsigned calc_nr_cache_blocks(size_t mem, sector_t block_size);