mds: remove duplicate context completion calls

After reverting commit df404e0, the context completion in
MDSRank handling for `dump stray` command isn't required.

Also fixup the incorrect usage of std::unique_ptr with context
completion class (Context). Contexts delete themselves upon
completion.

Introduced-by: 801951e8c0d62dbbe724ce506fb44bc809bb7d4f
Signed-off-by: Venky Shankar <vshankar@redhat.com>

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index c17c77c152cda3a0448ffe08530da0aa025d42ec..308d1304b0427c44b9ae553f5dcdfdbc91b5863a 100644 (file)
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -10289,13 +10289,18 @@ void MDCache::notify_global_snaprealm_update(int snap_op)
 // STRAYS
 
 struct C_MDC_RetryScanStray : public MDCacheContext {
-  dirfrag_t next;
-  std::unique_ptr<MDCache::C_MDS_DumpStrayDirCtx> cmd_ctx;
-  C_MDC_RetryScanStray(MDCache *c,  dirfrag_t n, std::unique_ptr<MDCache::C_MDS_DumpStrayDirCtx> ctx) :
-   MDCacheContext(c), next(n), cmd_ctx(std::move(ctx)) {}
+  C_MDC_RetryScanStray(MDCache *c,  dirfrag_t n, MDCache::C_MDS_DumpStrayDirCtx *ctx) :
+    MDCacheContext(c),
+    next(n),
+    cmd_ctx(ctx) {
+  }
+
   void finish(int r) override {
-    mdcache->scan_stray_dir(next, std::move(cmd_ctx));
+    mdcache->scan_stray_dir(next, cmd_ctx);
   }
+
+  dirfrag_t next;
+  MDCache::C_MDS_DumpStrayDirCtx *cmd_ctx;
 };
 
 /*
@@ -10304,7 +10309,7 @@ struct C_MDC_RetryScanStray : public MDCacheContext {
  * The cmd_ctx holds the formatter to dump stray dir content while scanning.
  * The function can return EAGAIN, to make possible waiting semantics clear.
 */
