From 84ca31a5ac32469631101356943b6d0b9c545bde Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Mon, 5 Aug 2019 00:45:04 +0800 Subject: [PATCH] osd: pg as a mutex since `PG` satisfies `BasicLockable`, we can just use lockers from standard library along with it. Signed-off-by: Kefu Chai --- src/osd/OSD.cc | 6 ++-- src/osd/PG.cc | 16 +++------- src/osd/PrimaryLogPG.cc | 70 +++++++++++++---------------------------- 3 files changed, 29 insertions(+), 63 deletions(-) diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index d67002909e9..a7c4d007ca3 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -2978,7 +2978,7 @@ int OSD::init() pgs.insert(pg); } for (auto pg : pgs) { - pg->lock(); + std::scoped_lock l{*pg}; set> new_children; set> merge_pgs; service.identify_splits_and_merges(pg->get_osdmap(), osdmap, pg->pg_id, @@ -2995,7 +2995,6 @@ int OSD::init() } assert(merge_pgs.empty()); } - pg->unlock(); } } @@ -9994,9 +9993,8 @@ void OSD::set_perf_queries( _get_pgs(&pgs); for (auto& pg : pgs) { if (pg->is_primary()) { - pg->lock(); + std::scoped_lock l{*pg}; pg->set_dynamic_perf_stats_queries(supported_queries); - pg->unlock(); } } } diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 238698da13c..786bc501f3a 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -493,11 +493,10 @@ Context *PG::finish_recovery() void PG::_finish_recovery(Context *c) { - lock(); + std::scoped_lock locker{*this}; // When recovery is initiated by a repair, that flag is left on state_clear(PG_STATE_REPAIR); if (recovery_state.is_deleting()) { - unlock(); return; } if (c == finish_sync_event) { @@ -517,7 +516,6 @@ void PG::_finish_recovery(Context *c) } else { dout(10) << "_finish_recovery -- stale" << dendl; } - unlock(); } void PG::start_recovery_op(const hobject_t& soid) @@ -885,10 +883,9 @@ void PG::init( void PG::shutdown() { ch->flush(); - lock(); + std::scoped_lock l{*this}; recovery_state.shutdown(); on_shutdown(); - unlock(); } #pragma GCC diagnostic ignored "-Wpragmas" @@ -3308,11 +3305,10 @@ struct FlushState { epoch_t epoch; FlushState(PG *pg, epoch_t epoch) : pg(pg), epoch(epoch) {} ~FlushState() { - pg->lock(); + std::scoped_lock l{*pg}; if (!pg->pg_has_reset_since(epoch)) { pg->recovery_state.complete_flush(); } - pg->unlock(); } }; typedef std::shared_ptr FlushStateRef; @@ -3739,12 +3735,11 @@ void PG::do_delete_work(ObjectStore::Transaction &t) dout(20) << __func__ << " wake up at " << ceph_clock_now() << ", re-queuing delete" << dendl; - lock(); + std::scoped_lock locker{*this}; delete_needs_sleep = false; if (!pg_has_reset_since(e)) { osd->queue_for_pg_delete(get_pgid(), e); } - unlock(); }); auto delete_schedule_time = ceph::real_clock::now(); @@ -3871,9 +3866,8 @@ ostream& operator<<(ostream& out, const PG::BackfillInterval& bi) void PG::dump_pgstate_history(Formatter *f) { - lock(); + std::scoped_lock l{*this}; recovery_state.dump_history(f); - unlock(); } void PG::dump_missing(Formatter *f) diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index c7652d316f5..63314c39fc2 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -118,12 +118,11 @@ public: BlessedGenContext(PrimaryLogPG *pg, GenContext *c, epoch_t e) : pg(pg), c(c), e(e) {} void finish(T t) override { - pg->lock(); + std::scoped_lock locker{*pg}; if (pg->pg_has_reset_since(e)) c.reset(); else c.release()->complete(t); - pg->unlock(); } bool sync_finish(T t) { // we assume here all blessed/wrapped Contexts can complete synchronously. @@ -173,12 +172,11 @@ public: BlessedContext(PrimaryLogPG *pg, Context *c, epoch_t e) : pg(pg), c(c), e(e) {} void finish(int r) override { - pg->lock(); + std::scoped_lock locker{*pg}; if (pg->pg_has_reset_since(e)) c.reset(); else c.release()->complete(r); - pg->unlock(); } bool sync_finish(int r) { // we assume here all blessed/wrapped Contexts can complete synchronously. @@ -225,9 +223,8 @@ class PrimaryLogPG::C_OSD_AppliedRecoveredObject : public Context { return true; } void finish(int r) override { - pg->lock(); + std::scoped_lock locker{*pg}; pg->_applied_recovered_object(obc); - pg->unlock(); } }; @@ -255,9 +252,8 @@ class PrimaryLogPG::C_OSD_AppliedRecoveredObjectReplica : public Context { return true; } void finish(int r) override { - pg->lock(); + std::scoped_lock locker{*pg}; pg->_applied_recovered_object_replica(); - pg->unlock(); } }; @@ -2375,10 +2371,9 @@ struct C_ManifestFlush : public Context { void finish(int r) override { if (r == -ECANCELED) return; - pg->lock(); + std::scoped_lock locker{*pg}; pg->handle_manifest_flush(oid, tid, r, offset, last_offset, lpr); pg->osd->logger->tinc(l_osd_tier_flush_lat, ceph_clock_now() - start); - pg->unlock(); } }; @@ -2889,16 +2884,14 @@ struct C_ProxyRead : public Context { void finish(int r) override { if (prdop->canceled) return; - pg->lock(); + std::scoped_lock locker{*pg}; if (prdop->canceled) { - pg->unlock(); return; } if (last_peering_reset == pg->get_last_peering_reset()) { pg->finish_proxy_read(oid, tid, r); pg->osd->logger->tinc(l_osd_tier_r_lat, ceph_clock_now() - start); } - pg->unlock(); } }; @@ -2922,9 +2915,8 @@ struct C_ProxyChunkRead : public Context { void finish(int r) override { if (prdop->canceled) return; - pg->lock(); + std::scoped_lock locker{*pg}; if (prdop->canceled) { - pg->unlock(); return; } if (last_peering_reset == pg->get_last_peering_reset()) { @@ -2953,7 +2945,6 @@ struct C_ProxyChunkRead : public Context { delete obj_op; } } - pg->unlock(); } }; @@ -3160,15 +3151,13 @@ struct C_ProxyWrite_Commit : public Context { void finish(int r) override { if (pwop->canceled) return; - pg->lock(); + std::scoped_lock locker{*pg}; if (pwop->canceled) { - pg->unlock(); return; } if (last_peering_reset == pg->get_last_peering_reset()) { pg->finish_proxy_write(oid, tid, r); } - pg->unlock(); } }; @@ -3297,7 +3286,7 @@ public: : pg(pg), ctx(ctx), osd_op(osd_op), last_peering_reset(lpr) {} void finish(int r) override { - pg->lock(); + std::scoped_lock locker{*pg}; if (last_peering_reset == pg->get_last_peering_reset()) { if (r >= 0) { osd_op.rval = 0; @@ -3309,7 +3298,6 @@ public: pg->close_op_ctx(ctx); } } - pg->unlock(); } }; @@ -8566,12 +8554,11 @@ struct C_Copyfrom : public Context { void finish(int r) override { if (r == -ECANCELED) return; - pg->lock(); + std::scoped_lock l{*pg}; if (last_peering_reset == pg->get_last_peering_reset()) { pg->process_copy_chunk(oid, tid, r); cop.reset(); } - pg->unlock(); } }; @@ -8612,12 +8599,11 @@ struct C_CopyChunk : public Context { void finish(int r) override { if (r == -ECANCELED) return; - pg->lock(); + std::scoped_lock l{*pg}; if (last_peering_reset == pg->get_last_peering_reset()) { pg->process_copy_chunk_manifest(oid, tid, r, offset); cop.reset(); } - pg->unlock(); } }; @@ -9828,12 +9814,11 @@ struct C_Flush : public Context { void finish(int r) override { if (r == -ECANCELED) return; - pg->lock(); + std::scoped_lock locker{*pg}; if (last_peering_reset == pg->get_last_peering_reset()) { pg->finish_flush(oid, tid, r); pg->osd->logger->tinc(l_osd_tier_flush_lat, ceph_clock_now() - start); } - pg->unlock(); } }; @@ -10640,7 +10625,7 @@ void PrimaryLogPG::submit_log_entries( epoch_t epoch) : pg(pg), rep_tid(rep_tid), epoch(epoch) {} void finish(int) override { - pg->lock(); + std::scoped_lock l{*pg}; if (!pg->pg_has_reset_since(epoch)) { auto it = pg->log_entry_update_waiting_on.find(rep_tid); ceph_assert(it != pg->log_entry_update_waiting_on.end()); @@ -10652,7 +10637,6 @@ void PrimaryLogPG::submit_log_entries( pg->log_entry_update_waiting_on.erase(it); } } - pg->unlock(); } }; t.register_on_commit( @@ -10676,13 +10660,12 @@ void PrimaryLogPG::cancel_log_updates() void PrimaryLogPG::get_watchers(list *ls) { - lock(); + std::scoped_lock l{*this}; pair i; while (object_contexts.get_next(i.first, &i)) { ObjectContextRef obc(i.second); get_obc_watchers(obc, *ls); } - unlock(); } void PrimaryLogPG::get_obc_watchers(ObjectContextRef obc, list &pg_watchers) @@ -11348,7 +11331,7 @@ int PrimaryLogPG::recover_missing( epoch_t cur_epoch = get_osdmap_epoch(); remove_missing_object(soid, v, new FunctionContext( [=](int) { - lock(); + std::scoped_lock locker{*this}; if (!pg_has_reset_since(cur_epoch)) { bool object_missing = false; for (const auto& shard : get_acting_recovery_backfill()) { @@ -11372,7 +11355,6 @@ int PrimaryLogPG::recover_missing( pgbackend->run_recovery_op(recovery_handle, priority); } } - unlock(); })); return PULL_YES; } @@ -11432,16 +11414,16 @@ void PrimaryLogPG::remove_missing_object(const hobject_t &soid, epoch_t cur_epoch = get_osdmap_epoch(); t.register_on_complete(new FunctionContext( [=](int) { - lock(); + std::unique_lock locker{*this}; if (!pg_has_reset_since(cur_epoch)) { ObjectStore::Transaction t2; on_local_recover(soid, recovery_info, ObjectContextRef(), true, &t2); t2.register_on_complete(on_complete); int r = osd->store->queue_transaction(ch, std::move(t2), nullptr); ceph_assert(r == 0); - unlock(); + locker.unlock(); } else { - unlock(); + locker.unlock(); on_complete->complete(-EAGAIN); } })); @@ -11472,15 +11454,13 @@ void PrimaryLogPG::finish_degraded_object(const hobject_t& oid) void PrimaryLogPG::_committed_pushed_object( epoch_t epoch, eversion_t last_complete) { - lock(); + std::scoped_lock locker{*this}; if (!pg_has_reset_since(epoch)) { recovery_state.recovery_committed_to(last_complete); } else { dout(10) << __func__ << " pg has changed, not touching last_complete_ondisk" << dendl; } - - unlock(); } void PrimaryLogPG::_applied_recovered_object(ObjectContextRef obc) @@ -11607,7 +11587,7 @@ void PrimaryLogPG::do_update_log_missing(OpRequestRef &op) [=](int) { const MOSDPGUpdateLogMissing *msg = static_cast( op->get_req()); - lock(); + std::scoped_lock locker{*this}; if (!pg_has_reset_since(msg->get_epoch())) { update_last_complete_ondisk(new_lcod); MOSDPGUpdateLogMissingReply *reply = @@ -11621,7 +11601,6 @@ void PrimaryLogPG::do_update_log_missing(OpRequestRef &op) reply->set_priority(CEPH_MSG_PRIO_HIGH); msg->get_connection()->send_message(reply); } - unlock(); }); if (get_osdmap()->require_osd_release >= ceph_release_t::kraken) { @@ -13782,10 +13761,9 @@ void PrimaryLogPG::agent_clear() // Return false if no objects operated on since start of object hash space bool PrimaryLogPG::agent_work(int start_max, int agent_flush_quota) { - lock(); + std::scoped_lock locker{*this}; if (!agent_state) { dout(10) << __func__ << " no agent state, stopping" << dendl; - unlock(); return true; } @@ -13793,7 +13771,6 @@ bool PrimaryLogPG::agent_work(int start_max, int agent_flush_quota) if (agent_state->is_idle()) { dout(10) << __func__ << " idle, stopping" << dendl; - unlock(); return true; } @@ -13939,11 +13916,9 @@ bool PrimaryLogPG::agent_work(int start_max, int agent_flush_quota) if (need_delay) { ceph_assert(agent_state->delaying == false); agent_delay(); - unlock(); return false; } agent_choose_mode(); - unlock(); return true; } @@ -14188,12 +14163,11 @@ void PrimaryLogPG::agent_delay() void PrimaryLogPG::agent_choose_mode_restart() { dout(20) << __func__ << dendl; - lock(); + std::scoped_lock locker{*this}; if (agent_state && agent_state->delaying) { agent_state->delaying = false; agent_choose_mode(true); } - unlock(); } bool PrimaryLogPG::agent_choose_mode(bool restart, OpRequestRef op) -- 2.47.3