From: Samuel Just Date: Fri, 20 Feb 2015 21:43:46 +0000 (-0800) Subject: DBObjectMap: lock header_lock on sync() X-Git-Tag: v0.80.10~52^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=5865411360f722ec511f2df6656d4ba975bef8eb;p=ceph.git 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; --- diff --git a/src/os/DBObjectMap.cc b/src/os/DBObjectMap.cc index 5d2a2e61624..0a258001146 100644 --- a/src/os/DBObjectMap.cc +++ b/src/os/DBObjectMap.cc @@ -1042,6 +1042,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); @@ -1083,7 +1085,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); Header header = lookup_map_header(*oid); @@ -1093,11 +1094,18 @@ int DBObjectMap::sync(const ghobject_t *oid, header->spos = *spos; set_map_header(*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;