-int MDCache::scan_stray_dir(dirfrag_t next, std::unique_ptr<MDCache::C_MDS_DumpStrayDirCtx> cmd_ctx)
+int MDCache::scan_stray_dir(dirfrag_t next, C_MDS_DumpStrayDirCtx *cmd_ctx)
 {
   dout(10) << "scan_stray_dir " << next << dendl;
 
@@ -10319,41 +10324,47 @@ int MDCache::scan_stray_dir(dirfrag_t next, std::unique_ptr<MDCache::C_MDS_DumpS
     strays[i]->get_dirfrags(ls);
 
     for (const auto& dir : ls) {
-      if (dir->get_frag() < next.frag)
-       continue;
+      if (dir->get_frag() < next.frag) {
+        continue;
+      }
 
       if (!dir->can_auth_pin()) {
-       dir->add_waiter(CDir::WAIT_UNFREEZE, new C_MDC_RetryScanStray(this, dir->dirfrag(), std::move(cmd_ctx)));
-       return -EAGAIN;
+        dir->add_waiter(CDir::WAIT_UNFREEZE, new C_MDC_RetryScanStray(this, dir->dirfrag(), cmd_ctx));
+        return 0;
       }
 
       if (!dir->is_complete()) {
-       dir->fetch(new C_MDC_RetryScanStray(this, dir->dirfrag(), std::move(cmd_ctx)));
-       return -EAGAIN;
+        dout(20) << __func__ << ": fetching: " << *dir << dendl;
+        dir->fetch(new C_MDC_RetryScanStray(this, dir->dirfrag(), cmd_ctx));
+        return 0;
       }
 
+      dout(20) << __func__ << "dir=" << *dir << " is complete" << dendl;
+
       for (auto &p : dir->items) {
-       CDentry *dn = p.second;
-       dn->state_set(CDentry::STATE_STRAY);
-       CDentry::linkage_t *dnl = dn->get_projected_linkage();
-       if (dnl->is_primary()) {
-         CInode *in = dnl->get_inode();
-    // only if we came from asok cmd handler
-    if (cmd_ctx) {
-      cmd_ctx->begin_dump();
-      cmd_ctx->get_formatter()->open_object_section("stray_inode");
-      cmd_ctx->get_formatter()->dump_int("ino: ", in->ino());
-      cmd_ctx->get_formatter()->dump_string("stray_prior_path: ", in->get_inode()->stray_prior_path);
-      in->dump(cmd_ctx->get_formatter(), CInode::DUMP_CAPS);
-      cmd_ctx->get_formatter()->close_section();
-    }
-         if (in->get_inode()->nlink == 0)
-           in->state_set(CInode::STATE_ORPHAN);
-    // no need to evaluate stray when dumping the dir content
-    if (!cmd_ctx) {
-           maybe_eval_stray(in);
-    }
-       }
+        CDentry *dn = p.second;
+        dn->state_set(CDentry::STATE_STRAY);
+        CDentry::linkage_t *dnl = dn->get_projected_linkage();
+        if (dnl->is_primary()) {
+          CInode *in = dnl->get_inode();
+          // only if we came from asok cmd handler
+          if (cmd_ctx) {
+            cmd_ctx->begin_dump();
+            cmd_ctx->get_formatter()->open_object_section("stray_inode");
+            cmd_ctx->get_formatter()->dump_int("ino: ", in->ino());
+            cmd_ctx->get_formatter()->dump_string("stray_prior_path: ",
+                                                  in->get_inode()->stray_prior_path);
+            in->dump(cmd_ctx->get_formatter(), CInode::DUMP_CAPS);
+            cmd_ctx->get_formatter()->close_section();
+          }
+          if (in->get_inode()->nlink == 0) {
+            in->state_set(CInode::STATE_ORPHAN);
+          }
+          // no need to evaluate stray when dumping the dir content
+          if (!cmd_ctx) {
+            maybe_eval_stray(in);
+          }
+        }
       }
     }
     next.frag = frag_t();
@@ -10362,6 +10373,7 @@ int MDCache::scan_stray_dir(dirfrag_t next, std::unique_ptr<MDCache::C_MDS_DumpS
   if (cmd_ctx) {
     cmd_ctx->end_dump();
     cmd_ctx->complete(0);
+    dout(20) << __func__ << ": done" << dendl;
   }
   return 0;
 }
@@ -10374,9 +10386,10 @@ void MDCache::fetch_backtrace(inodeno_t ino, int64_t pool, bufferlist& bl, Conte
     mds->logger->inc(l_mds_openino_backtrace_fetch);
 }
 
-int MDCache::stray_status(std::unique_ptr<C_MDS_DumpStrayDirCtx> ctx)
+void MDCache::stray_status(C_MDS_DumpStrayDirCtx *ctx)
 {
-  return scan_stray_dir(dirfrag_t(), std::move(ctx));
+  dout(20) << __func__ << dendl;
+  scan_stray_dir(dirfrag_t(), ctx);
 }
 
 // ========================================================================================

diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h
index 7a043bbdde3633d2b39b526b7f12394ffd71d51c..4e46c76dc584859845a09232b6695c10348414b4 100644 (file)
--- a/src/mds/MDCache.h
+++ b/src/mds/MDCache.h
@@ -23,6 +23,7 @@
 
 #include "common/DecayCounter.h"
 #include "common/MemoryModel.h"
+#include "common/admin_finisher.h"
 #include "include/common_fwd.h"
 #include "include/types.h"
 #include "include/filepath.h"
@@ -654,32 +655,42 @@ private:
    */
   class C_MDS_DumpStrayDirCtx : public MDSInternalContext {
   public:
-  void finish(int r) override {
-    ceph_assert(on_finish);
-    on_finish(r);
-  }
-  Formatter* get_formatter() const {
-    ceph_assert(dump_formatter);
-    return dump_formatter;
-  }
-  void begin_dump() {
-    if(!started) {
-      started = true;
-      get_formatter()->open_array_section("strays");
+    void finish(int r) override {
+      ceph_assert(on_finish);
+      bufferlist bl;
+      on_finish(r, "", bl);
     }
-  }
-  void end_dump() {
-    if(started) {
-      get_formatter()->close_section();
+
+    Formatter* get_formatter() const {
+      ceph_assert(dump_formatter);
+      return dump_formatter;
     }
-  }
-  C_MDS_DumpStrayDirCtx(MDCache *c, Formatter* f, std::function<void(int)>&& ext_on_finish) : 
-   MDSInternalContext(c->mds), cache(c), dump_formatter(f), on_finish(std::move(ext_on_finish)) {}
+
+    void begin_dump() {
+      if(!started) {
+        started = true;
+        get_formatter()->open_array_section("strays");
+      }
+    }
+
+    void end_dump() {
+      if(started) {
+        get_formatter()->close_section();
+      }
+    }
+
+    C_MDS_DumpStrayDirCtx(MDCache *c, Formatter* f, asok_finisher ext_on_finish) : 
+      MDSInternalContext(c->mds),
+      cache(c),
+      dump_formatter(f),
+      on_finish(ext_on_finish) {
+    }
+
   private:
-  MDCache *cache;
-  Formatter* dump_formatter;
-  std::function<void(int)> on_finish;
-  bool started = false;
+    MDCache *cache;
+    Formatter* dump_formatter;
+    asok_finisher on_finish;
+    bool started = false;
   };
 
   MDRequestRef lock_path(LockPathConfig config, std::function<void(MDRequestRef const& mdr)> on_locked = {});
@@ -1113,7 +1124,7 @@ private:
   void dump_tree(CInode *in, const int cur_depth, const int max_depth, Formatter *f);
 
   void cache_status(Formatter *f);
-  int stray_status(std::unique_ptr<C_MDS_DumpStrayDirCtx> ctx);
+  void stray_status(C_MDS_DumpStrayDirCtx *ctx);
 
   void dump_resolve_status(Formatter *f) const;
   void dump_rejoin_status(Formatter *f) const;
@@ -1324,7 +1335,7 @@ private:
   void handle_open_ino(const cref_t<MMDSOpenIno> &m, int err=0);
   void handle_open_ino_reply(const cref_t<MMDSOpenInoReply> &m);
 
-  int scan_stray_dir(dirfrag_t next=dirfrag_t(), std::unique_ptr<C_MDS_DumpStrayDirCtx> ctx = nullptr);
+  int scan_stray_dir(dirfrag_t next=dirfrag_t(), C_MDS_DumpStrayDirCtx *ctx = nullptr);
   // -- replicas --
   void handle_discover(const cref_t<MDiscover> &dis);
   void handle_discover_reply(const cref_t<MDiscoverReply> &m);

diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc
index 58e5196665b4088ee514d10d1b3b9ce80969d0e4..63656ac5b6a0e31129547cf55ce4b2e2d6711b8e 100644 (file)
--- a/src/mds/MDSRank.cc
+++ b/src/mds/MDSRank.cc
@@ -3108,19 +3108,9 @@ void MDSRankDispatcher::handle_asok_command(
   } else if (command == "dump stray") {
     dout(10) << "dump_stray start" <<  dendl;
     // the context is a wrapper for formatter to be used while scanning stray dir
-    auto context = std::make_unique<MDCache::C_MDS_DumpStrayDirCtx>(mdcache, f,
-     [this,on_finish](int r) {
-      // completion callback, will be called when scan is done
-      dout(10) << "dump_stray done" <<  dendl;
-      bufferlist bl;
-      on_finish(r, "", bl);
-    });
+    auto ctx = new MDCache::C_MDS_DumpStrayDirCtx(mdcache, f, on_finish);
     std::lock_guard l(mds_lock);
-    r = mdcache->stray_status(std::move(context));
-    // since the scanning op can be async, we want to know it, for better semantics
-    if (r == -EAGAIN) {
-     dout(10) << "dump_stray wait" << dendl;
-    }
+    mdcache->stray_status(ctx);
     return;
   } else {
     r = -ENOSYS;