MDS: clean up internal MDRequests the standard way

All cleanup is now routed through respond_to_request(),
which invokes the internal_op_finish Context*, then does
mdcache->request_finish(). This is easier to reason about,
and indeed fixes a bug (I was not cleaning up locks
following flush). Use the MDSContinuation to facilitate
this in scrub's case.

Signed-off-by: Greg Farnum <gfarnum@redhat.com>

diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc
index fda1f704769f57429a6cfb2a8b6ee584105248e6..ea6e1d2045f02f2c48302c7e3715d0f9667e4e4f 100644 (file)
--- a/src/mds/CInode.cc
+++ b/src/mds/CInode.cc
@@ -44,7 +44,7 @@
 #include "global/global_context.h"
 #include "include/assert.h"
 
-#include "common/Continuation.h"
+#include "mds/MDSContinuation.h"
 
 #define dout_subsys ceph_subsys_mds
 #undef dout_prefix
@@ -3472,9 +3472,9 @@ void InodeStore::generate_test_instances(list<InodeStore*> &ls)
 }
 
 void CInode::validate_disk_state(CInode::validated_data *results,
-                                 Context *fin)
+                                 MDRequestRef &mdr)
 {
-  class ValidationContinuation : public Continuation {
+  class ValidationContinuation : public MDSContinuation {
   public:
     CInode *in;
     CInode::validated_data *results;
@@ -3490,8 +3490,8 @@ void CInode::validate_disk_state(CInode::validated_data *results,
 
     ValidationContinuation(CInode *i,
                            CInode::validated_data *data_r,
-                           Context *fin) :
-                             Continuation(fin),
+                           MDRequestRef &mdr) :
+                             MDSContinuation(mdr, i->mdcache->mds->server),
                              in(i),
                              results(data_r),
                              shadow_in(NULL) {
@@ -3520,9 +3520,7 @@ void CInode::validate_disk_state(CInode::validated_data *results,
 
       results->passed_validation = false; // we haven't finished it yet
 
-      MDSIOContextWrapper *mdsioc =
-          new MDSIOContextWrapper(in->mdcache->mds, get_callback(BACKTRACE));
-      C_OnFinisher *conf = new C_OnFinisher(mdsioc,
+      C_OnFinisher *conf = new C_OnFinisher(get_io_callback(BACKTRACE),
                                             &in->mdcache->mds->finisher);
 
       in->fetch_backtrace(conf, &bl);
@@ -3590,8 +3588,7 @@ void CInode::validate_disk_state(CInode::validated_data *results,
         in->mdcache->create_unlinked_system_inode(shadow_in,
                                                   in->inode.ino,
                                                   in->inode.mode);
-        shadow_in->fetch(new MDSInternalContextWrapper(in->mdcache->mds,
-                                                       get_callback(INODE)));
+        shadow_in->fetch(get_internal_callback(INODE));
         return false;
       } else {
         return fetch_dirfrag_rstats();
@@ -3636,8 +3633,7 @@ void CInode::validate_disk_state(CInode::validated_data *results,
           dirfrag->fetch(gather.new_sub(), false);
       }
       if (gather.has_subs()) {
-        gather.set_finisher(new MDSInternalContextWrapper(in->mdcache->mds,
-                                                          get_callback(DIRFRAGS)));
+        gather.set_finisher(get_internal_callback(DIRFRAGS));
         gather.activate();
         return false;
       } else {
@@ -3685,7 +3681,7 @@ void CInode::validate_disk_state(CInode::validated_data *results,
 
   ValidationContinuation *vc = new ValidationContinuation(this,
                                                           results,
-                                                          fin);
+                                                          mdr);
   vc->begin();
 }
 

diff --git a/src/mds/CInode.h b/src/mds/CInode.h
index d014d2859706ea5178b33f30cede6ad954ab5d93..7e2af07432b8834064ab455d77d20993277ae922 100644 (file)
--- a/src/mds/CInode.h
+++ b/src/mds/CInode.h
@@ -49,6 +49,9 @@ class Session;
 class MClientCaps;
 struct ObjectOperation;
 class EMetaBlob;
+struct MDRequestImpl;
+typedef ceph::shared_ptr<MDRequestImpl> MDRequestRef;
+
 
 ostream& operator<<(ostream& out, CInode& in);
 
@@ -919,7 +922,7 @@ public:
    * been completed.
    */
   void validate_disk_state(validated_data *results,
-                           Context *fin);
+                           MDRequestRef& mdr);
   static void dump_validation_results(const validated_data& results,
                                       Formatter *f);
 private:

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index 40de91d3ee9dde1f3b1b5a6ef2326299f30391f5..2d62f4ed710596b0c20134164bf714043a6e4cc7 100644 (file)
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -11506,13 +11506,12 @@ void C_MDS_RetryRequest::finish(int r)
 class C_scrub_dentry_finish : public Context {
 public:
   CInode::validated_data results;
-  MDCache *mdcache;
   MDRequestRef mdr;
   Context *on_finish;
   Formatter *formatter;
-  C_scrub_dentry_finish(MDCache *mdc, MDRequestRef& mdr,
+  C_scrub_dentry_finish(MDRequestRef& mdr,
                         Context *fin, Formatter *f) :
-    mdcache(mdc), mdr(mdr), on_finish(fin), formatter(f) {}
+    mdr(mdr), on_finish(fin), formatter(f) {}
 
   void finish(int r) {
     if (r >= 0) { // we got into the scrubbing dump it
@@ -11522,7 +11521,6 @@ public:
       formatter->dump_int("return_code", r);
       formatter->close_section(); // results
     }
-    mdcache->request_finish(mdr);
     on_finish->complete(r);
   }
 };
@@ -11533,7 +11531,7 @@ void MDCache::scrub_dentry(const string& path, Formatter *f, Context *fin)
   MDRequestRef mdr = request_start_internal(CEPH_MDS_OP_VALIDATE);
   filepath fp(path.c_str());
   mdr->set_filepath(fp);
-  C_scrub_dentry_finish *csd = new C_scrub_dentry_finish(this, mdr, fin, f);
+  C_scrub_dentry_finish *csd = new C_scrub_dentry_finish(mdr, fin, f);
   mdr->internal_op_finish = csd;
   mdr->internal_op_private = &csd->results;
   scrub_dentry_work(mdr);
@@ -11556,7 +11554,7 @@ void MDCache::scrub_dentry_work(MDRequestRef& mdr)
   CInode::validated_data *vr =
       static_cast<CInode::validated_data*>(mdr->internal_op_private);
 
-  in->validate_disk_state(vr, mdr->internal_op_finish);
+  in->validate_disk_state(vr, mdr);
   return;
 }
 
@@ -11570,6 +11568,16 @@ void MDCache::flush_dentry(const string& path, Context *fin)
   flush_dentry_work(mdr);
 }
 
+class C_FinishIOMDR : public MDSInternalContextBase {
+protected:
+  MDS *mds;
+  MDRequestRef mdr;
+  MDS *get_mds() { return mds; }
+public:
+  C_FinishIOMDR(MDS *mds_, MDRequestRef& mdr_) : mds(mds_), mdr(mdr_) {}
+  void finish(int r) { mds->server->respond_to_request(mdr, r); }
+};
+
 void MDCache::flush_dentry_work(MDRequestRef& mdr)
 {
   set<SimpleLock*> rdlocks, wrlocks, xlocks;
@@ -11582,5 +11590,5 @@ void MDCache::flush_dentry_work(MDRequestRef& mdr)
   bool locked = mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks);
   if (!locked)
     return;
-  in->flush(new MDSInternalContextWrapper(mds,mdr->internal_op_finish));
+  in->flush(new C_FinishIOMDR(mds, mdr));
 }

diff --git a/src/mds/MDSContext.h b/src/mds/MDSContext.h
index 346330936758dd12908e6bdea4ea52f1241b81fc..a5e8458e9eb4d2a496d6d7d556a08d8735acd397 100644 (file)
--- a/src/mds/MDSContext.h
+++ b/src/mds/MDSContext.h
@@ -162,6 +162,4 @@ protected:
 
 typedef C_GatherBuilderBase<MDSInternalContextBase, MDSGather> MDSGatherBuilder;
 
-
 #endif  // MDS_CONTEXT_H
-

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index a747959d82e03026430000d01a8cc4fd7faa8bcd..9bb274ae93f84a541ac8f9beabaa361e3e94bfc5 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -910,6 +910,7 @@ void Server::respond_to_request(MDRequestRef& mdr, int r)
     if (!mdr->internal_op_finish)
       assert(0 == "trying to respond to internal op without finisher");
     mdr->internal_op_finish->complete(r);
+    mdcache->request_finish(mdr);
   }
 }
 
@@ -7653,4 +7654,3 @@ bool Server::waiting_for_reconnect(client_t c) const
 {
   return client_reconnect_gather.count(c) > 0;
 }
-