From: Josh Durgin <jdurgin@redhat.com>
Date: Tue, 24 Feb 2015 03:49:12 +0000 (-0800)
Subject: librbd: use snap_lock to protect ImageCtx->flags
X-Git-Tag: v0.93~6^2~13
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=40c2abb08a8d2cbf43c3facbe10252138173b498;p=ceph.git

librbd: use snap_lock to protect ImageCtx->flags

This is another step towards eliminating md_lock from the writeback
path. Almost all the places that use ImageCtx->flags already use
snap_lock, so there's no need to create a new lock. For the others,
add a helper, test_flags() that acquires the lock, similar to
test_features().

This also makes sure we look up flags of the snapshot we're operating
on, instead of those for head.

Signed-off-by: Josh Durgin <jdurgin@redhat.com>
---

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 493ad7551700c..01978dcfc5fd2 100644
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -473,6 +473,7 @@ namespace librbd {
 
   int ImageCtx::get_flags(librados::snap_t _snap_id, uint64_t *_flags) const
   {
+    assert(snap_lock.is_locked());
     if (_snap_id == CEPH_NOSNAP) {
       *_flags = flags;
       return 0;
@@ -485,6 +486,14 @@ namespace librbd {
     return -ENOENT;
   }
 
+  bool ImageCtx::test_flags(uint64_t test_flags) const
+  {
+    RWLock::RLocker l(snap_lock);
+    uint64_t snap_flags;
+    get_flags(snap_id, &snap_flags);
+    return ((snap_flags & test_flags) == test_flags);
+  }
+
   const parent_info* ImageCtx::get_parent_info(snap_t in_snap_id) const
   {
     assert(snap_lock.is_locked());
diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index a0ac2bf96592b..3d006982abcb3 100644
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -78,7 +78,7 @@ namespace librbd {
                    // isn't guarded by other locks below
                    // (size, features, image locks, etc)
     Mutex cache_lock; // used as client_lock for the ObjectCacher
-    RWLock snap_lock; // protects snapshot-related member variables and features
+    RWLock snap_lock; // protects snapshot-related member variables, features, and flags
     RWLock parent_lock; // protects parent_md and parent
     Mutex refresh_lock; // protects refresh_seq and last_refresh
     RWLock object_map_lock; // protects object map updates
@@ -161,6 +161,7 @@ namespace librbd {
 		     uint64_t *out_features) const;
     bool test_features(uint64_t test_features) const;
     int get_flags(librados::snap_t in_snap_id, uint64_t *flags) const;
+    bool test_flags(uint64_t test_flags) const;
     const parent_info* get_parent_info(librados::snap_t in_snap_id) const;
     int64_t get_parent_pool_id(librados::snap_t in_snap_id) const;
     std::string get_parent_image_id(librados::snap_t in_snap_id) const;
diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index c90a8b3420080..edd33b7e76e50 100644
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -114,7 +114,7 @@ bool ObjectMap::object_may_exist(uint64_t object_no) const
 {
   // Fall back to default logic if object map is disabled or invalid
   if (!m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP) ||
-      ((m_image_ctx.flags & RBD_FLAG_OBJECT_MAP_INVALID) != 0)) {
+      m_image_ctx.test_flags(RBD_FLAG_OBJECT_MAP_INVALID)) {
     return true;
   }
 
@@ -290,7 +290,10 @@ bool ObjectMap::aio_update(uint64_t start_object_no, uint64_t end_object_no,
 }
 
 void ObjectMap::invalidate() {
-  if ((m_image_ctx.flags & RBD_FLAG_OBJECT_MAP_INVALID) != 0) {
+  assert(m_image_ctx.snap_lock.is_wlocked());
+  uint64_t flags;
+  m_image_ctx.get_flags(m_image_ctx.snap_id, &flags);
+  if ((flags & RBD_FLAG_OBJECT_MAP_INVALID) != 0) {
     return;
   }
 
@@ -351,12 +354,12 @@ bool ObjectMap::Request::should_complete(int r) {
 }
 
 bool ObjectMap::Request::invalidate() {
-  if ((m_image_ctx.flags & RBD_FLAG_OBJECT_MAP_INVALID) != 0) {
+  if (m_image_ctx.test_flags(RBD_FLAG_OBJECT_MAP_INVALID)) {
     return true;
   }
 
   CephContext *cct = m_image_ctx.cct;
-  RWLock::WLocker l(m_image_ctx.md_lock);
+  RWLock::WLocker snap_locker(m_image_ctx.snap_lock);
 
   lderr(cct) << &m_image_ctx << " invalidating object map" << dendl;
   m_state = STATE_INVALIDATE;
diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index c51ae75bce2bf..6db4eba49852f 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -319,7 +319,7 @@ namespace librbd {
     }
 
     {
-      RWLock::RLocker l(ictx->md_lock);
+      RWLock::WLocker l(ictx->snap_lock);
       if (ictx->object_map != NULL) {
 	ictx->object_map->rollback(snap_id);
       }