From 58d08714340049a5165ad682ec5b54292525b45d Mon Sep 17 00:00:00 2001
From: Haomai Wang <haomaiwang@gmail.com>
Date: Thu, 20 Mar 2014 14:09:49 +0800
Subject: [PATCH] Remove exclusive lock on GenericObjectMap

Now most of GenericObjectMap interfaces use header as argument not the union of
coll_t and ghobject_t. So caller should be responsible for maintain the
exclusive header.

Signed-off-by: Haomai Wang <haomaiwang@gmail.com>
---
 src/common/random_cache.hpp |  2 +-
 src/os/GenericObjectMap.cc  | 46 +++++++++----------------------------
 src/os/GenericObjectMap.h   | 26 ---------------------
 3 files changed, 12 insertions(+), 62 deletions(-)

diff --git a/src/common/random_cache.hpp b/src/common/random_cache.hpp
index da195f8f40e65..c627847064315 100644
--- a/src/common/random_cache.hpp
+++ b/src/common/random_cache.hpp
@@ -77,7 +77,7 @@ class RandomCache {
 
   void clear(K key) {
     Mutex::Locker l(lock);
-    contents.erase(key)
+    contents.erase(key);
   }
 
   void set_size(size_t new_size) {
diff --git a/src/os/GenericObjectMap.cc b/src/os/GenericObjectMap.cc
index 4d41c50ae3a59..011c83b563263 100644
--- a/src/os/GenericObjectMap.cc
+++ b/src/os/GenericObjectMap.cc
@@ -689,8 +689,6 @@ void GenericObjectMap::rename(const Header old_header, const coll_t &cid,
   old_header->cid = cid;
   old_header->oid = target;
   set_header(cid, target, *old_header, t);
-
-  // "in_use" still hold the "seq"
 }
 
 int GenericObjectMap::init(bool do_upgrade)
@@ -926,35 +924,18 @@ GenericObjectMap::Header GenericObjectMap::_lookup_header(
   to_get.insert(header_key(cid, oid));
   _Header header;
 
-  while (1) {
-    map<string, bufferlist> out;
-    bool try_again = false;
-
-    int r = db->get(GHOBJECT_TO_SEQ_PREFIX, to_get, &out);
-    if (r < 0)
-      return Header();
-    if (out.empty())
-      return Header();
-
-    bufferlist::iterator iter = out.begin()->second.begin();
-    header.decode(iter);
-
-    while (in_use.count(header.seq)) {
-      header_cond.Wait(header_lock);
+  map<string, bufferlist> out;
 
-      // Another thread is hold this header, wait for it.
-      // Because the seq of this object may change, such as clone
-      // and rename operation, here need to look up "seq" again
-      try_again = true;
-    }
+  int r = db->get(GHOBJECT_TO_SEQ_PREFIX, to_get, &out);
+  if (r < 0)
+    return Header();
+  if (out.empty())
+    return Header();
 
-    if (!try_again) {
-      break;
-    }
-  }
+  bufferlist::iterator iter = out.begin()->second.begin();
+  header.decode(iter);
 
-  Header ret = Header(new _Header(header), RemoveOnDelete(this));
-  in_use.insert(ret->seq);
+  Header ret = Header(new _Header(header));
   return ret;
 }
 
@@ -962,7 +943,7 @@ GenericObjectMap::Header GenericObjectMap::_generate_new_header(
     const coll_t &cid, const ghobject_t &oid, Header parent,
     KeyValueDB::Transaction t)
 {
-  Header header = Header(new _Header(), RemoveOnDelete(this));
+  Header header = Header(new _Header());
   header->seq = state.seq++;
   if (parent) {
     header->parent = parent->seq;
@@ -970,8 +951,6 @@ GenericObjectMap::Header GenericObjectMap::_generate_new_header(
   header->num_children = 1;
   header->oid = oid;
   header->cid = cid;
-  assert(!in_use.count(header->seq));
-  in_use.insert(header->seq);
 
   write_state(t);
   return header;
@@ -980,8 +959,6 @@ GenericObjectMap::Header GenericObjectMap::_generate_new_header(
 GenericObjectMap::Header GenericObjectMap::lookup_parent(Header input)
 {
   Mutex::Locker l(header_lock);
-  while (in_use.count(input->parent))
-    header_cond.Wait(header_lock);
   map<string, bufferlist> out;
   set<string> keys;
   keys.insert(PARENT_KEY);
@@ -999,13 +976,12 @@ GenericObjectMap::Header GenericObjectMap::lookup_parent(Header input)
     return Header();
   }
 
-  Header header = Header(new _Header(), RemoveOnDelete(this));
+  Header header = Header(new _Header());
   header->seq = input->parent;
   bufferlist::iterator iter = out.begin()->second.begin();
   header->decode(iter);
   dout(20) << "lookup_parent: parent seq is " << header->seq << " with parent "
            << header->parent << dendl;
-  in_use.insert(header->seq);
   return header;
 }
 
diff --git a/src/os/GenericObjectMap.h b/src/os/GenericObjectMap.h
index 80e100eaa7123..3c5e3cb9856f9 100644
--- a/src/os/GenericObjectMap.h
+++ b/src/os/GenericObjectMap.h
@@ -74,12 +74,6 @@ class GenericObjectMap {
    * Serializes access to next_seq as well as the in_use set
    */
   Mutex header_lock;
-  Cond header_cond;
-
-  /**
-   * Set of headers currently in use
-   */
-  set<uint64_t> in_use;
 
   GenericObjectMap(KeyValueDB *db) : db(db), header_lock("GenericObjectMap") {}
 
@@ -426,26 +420,6 @@ protected:
   // Sets header @see set_header
   void _set_header(Header header, const bufferlist &bl,
                    KeyValueDB::Transaction t);
-
-  /** 
-   * Removes header seq lock once Header is out of scope
-   * @see _lookup_header
-   * @see lookup_parent
-   * @see generate_new_header
-   */
-  class RemoveOnDelete {
-  public:
-    GenericObjectMap *db;
-    RemoveOnDelete(GenericObjectMap *db) :
-      db(db) {}
-    void operator() (_Header *header) {
-      Mutex::Locker l(db->header_lock);
-      db->in_use.erase(header->seq);
-      db->header_cond.Signal();
-      delete header;
-    }
-  };
-  friend class RemoveOnDelete;
 };
 WRITE_CLASS_ENCODER(GenericObjectMap::_Header)
 WRITE_CLASS_ENCODER(GenericObjectMap::State)
-- 
2.39.5