From: Josh Durgin <josh.durgin@inktank.com>
Date: Mon, 25 Feb 2013 19:33:48 +0000 (-0800)
Subject: librbd: drop snap_lock before invalidating cache
X-Git-Tag: v0.58~11^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=5806226cf0743bb44eaf7bc815897c6846d43233;p=ceph.git

librbd: drop snap_lock before invalidating cache

Writeback will take the snap_lock, so read everything we need under it
before invalidating the cache. This avoids a recursive lock when writeback
uses snap_lock while snap_rollback() was holding it.

Remove a not-very-useful debugging message that depended on snap_lock being held.

Fixes: #4249
Backport: bobtail
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
---

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 33b948d23104..a4c25dfaa524 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1749,17 +1749,23 @@ reprotect_and_return_err:
       return r;
 
     Mutex::Locker l(ictx->md_lock);
-    Mutex::Locker l2(ictx->snap_lock);
-    if (!ictx->snap_exists)
-      return -ENOENT;
+    snap_t snap_id;
+    uint64_t new_size;
+    {
+      // need to drop snap_lock before invalidating cache
+      Mutex::Locker l2(ictx->snap_lock);
+      if (!ictx->snap_exists)
+	return -ENOENT;
 
-    if (ictx->snap_id != CEPH_NOSNAP || ictx->read_only)
-      return -EROFS;
+      if (ictx->snap_id != CEPH_NOSNAP || ictx->read_only)
+	return -EROFS;
 
-    snap_t snap_id = ictx->get_snap_id(snap_name);
-    if (snap_id == CEPH_NOSNAP) {
-      lderr(cct) << "No such snapshot found." << dendl;
-      return -ENOENT;
+      snap_id = ictx->get_snap_id(snap_name);
+      if (snap_id == CEPH_NOSNAP) {
+	lderr(cct) << "No such snapshot found." << dendl;
+	return -ENOENT;
+      }
+      new_size = ictx->get_image_size(ictx->snap_id);
     }
 
     // need to flush any pending writes before resizing and rolling back -
@@ -1767,9 +1773,6 @@ reprotect_and_return_err:
     // the current version, so we have to invalidate that too.
     ictx->invalidate_cache();
 
-    uint64_t new_size = ictx->get_image_size(ictx->snap_id);
-    ictx->get_snap_size(snap_name, &new_size);
-
     ldout(cct, 2) << "resizing to snapshot size..." << dendl;
     NoOpProgressContext no_op;
     r = resize_helper(ictx, new_size, no_op);
@@ -1785,10 +1788,6 @@ reprotect_and_return_err:
       return r;
     }
 
-    snap_t new_snap_id = ictx->get_snap_id(snap_name);
-    ldout(cct, 20) << "snap_id is " << ictx->snap_id << " new snap_id is "
-		   << new_snap_id << dendl;
-
     notify_change(ictx->md_ctx, ictx->header_oid, NULL, ictx);
 
     ictx->perfcounter->inc(l_librbd_snap_rollback);