From: Somnath Roy <somnath.roy@sandisk.com>
Date: Thu, 31 Jul 2014 21:56:42 +0000 (-0700)
Subject: FileStore: No need to hold Index lock during omap calls
X-Git-Tag: v0.85~37^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3e7848d52bc4d5d5852ca79aad5ec3524b781feb;p=ceph.git

FileStore: No need to hold Index lock during omap calls

The Index lock is held during all the omap calls which is
not necessary.

Signed-off-by: Somnath Roy <somnath.roy@sandisk.com>
---

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index 4c4a16581501..feb317162ca6 100644
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -258,8 +258,6 @@ int FileStore::lfn_open(coll_t cid,
   }
 
   int fd, exist;
-
-  
   assert(NULL != (*index).index);
   if (need_lock) {
     ((*index).index)->access_lock.get_write();
@@ -3716,9 +3714,6 @@ int FileStore::getattr(coll_t cid, const ghobject_t& oid, const char *name, buff
       dout(10) << __func__ << " could not get index r = " << r << dendl;
       goto out;
     }
-    assert(NULL != index.index);
-    RWLock::RLocker l((index.index)->access_lock);
-
     r = object_map->get_xattrs(oid, to_get, &got);
     if (r < 0 && r != -ENOENT) {
       dout(10) << __func__ << " get_xattrs err r =" << r << dendl;
@@ -3779,9 +3774,6 @@ int FileStore::getattrs(coll_t cid, const ghobject_t& oid, map<string,bufferptr>
     goto out;
   }
   {
-    assert(NULL != index.index);
-    RWLock::RLocker l((index.index)->access_lock);
-
     r = object_map->get_all_xattrs(oid, &omap_attrs);
     if (r < 0 && r != -ENOENT) {
       dout(10) << __func__ << " could not get omap_attrs r = " << r << dendl;
@@ -3945,9 +3937,6 @@ int FileStore::_rmattr(coll_t cid, const ghobject_t& oid, const char *name,
       dout(10) << __func__ << " could not get index r = " << r << dendl;
       goto out_close;
     }
-    assert(NULL != index.index);
-    RWLock::RLocker l((index.index)->access_lock);
-
     set<string> to_remove;
     to_remove.insert(string(name));
     r = object_map->remove_xattrs(oid, to_remove, &spos);
@@ -4008,9 +3997,6 @@ int FileStore::_rmattrs(coll_t cid, const ghobject_t& oid,
     goto out_close;
   }
   {
-    assert(NULL != index.index);
-    RWLock::RLocker l((index.index)->access_lock);
-
     r = object_map->get_all_xattrs(oid, &omap_attrs);
     if (r < 0 && r != -ENOENT) {
       dout(10) << __func__ << " could not get omap_attrs r = " << r << dendl;
@@ -4455,13 +4441,13 @@ int FileStore::omap_get(coll_t c, const ghobject_t &hoid,
   int r = get_index(c, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::RLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   r = object_map->get(hoid, header, out);
   if (r < 0 && r != -ENOENT) {
     assert(!m_filestore_fail_eio || r != -EIO);
@@ -4481,14 +4467,13 @@ int FileStore::omap_get_header(
   int r = get_index(c, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::RLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   r = object_map->get_header(hoid, bl);
   if (r < 0 && r != -ENOENT) {
     assert(allow_eio || !m_filestore_fail_eio || r != -EIO);
@@ -4504,14 +4489,13 @@ int FileStore::omap_get_keys(coll_t c, const ghobject_t &hoid, set<string> *keys
   int r = get_index(c, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::RLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   r = object_map->get_keys(hoid, keys);
   if (r < 0 && r != -ENOENT) {
     assert(!m_filestore_fail_eio || r != -EIO);
@@ -4529,14 +4513,13 @@ int FileStore::omap_get_values(coll_t c, const ghobject_t &hoid,
   int r = get_index(c, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::RLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   r = object_map->get_values(hoid, keys, out);
   if (r < 0 && r != -ENOENT) {
     assert(!m_filestore_fail_eio || r != -EIO);
@@ -4555,14 +4538,13 @@ int FileStore::omap_check_keys(coll_t c, const ghobject_t &hoid,
   int r = get_index(c, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::RLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   r = object_map->check_keys(hoid, keys, out);
   if (r < 0 && r != -ENOENT) {
     assert(!m_filestore_fail_eio || r != -EIO);
@@ -4579,14 +4561,13 @@ ObjectMap::ObjectMapIterator FileStore::get_omap_iterator(coll_t c,
   int r = get_index(c, &index);
   if (r < 0)
     return ObjectMap::ObjectMapIterator(); 
-
-  assert(NULL != index.index);
-  RWLock::RLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return ObjectMap::ObjectMapIterator();
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return ObjectMap::ObjectMapIterator();
+  }
   return object_map->get_iterator(hoid);
 }
 
@@ -4812,14 +4793,13 @@ int FileStore::_omap_clear(coll_t cid, const ghobject_t &hoid,
   int r = get_index(cid, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::WLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   r = object_map->clear_keys_header(hoid, &spos);
   if (r < 0 && r != -ENOENT)
     return r;
@@ -4834,14 +4814,13 @@ int FileStore::_omap_setkeys(coll_t cid, const ghobject_t &hoid,
   int r = get_index(cid, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::WLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   return object_map->set_keys(hoid, aset, &spos);
 }
 
@@ -4853,14 +4832,13 @@ int FileStore::_omap_rmkeys(coll_t cid, const ghobject_t &hoid,
   int r = get_index(cid, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::WLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   r = object_map->rm_keys(hoid, keys, &spos);
   if (r < 0 && r != -ENOENT)
     return r;
@@ -4893,14 +4871,13 @@ int FileStore::_omap_setheader(coll_t cid, const ghobject_t &hoid,
   int r = get_index(cid, &index);
   if (r < 0)
     return r;
-
-  assert(NULL != index.index);
-  RWLock::WLocker l((index.index)->access_lock);
-
-  r = lfn_find(hoid, index);
-
-  if (r < 0)
-    return r;
+  {
+    assert(NULL != index.index);
+    RWLock::RLocker l((index.index)->access_lock);
+    r = lfn_find(hoid, index);
+    if (r < 0)
+      return r;
+  }
   return object_map->set_header(hoid, bl, &spos);
 }