librbd: use finisher for copy-on-read copyup fulfillment

When the RBD cache is enabled, the ObjectCacher does not allow
reentrancy to read the full object.  As a temporary workaround,
use the Finisher to handle CoR read requests.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc
index 14a6328d37c36a606f76cb811e2bd77505508229..9a486fc5ecf4da453685c811069e3594cbe26dc3 100644 (file)
--- a/src/librbd/AioRequest.cc
+++ b/src/librbd/AioRequest.cc
@@ -158,13 +158,13 @@ namespace librbd {
             }
           }
 
-          read_from_parent(image_extents);
-
-          if (is_copy_on_read(m_ictx, m_snap_id))
+          if (is_copy_on_read(m_ictx, m_snap_id)) {
             m_state = LIBRBD_AIO_READ_COPYUP; 
-          else 
+          } else {
             m_state = LIBRBD_AIO_READ_GUARD;
+         }
 
+          read_from_parent(image_extents);
           finished = false;
         }
       }
@@ -204,7 +204,7 @@ namespace librbd {
             m_ictx->copyup_list[m_object_no] = new_req;
             m_ictx->copyup_list_lock.Unlock();
 
-            new_req->read_from_parent(m_image_extents);
+            new_req->queue_read_from_parent(m_image_extents);
           }
         } else {
           m_ictx->copyup_list_lock.Unlock();

diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc
index c82e2e7e77a23a346e10817cd6a104d031cc91ea..d3790a96fa48168e0376af544e126e4e8ce4e0df 100644 (file)
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -113,6 +113,14 @@ namespace librbd {
     aio_read(m_ictx->parent, image_extents, NULL, &m_copyup_data, m_parent_completion, 0);
   }
 
+  void CopyupRequest::queue_read_from_parent(vector<pair<uint64_t,uint64_t> >& image_extents)
+  {
+    // TODO: once the ObjectCacher allows reentrant read requests, the finisher
+    // should be eliminated
+    C_ReadFromParent *ctx = new C_ReadFromParent(this, image_extents);
+    m_ictx->copyup_finisher->queue(ctx);
+  }
+
   void CopyupRequest::rbd_read_from_parent_cb(completion_t cb, void *arg)
   {
     CopyupRequest *req = reinterpret_cast<CopyupRequest *>(arg);

diff --git a/src/librbd/CopyupRequest.h b/src/librbd/CopyupRequest.h
index 246af27b7ac21c731ca54cfdcbb2bd8065f147e6..c5572cfad927821b578c8913e74f4958ea46d741 100644 (file)
--- a/src/librbd/CopyupRequest.h
+++ b/src/librbd/CopyupRequest.h
@@ -28,12 +28,28 @@ namespace librbd {
     void complete_all(int r);
     void send_copyup(int r);
     void read_from_parent(vector<pair<uint64_t,uint64_t> >& image_extents);
+    void queue_read_from_parent(vector<pair<uint64_t,uint64_t> >& image_extents);
 
     static void rbd_read_from_parent_cb(completion_t cb, void *arg);
     static void rbd_copyup_cb(completion_t aio_completion_impl, void *arg);
 
   private:
     ImageCtx *m_ictx;
+
+    class C_ReadFromParent : public Context {
+    public:
+      C_ReadFromParent(CopyupRequest *c, vector<pair<uint64_t,uint64_t> > i)
+        : m_req(c), m_image_extents(i) {}
+
+      virtual void finish(int r) {
+        m_req->read_from_parent(m_image_extents);
+      }
+
+    private:
+      CopyupRequest *m_req;
+      vector<pair<uint64_t,uint64_t> > m_image_extents;
+    };
+
     std::string m_oid;
     uint64_t m_object_no;
     Mutex m_lock;

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index c49d05233838a6dffce7a6b2a740f00572151b9d..ddc5cfe524fb1d63506aa7bf060400d5ee0f099f 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -57,7 +57,7 @@ namespace librbd {
       stripe_unit(0), stripe_count(0),
       object_cacher(NULL), writeback_handler(NULL), object_set(NULL),
       readahead(),
-      total_bytes_read(0),
+      total_bytes_read(0), copyup_finisher(NULL),
       pending_aio(0)
   {
     md_ctx.dup(p);
@@ -103,6 +103,11 @@ namespace librbd {
       object_set->return_enoent = true;
       object_cacher->start();
     }
+
+    if (cct->_conf->rbd_clone_copy_on_read) {
+      copyup_finisher = new Finisher(cct);
+      copyup_finisher->start();
+    }
   }
 
   ImageCtx::~ImageCtx() {
@@ -119,6 +124,10 @@ namespace librbd {
       delete object_set;
       object_set = NULL;
     }
+    if (copyup_finisher != NULL) {
+      delete copyup_finisher;
+      copyup_finisher = NULL;
+    }
     delete[] format_string;
   }
 

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index c451004c47d4823e1355059d9b71388c021734be..41dfd33980051f48ad01ae22f4bdf0138bc44866 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -27,6 +27,7 @@
 #include "librbd/parent_types.h"
 
 class CephContext;
+class Finisher;
 class PerfCounters;
 
 namespace librbd {
@@ -101,6 +102,8 @@ namespace librbd {
 
     Readahead readahead;
     uint64_t total_bytes_read;
+
+    Finisher *copyup_finisher;
     std::map<uint64_t, CopyupRequest*> copyup_list;
 
     Cond pending_aio_cond;

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 9745655264a28f4fc2da717a02e7272b76d79678..3d977f3eb97401c61970dd2209fe87a03ced278d 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2281,7 +2281,6 @@ reprotect_and_return_err:
     if (ictx->image_watcher != NULL) {
       ictx->image_watcher->flush_aio_operations();
     }
-    ictx->wait_for_pending_copyup();
     if (ictx->object_cacher) {
       ictx->shutdown_cache(); // implicitly flushes
     } else {
@@ -2289,6 +2288,12 @@ reprotect_and_return_err:
       ictx->wait_for_pending_aio();
     }
 
+    if (ictx->copyup_finisher != NULL) {
+      ictx->copyup_finisher->wait_for_empty();
+      ictx->copyup_finisher->stop();
+    }
+    ictx->wait_for_pending_copyup();
+
     if (ictx->parent) {
       close_image(ictx->parent);
       ictx->parent = NULL;