From 4324c8bc7e66633035c15995e3f82ef91d3a5e8c Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 20 Oct 2017 08:51:17 -0500 Subject: [PATCH] os/bluestore/BlueFS: fix race with log flush during async log compaction During async log compaction we rely on _flush-and_sync_log to update the log_writer to jump_to. However, if racing threads are also trying to flush the log and manage to flush our new log events for us, then our flush will turn into a no-op, and we won't update jump_to correctly at all. This results in a corrupted log size a bit later one. Fix by ensuring that there are no in-progress flushes before we add our log entries. Also, add asserts to _flush_and_sync_log to make sure we never bail out early if jump_to is set (which would indicate this or another similar bug is still present). Fixes: http://tracker.ceph.com/issues/21878 Signed-off-by: Sage Weil --- src/os/bluestore/BlueFS.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 88f48eefec743..2550e3e941b09 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -1195,6 +1195,14 @@ void BlueFS::_compact_log_async(std::unique_lock& l) new_log = new File; new_log->fnode.ino = 0; // so that _flush_range won't try to log the fnode + // 0. wait for any racing flushes to complete. (We do not want to block + // in _flush_sync_log with jump_to set or else a racing thread might flush + // our entries and our jump_to update won't be correct.) + while (log_flushing) { + dout(10) << __func__ << " log is currently flushing, waiting" << dendl; + log_cond.wait(l); + } + // 1. allocate new log space and jump to it. old_log_jump_to = log_file->fnode.get_allocated(); uint64_t need = old_log_jump_to + cct->_conf->bluefs_max_log_runway; @@ -1345,16 +1353,19 @@ int BlueFS::_flush_and_sync_log(std::unique_lock& l, while (log_flushing) { dout(10) << __func__ << " want_seq " << want_seq << " log is currently flushing, waiting" << dendl; + assert(!jump_to); log_cond.wait(l); } if (want_seq && want_seq <= log_seq_stable) { dout(10) << __func__ << " want_seq " << want_seq << " <= log_seq_stable " << log_seq_stable << ", done" << dendl; + assert(!jump_to); return 0; } if (log_t.empty() && dirty_files.empty()) { dout(10) << __func__ << " want_seq " << want_seq << " " << log_t << " not dirty, dirty_files empty, no-op" << dendl; + assert(!jump_to); return 0; } -- 2.39.5