From 76535116823f02f0392226e5725fbfef14c277ba Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Fri, 20 Feb 2015 13:43:46 -0800 Subject: [PATCH] DBObjectMap: lock header_lock on sync() Otherwise, we can race with another thread updating state.seq resulting in the old, smaller value getting persisted. If there is a crash at that time, we will reuse a sequence number, resulting in an inconsistent node tree and bug #9891. Fixes: 9891 Backport: giant, firefly, dumpling Signed-off-by: Samuel Just (cherry picked from commit 2b63dd25fc1c73fa42e52e9ea4ab5a45dd9422a0) Conflicts: src/os/DBObjectMap.cc because we have state.v = 1; instead of state.v = 2; --- src/os/DBObjectMap.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/os/DBObjectMap.cc b/src/os/DBObjectMap.cc index c5a5b69c95b6..ba73dc8ee3af 100644 --- a/src/os/DBObjectMap.cc +++ b/src/os/DBObjectMap.cc @@ -1070,6 +1070,8 @@ int DBObjectMap::upgrade() db->submit_transaction(t); } state.v = 1; + + Mutex::Locker l(header_lock); KeyValueDB::Transaction t = db->get_transaction(); write_state(t); db->submit_transaction_sync(t); @@ -1111,7 +1113,6 @@ int DBObjectMap::init(bool do_upgrade) int DBObjectMap::sync(const ghobject_t *oid, const SequencerPosition *spos) { KeyValueDB::Transaction t = db->get_transaction(); - write_state(t); if (oid) { assert(spos); MapHeaderLock hl(this, *oid); @@ -1122,11 +1123,18 @@ int DBObjectMap::sync(const ghobject_t *oid, header->spos = *spos; set_map_header(hl, *oid, *header, t); } + Mutex::Locker l(header_lock); + write_state(t); + return db->submit_transaction_sync(t); + } else { + Mutex::Locker l(header_lock); + write_state(t); + return db->submit_transaction_sync(t); } - return db->submit_transaction_sync(t); } int DBObjectMap::write_state(KeyValueDB::Transaction _t) { + assert(header_lock.is_locked_by_me()); dout(20) << "dbobjectmap: seq is " << state.seq << dendl; KeyValueDB::Transaction t = _t ? _t : db->get_transaction(); bufferlist bl; -- 2.47.3