From: Greg Farnum Date: Thu, 6 Nov 2014 19:10:29 +0000 (-0800) Subject: MDS: clean up internal MDRequests the standard way X-Git-Tag: v0.89~50^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=15d487f73da436b2e9da98e0aed9c0cbff33e845;p=ceph.git 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 --- diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc index fda1f704769f..ea6e1d2045f0 100644 --- 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 &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 d014d2859706..7e2af07432b8 100644 --- 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 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 40de91d3ee9d..2d62f4ed7105 100644 --- 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(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 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 346330936758..a5e8458e9eb4 100644 --- a/src/mds/MDSContext.h +++ b/src/mds/MDSContext.h @@ -162,6 +162,4 @@ protected: typedef C_GatherBuilderBase MDSGatherBuilder; - #endif // MDS_CONTEXT_H - diff --git a/src/mds/Server.cc b/src/mds/Server.cc index a747959d82e0..9bb274ae93f8 100644 --- 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; } -