From: Yan, Zheng Date: Fri, 19 Jun 2020 02:37:01 +0000 (+0800) Subject: mds: remove on_finish from {CInode,CDir}::scrub_info_t X-Git-Tag: v16.1.0~577^2~8 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=03908aa04344355558c2cab3910418909b75fe2a;p=ceph.git mds: remove on_finish from {CInode,CDir}::scrub_info_t A CInode/CDir is scrubbed no longer means corresponding subtree is fully scrubbed. The on_finish in {CInode,CDir}::scrub_info_t become useless. This patch also removes code that flushs journal if scrub has repaired anything. Later patch will add the code back at different place. Signed-off-by: "Yan, Zheng" --- diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc index 5073c6b4ea3..58149e653b2 100644 --- a/src/mds/CDir.cc +++ b/src/mds/CDir.cc @@ -3473,30 +3473,26 @@ void CDir::scrub_info_create() const me->scrub_infop.swap(si); } -void CDir::scrub_initialize(const ScrubHeaderRef& header, MDSContext* f) +void CDir::scrub_initialize(const ScrubHeaderRef& header) { ceph_assert(header); // FIXME: weird implicit construction, is someone else meant // to be calling scrub_info_create first? scrub_info(); scrub_infop->header = header; - scrub_infop->on_finish = f; scrub_infop->directory_scrubbing = true; } -void CDir::scrub_aborted(MDSContext **c) { +void CDir::scrub_aborted() { dout(20) << __func__ << dendl; ceph_assert(scrub_is_in_progress()); - *c = scrub_infop->on_finish; - scrub_infop->on_finish = nullptr; - scrub_infop->directory_scrubbing = false; scrub_infop->last_scrub_dirty = false; scrub_infop.reset(); } -void CDir::scrub_finished(MDSContext **c) +void CDir::scrub_finished() { dout(20) << __func__ << dendl; ceph_assert(scrub_is_in_progress()); @@ -3508,9 +3504,6 @@ void CDir::scrub_finished(MDSContext **c) scrub_infop->last_recursive = scrub_infop->last_local; scrub_infop->last_scrub_dirty = true; - - *c = scrub_infop->on_finish; - scrub_infop->on_finish = nullptr; } void CDir::scrub_maybe_delete_info() diff --git a/src/mds/CDir.h b/src/mds/CDir.h index db9dee02801..581f3a818f8 100644 --- a/src/mds/CDir.h +++ b/src/mds/CDir.h @@ -96,8 +96,6 @@ public: scrub_info_t() {} - MDSContext *on_finish = nullptr; - scrub_stamps last_recursive; // when we last finished a recursive scrub scrub_stamps last_local; // when we last did a local scrub @@ -299,7 +297,7 @@ public: * @pre The CDir is marked complete. * @post It has set up its internal scrubbing state. */ - void scrub_initialize(const ScrubHeaderRef& header, MDSContext* f); + void scrub_initialize(const ScrubHeaderRef& header); ScrubHeaderRef get_scrub_header() { return scrub_infop ? scrub_infop->header : nullptr; } @@ -312,9 +310,9 @@ public: * Call this once all CDentries have been scrubbed, according to * scrub_dentry_next's listing. It finalizes the scrub statistics. */ - void scrub_finished(MDSContext **c); + void scrub_finished(); - void scrub_aborted(MDSContext **c); + void scrub_aborted(); /** * Tell the CDir to do a local scrub of itself. * @pre The CDir is_complete(). diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc index 11617ff8206..c812b7bfd2d 100644 --- a/src/mds/CInode.cc +++ b/src/mds/CInode.cc @@ -5109,31 +5109,26 @@ void CInode::scrub_maybe_delete_info() } } -void CInode::scrub_initialize(ScrubHeaderRef& header, - MDSContext *f) +void CInode::scrub_initialize(ScrubHeaderRef& header) { dout(20) << __func__ << " with scrub_version " << get_version() << dendl; scrub_info(); - scrub_infop->on_finish = f; scrub_infop->scrub_in_progress = true; scrub_infop->queued_frags.clear(); scrub_infop->header = header; // right now we don't handle remote inodes } -void CInode::scrub_aborted(MDSContext **c) { +void CInode::scrub_aborted() { dout(20) << __func__ << dendl; ceph_assert(scrub_is_in_progress()); - *c = nullptr; - std::swap(*c, scrub_infop->on_finish); - scrub_infop->scrub_in_progress = false; scrub_maybe_delete_info(); } -void CInode::scrub_finished(MDSContext **c) { +void CInode::scrub_finished() { dout(20) << __func__ << dendl; ceph_assert(scrub_is_in_progress()); @@ -5141,16 +5136,6 @@ void CInode::scrub_finished(MDSContext **c) { scrub_infop->last_scrub_stamp = ceph_clock_now(); scrub_infop->last_scrub_dirty = true; scrub_infop->scrub_in_progress = false; - - *c = scrub_infop->on_finish; - scrub_infop->on_finish = NULL; - - if (scrub_infop->header->get_origin() == this) { - // We are at the point that a tagging scrub was initiated - LogChannelRef clog = mdcache->mds->clog; - clog->info() << "scrub complete with tag '" - << scrub_infop->header->get_tag() << "'"; - } } int64_t CInode::get_backtrace_pool() const diff --git a/src/mds/CInode.h b/src/mds/CInode.h index 470fcfaec8e..44e71f0b6a8 100644 --- a/src/mds/CInode.h +++ b/src/mds/CInode.h @@ -298,8 +298,6 @@ class CInode : public MDSCacheObject, public InodeStoreBase, public Counterqueued_frags; } - void scrub_set_finisher(MDSContext *c) { - ceph_assert(!scrub_infop->on_finish); - scrub_infop->on_finish = c; - } - bool is_multiversion() const { return snaprealm || // other snaprealms will link to me get_inode()->is_dir() || // links to me in other snaps diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc index 2868dc3686e..d8de397d69f 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -12718,12 +12718,6 @@ public: C_MDS_EnqueueScrub(std::string_view tag, Formatter *f, Context *fin) : tag(tag), formatter(f), on_finish(fin), header(nullptr) {} - Context *take_finisher() { - Context *fin = on_finish; - on_finish = NULL; - return fin; - } - void finish(int r) override { if (r == 0) { // since recursive scrub is asynchronous, dump minimal output @@ -12772,8 +12766,7 @@ void MDCache::enqueue_scrub( } C_MDS_EnqueueScrub *cs = new C_MDS_EnqueueScrub(tag_str, f, fin); - cs->header = std::make_shared( - tag_str, is_internal, force, recursive, repair, f); + cs->header = std::make_shared(tag_str, is_internal, force, recursive, repair); mdr->internal_op_finish = cs; enqueue_scrub_work(mdr); @@ -12790,57 +12783,11 @@ void MDCache::enqueue_scrub_work(MDRequestRef& mdr) C_MDS_EnqueueScrub *cs = static_cast(mdr->internal_op_finish); ScrubHeaderRef header = cs->header; - - // Cannot scrub same dentry twice at same time - if (in->scrub_is_in_progress()) { - mds->server->respond_to_request(mdr, -EBUSY); - return; - } else { - in->scrub_info(); - } - header->set_origin(in); - Context *fin = nullptr; - if (!header->get_recursive()) - fin = cs->take_finisher(); - - // If the scrub did some repair, then flush the journal at the end of - // the scrub. Otherwise in the case of e.g. rewriting a backtrace - // the on disk state will still look damaged. - auto scrub_finish = new LambdaContext([this, header, fin](int r){ - if (!header->get_repaired()) { - if (fin) - fin->complete(r); - return; - } - - auto flush_finish = new LambdaContext([this, fin](int r){ - dout(4) << "Expiring log segments because scrub did some repairs" << dendl; - mds->mdlog->trim_all(); + int r = mds->scrubstack->enqueue(in, header, !header->get_recursive()); - if (fin) { - MDSGatherBuilder gather(g_ceph_context); - auto& expiring_segments = mds->mdlog->get_expiring_segments(); - for (auto logseg : expiring_segments) - logseg->wait_for_expiry(gather.new_sub()); - ceph_assert(gather.has_subs()); - gather.set_finisher(new MDSInternalContextWrapper(mds, fin)); - gather.activate(); - } - }); - - dout(4) << "Flushing journal because scrub did some repairs" << dendl; - mds->mdlog->start_new_segment(); - mds->mdlog->flush(); - mds->mdlog->wait_for_safe(new MDSInternalContextWrapper(mds, flush_finish)); - }); - - mds->scrubstack->enqueue(in, header, - new MDSInternalContextWrapper(mds, scrub_finish), - !header->get_recursive()); - - mds->server->respond_to_request(mdr, 0); + mds->server->respond_to_request(mdr, r); return; } diff --git a/src/mds/ScrubHeader.h b/src/mds/ScrubHeader.h index 0925c28d6b8..ecf24c1abcb 100644 --- a/src/mds/ScrubHeader.h +++ b/src/mds/ScrubHeader.h @@ -35,9 +35,9 @@ class CInode; class ScrubHeader { public: ScrubHeader(std::string_view tag_, bool is_tag_internal_, bool force_, - bool recursive_, bool repair_, ceph::Formatter *f_) + bool recursive_, bool repair_) : tag(tag_), is_tag_internal(is_tag_internal_), force(force_), - recursive(recursive_), repair(repair_), formatter(f_) {} + recursive(recursive_), repair(repair_) {} // Set after construction because it won't be known until we've // started resolving path and locking @@ -49,7 +49,6 @@ public: bool is_internal_tag() const { return is_tag_internal; } CInode *get_origin() const { return origin; } const std::string& get_tag() const { return tag; } - ceph::Formatter& get_formatter() const { return *formatter; } bool get_repaired() const { return repaired; } void set_repaired() { repaired = true; } @@ -60,7 +59,6 @@ protected: const bool force; const bool recursive; const bool repair; - ceph::Formatter* const formatter; CInode *origin = nullptr; bool repaired = false; // May be set during scrub if repairs happened diff --git a/src/mds/ScrubStack.cc b/src/mds/ScrubStack.cc index f454f2334ca..d5c04f66344 100644 --- a/src/mds/ScrubStack.cc +++ b/src/mds/ScrubStack.cc @@ -58,20 +58,27 @@ void ScrubStack::dequeue(MDSCacheObject *obj) stack_size--; } -void ScrubStack::_enqueue(MDSCacheObject *obj, ScrubHeaderRef& header, - MDSContext *on_finish, bool top) +int ScrubStack::_enqueue(MDSCacheObject *obj, ScrubHeaderRef& header, bool top) { ceph_assert(ceph_mutex_is_locked_by_me(mdcache->mds->mds_lock)); if (CInode *in = dynamic_cast(obj)) { - dout(10) << __func__ << " with {" << *in << "}" - << ", on_finish=" << on_finish << ", top=" << top << dendl; - in->scrub_initialize(header, on_finish); + if (in->scrub_is_in_progress()) { + dout(10) << __func__ << " with {" << *in << "}" << ", already in scrubbing" << dendl; + return -EBUSY; + } + + dout(10) << __func__ << " with {" << *in << "}" << ", top=" << top << dendl; + in->scrub_initialize(header); } else if (CDir *dir = dynamic_cast(obj)) { - dout(10) << __func__ << " with {" << *dir << "}" - << ", on_finish=" << on_finish << ", top=" << top << dendl; + if (dir->scrub_is_in_progress()) { + dout(10) << __func__ << " with {" << *dir << "}" << ", already in scrubbing" << dendl; + return -EBUSY; + } + + dout(10) << __func__ << " with {" << *dir << "}" << ", top=" << top << dendl; // The edge directory must be in memory dir->auth_pin(this); - dir->scrub_initialize(header, on_finish); + dir->scrub_initialize(header); } else { ceph_assert(0 == "queue dentry to scrub stack"); } @@ -87,20 +94,21 @@ void ScrubStack::_enqueue(MDSCacheObject *obj, ScrubHeaderRef& header, scrub_stack.push_back(&obj->item_scrub); } -void ScrubStack::enqueue(CInode *in, ScrubHeaderRef& header, - MDSContext *on_finish, bool top) +void ScrubStack::enqueue(CInode *in, ScrubHeaderRef& header, bool top) { // abort in progress - if (clear_stack) { - on_finish->complete(-EAGAIN); - return; - } + if (clear_stack) + return -EAGAIN; scrub_origins.emplace(in); clog_scrub_summary(in); - _enqueue(in, header, on_finish, top); + int r = _enqueue(in, header, top); + if (r < 0) + return r; + kick_off_scrubs(); + return 0; } void ScrubStack::add_to_waiting(MDSCacheObject *obj) @@ -187,10 +195,6 @@ void ScrubStack::kick_off_scrubs() // it's a regular file, symlink, or hard link dequeue(in); // we only touch it this once, so remove from stack - if (!in->scrub_info()->on_finish) { - scrubs_in_progress++; - in->scrub_set_finisher(&scrub_kick); - } scrub_file_inode(in); } else { bool added_children = false; @@ -307,7 +311,7 @@ void ScrubStack::scrub_dir_inode(CInode *in, bool *added_children, bool *done) dout(20) << __func__ << " barebones " << *dir << dendl; dir->fetch(gather.new_sub()); } else { - _enqueue(dir, header, nullptr, true); + _enqueue(dir, header, true); queued.insert_raw(dir->get_frag()); *added_children = true; } @@ -352,33 +356,29 @@ void ScrubStack::scrub_dir_inode(CInode *in, bool *added_children, bool *done) class C_InodeValidated : public MDSInternalContext { - public: - ScrubStack *stack; - CInode::validated_data result; - CInode *target; - - C_InodeValidated(MDSRank *mds, ScrubStack *stack_, CInode *target_) - : MDSInternalContext(mds), stack(stack_), target(target_) - {} +public: + ScrubStack *stack; + CInode::validated_data result; + CInode *target; - void finish(int r) override - { - stack->_validate_inode_done(target, r, result); - } + C_InodeValidated(MDSRank *mds, ScrubStack *stack_, CInode *target_) + : MDSInternalContext(mds), stack(stack_), target(target_) + { + stack->scrubs_in_progress++; + } + void finish(int r) override { + stack->_validate_inode_done(target, r, result); + stack->scrubs_in_progress--; + stack->kick_off_scrubs(); + } }; void ScrubStack::scrub_dir_inode_final(CInode *in) { dout(20) << __func__ << " " << *in << dendl; - if (!in->scrub_info()->on_finish) { - scrubs_in_progress++; - in->scrub_set_finisher(&scrub_kick); - } - C_InodeValidated *fin = new C_InodeValidated(mdcache->mds, this, in); in->validate_disk_state(&fin->result, fin); - return; } @@ -410,7 +410,7 @@ void ScrubStack::scrub_dirfrag(CDir *dir, bool *done) continue; } if (dnl->is_primary()) { - _enqueue(dnl->get_inode(), header, nullptr, false); + _enqueue(dnl->get_inode(), header, false); } else if (dnl->is_remote()) { // TODO: check remote linkage } @@ -419,11 +419,8 @@ void ScrubStack::scrub_dirfrag(CDir *dir, bool *done) dir->scrub_local(); - MDSContext *c = nullptr; - dir->scrub_finished(&c); + dir->scrub_finished(); dir->auth_unpin(this); - if (c) - finisher->queue(new MDSIOContextWrapper(mdcache->mds, c), 0); *done = true; dout(10) << __func__ << " done" << dendl; @@ -489,28 +486,13 @@ void ScrubStack::_validate_inode_done(CInode *in, int r, dout(10) << __func__ << " scrub passed on inode " << *in << dendl; } - MDSContext *c = nullptr; - in->scrub_finished(&c); - if (c) - finisher->queue(new MDSIOContextWrapper(mdcache->mds, c), 0); - if (in == header->get_origin()) { scrub_origins.erase(in); clog_scrub_summary(in); - if (!header->get_recursive() && header->get_formatter()) { - if (r >= 0) { // we got into the scrubbing dump it - result.dump(header->get_formatter()); - } else { // we failed the lookup or something; dump ourselves - header->get_formatter()->open_object_section("results"); - header->get_formatter()->dump_int("return_code", r); - header->get_formatter()->close_section(); // results - } - } } -} -ScrubStack::C_KickOffScrubs::C_KickOffScrubs(ScrubStack *s) - : MDSInternalContext(s->mdcache->mds), stack(s) { } + in->scrub_finished(); +} void ScrubStack::complete_control_contexts(int r) { ceph_assert(ceph_mutex_is_locked_by_me(mdcache->mds->mds_lock)); @@ -670,18 +652,10 @@ void ScrubStack::abort_pending_scrubs() { scrub_origins.erase(in); clog_scrub_summary(in); } - MDSContext *ctx = nullptr; - in->scrub_aborted(&ctx); - if (ctx != nullptr) { - ctx->complete(-ECANCELED); - } + in->scrub_aborted(); } else if (CDir *dir = dynamic_cast(obj)) { - MDSContext *ctx = nullptr; - dir->scrub_aborted(&ctx); + dir->scrub_aborted(); dir->auth_unpin(this); - if (ctx != nullptr) { - ctx->complete(-ECANCELED); - } } else { ceph_abort(0 == "dentry in scrub stack"); } @@ -839,7 +813,7 @@ void ScrubStack::handle_scrub(const cref_t &m) if (!dfs.empty()) { ScrubHeaderRef header = std::make_shared(m->get_tag(), m->is_internal_tag(), m->is_force(), m->is_recursive(), - m->is_repair(), nullptr); + m->is_repair()); for (auto dir : dfs) { queued.insert_raw(dir->get_frag()); _enqueue(dir, header, nullptr, true); @@ -881,9 +855,9 @@ void ScrubStack::handle_scrub(const cref_t &m) ScrubHeaderRef header = std::make_shared(m->get_tag(), m->is_internal_tag(), m->is_force(), m->is_recursive(), - m->is_repair(), nullptr); + m->is_repair()); - _enqueue(in, header, nullptr, true); + _enqueue(in, header, true); in->scrub_queued_frags() = m->get_frags(); kick_off_scrubs(); @@ -911,10 +885,7 @@ void ScrubStack::handle_scrub(const cref_t &m) scrub_origins.erase(in); clog_scrub_summary(in); } - MDSContext *c = nullptr; - in->scrub_finished(&c); - if (c) - finisher->queue(new MDSIOContextWrapper(mdcache->mds, c), 0); + in->scrub_finished(); kick_off_scrubs(); } diff --git a/src/mds/ScrubStack.h b/src/mds/ScrubStack.h index 592b4e9ef64..4bd725390d4 100644 --- a/src/mds/ScrubStack.h +++ b/src/mds/ScrubStack.h @@ -35,8 +35,7 @@ public: clog(clog), finisher(finisher_), scrub_stack(member_offset(MDSCacheObject, item_scrub)), - scrub_waiting(member_offset(MDSCacheObject, item_scrub)), - scrub_kick(this) {} + scrub_waiting(member_offset(MDSCacheObject, item_scrub)) {} ~ScrubStack() { ceph_assert(scrub_stack.empty()); ceph_assert(!scrubs_in_progress); @@ -47,8 +46,7 @@ public: * @param in The inode to scrub * @param header The ScrubHeader propagated from wherever this scrub */ - void enqueue(CInode *in, ScrubHeaderRef& header, - MDSContext *on_finish, bool top); + void enqueue(CInode *in, ScrubHeaderRef& header, bool top); /** * Abort an ongoing scrub operation. The abort operation could be * delayed if there are in-progress scrub operations on going. The @@ -123,24 +121,6 @@ protected: }; std::map remote_scrubs; - class C_KickOffScrubs : public MDSInternalContext { - public: - C_KickOffScrubs(ScrubStack *s); - void finish(int r) override { } - void complete(int r) override { - if (r == -ECANCELED) { - return; - } - - stack->scrubs_in_progress--; - stack->kick_off_scrubs(); - // don't delete self - } - private: - ScrubStack *stack; - }; - C_KickOffScrubs scrub_kick; - friend class C_RetryScrub; private: // scrub abort is _not_ a state, rather it's an operation that's @@ -155,8 +135,7 @@ private: friend class C_InodeValidated; - void _enqueue(MDSCacheObject *obj, ScrubHeaderRef& header, - MDSContext *on_finish, bool top); + int _enqueue(MDSCacheObject *obj, ScrubHeaderRef& header, bool top); /** * Remove the inode/dirfrag from the stack. */