From: Kefu Chai Date: Sun, 30 Aug 2020 09:08:55 +0000 (+0800) Subject: osd: pass pg_log_t by lvalue reference X-Git-Tag: v16.1.0~1119^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b691bf9bca81555a1e187adfbdce72c4b80e4272;p=ceph.git osd: pass pg_log_t by lvalue reference for better readability, as we intent to relinquish the ownership to the callee. Signed-off-by: Kefu Chai --- diff --git a/src/osd/PGLog.cc b/src/osd/PGLog.cc index 0a01b8478a8..b0f74f02d94 100644 --- a/src/osd/PGLog.cc +++ b/src/osd/PGLog.cc @@ -356,7 +356,7 @@ void PGLog::rewind_divergent_log(eversion_t newhead, dirty_big_info = true; } -void PGLog::merge_log(pg_info_t &oinfo, pg_log_t &olog, pg_shard_t fromosd, +void PGLog::merge_log(pg_info_t &oinfo, pg_log_t&& olog, pg_shard_t fromosd, pg_info_t &info, LogEntryHandler *rollbacker, bool &dirty_info, bool &dirty_big_info) { @@ -399,7 +399,7 @@ void PGLog::merge_log(pg_info_t &oinfo, pg_log_t &olog, pg_shard_t fromosd, // splice into our log. log.log.splice(log.log.begin(), - olog.log, from, to); + std::move(olog.log), from, to); info.log_tail = log.tail = olog.tail; changed = true; diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h index 68075ca6d92..cea890a4c69 100644 --- a/src/osd/PGLog.h +++ b/src/osd/PGLog.h @@ -1245,7 +1245,7 @@ public: bool &dirty_big_info); void merge_log(pg_info_t &oinfo, - pg_log_t &olog, + pg_log_t&& olog, pg_shard_t from, pg_info_t &info, LogEntryHandler *rollbacker, bool &dirty_info, bool &dirty_big_info); diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc index b9578277ac6..c600431600c 100644 --- a/src/osd/PeeringState.cc +++ b/src/osd/PeeringState.cc @@ -2672,12 +2672,13 @@ void PeeringState::share_pg_info() } void PeeringState::merge_log( - ObjectStore::Transaction& t, pg_info_t &oinfo, pg_log_t &olog, + ObjectStore::Transaction& t, pg_info_t &oinfo, pg_log_t&& olog, pg_shard_t from) { PGLog::LogEntryHandlerRef rollbacker{pl->get_log_handler(t)}; pg_log.merge_log( - oinfo, olog, from, info, rollbacker.get(), dirty_info, dirty_big_info); + oinfo, std::move(olog), from, info, rollbacker.get(), + dirty_info, dirty_big_info); } void PeeringState::rewind_divergent_log( @@ -2714,7 +2715,7 @@ void PeeringState::proc_primary_info( void PeeringState::proc_master_log( ObjectStore::Transaction& t, pg_info_t &oinfo, - pg_log_t &olog, pg_missing_t& omissing, pg_shard_t from) + pg_log_t&& olog, pg_missing_t&& omissing, pg_shard_t from) { psdout(10) << "proc_master_log for osd." << from << ": " << olog << " " << omissing << dendl; @@ -2724,7 +2725,7 @@ void PeeringState::proc_master_log( // make any adjustments to their missing map; we are taking their // log to be authoritative (i.e., their entries are by definitely // non-divergent). - merge_log(t, oinfo, olog, from); + merge_log(t, oinfo, std::move(olog), from); peer_info[from] = oinfo; psdout(10) << " peer osd." << from << " now " << oinfo << " " << omissing << dendl; @@ -2743,13 +2744,13 @@ void PeeringState::proc_master_log( ceph_assert(cct->_conf->osd_find_best_info_ignore_history_les || info.last_epoch_started >= info.history.last_epoch_started); - peer_missing[from].claim(omissing); + peer_missing[from].claim(std::move(omissing)); } void PeeringState::proc_replica_log( pg_info_t &oinfo, const pg_log_t &olog, - pg_missing_t& omissing, + pg_missing_t&& omissing, pg_shard_t from) { psdout(10) << "proc_replica_log for osd." << from << ": " @@ -2769,7 +2770,7 @@ void PeeringState::proc_replica_log( << " need " << i->second.need << " have " << i->second.have << dendl; } - peer_missing[from].claim(omissing); + peer_missing[from].claim(std::move(omissing)); } void PeeringState::fulfill_info( @@ -5740,7 +5741,7 @@ boost::statechart::result PeeringState::Active::react(const MLogRec& logevt) psdout(10) << "searching osd." << logevt.from << " log for unfound items" << dendl; ps->proc_replica_log( - logevt.msg->info, logevt.msg->log, logevt.msg->missing, logevt.from); + logevt.msg->info, logevt.msg->log, std::move(logevt.msg->missing), logevt.from); bool got_missing = ps->search_for_missing( ps->peer_info[logevt.from], ps->peer_missing[logevt.from], @@ -6042,7 +6043,7 @@ boost::statechart::result PeeringState::ReplicaActive::react(const MLogRec& loge DECLARE_LOCALS; psdout(10) << "received log from " << logevt.from << dendl; ObjectStore::Transaction &t = context().get_cur_transaction(); - ps->merge_log(t, logevt.msg->info, logevt.msg->log, logevt.from); + ps->merge_log(t, logevt.msg->info, std::move(logevt.msg->log), logevt.from); ceph_assert(ps->pg_log.get_head() == ps->info.last_update); if (logevt.msg->lease) { ps->proc_lease(*logevt.msg->lease); @@ -6148,7 +6149,7 @@ boost::statechart::result PeeringState::Stray::react(const MLogRec& logevt) ps->pg_log.reset_backfill(); } else { - ps->merge_log(t, msg->info, msg->log, logevt.from); + ps->merge_log(t, msg->info, std::move(msg->log), logevt.from); } if (logevt.msg->lease) { ps->proc_lease(*logevt.msg->lease); @@ -6580,7 +6581,7 @@ boost::statechart::result PeeringState::GetLog::react(const GotLog&) if (msg) { psdout(10) << "processing master log" << dendl; ps->proc_master_log(context().get_cur_transaction(), - msg->info, msg->log, msg->missing, + msg->info, std::move(msg->log), std::move(msg->missing), auth_log_shard); } ps->start_flush(context< PeeringMachine >().get_cur_transaction()); @@ -6892,7 +6893,10 @@ boost::statechart::result PeeringState::GetMissing::react(const MLogRec& logevt) DECLARE_LOCALS; peer_missing_requested.erase(logevt.from); - ps->proc_replica_log(logevt.msg->info, logevt.msg->log, logevt.msg->missing, logevt.from); + ps->proc_replica_log(logevt.msg->info, + logevt.msg->log, + std::move(logevt.msg->missing), + logevt.from); if (peer_missing_requested.empty()) { if (ps->need_up_thru) { @@ -6965,7 +6969,7 @@ boost::statechart::result PeeringState::WaitUpThru::react(const MLogRec& logevt) { DECLARE_LOCALS; psdout(10) << "Noting missing from osd." << logevt.from << dendl; - ps->peer_missing[logevt.from].claim(logevt.msg->missing); + ps->peer_missing[logevt.from].claim(std::move(logevt.msg->missing)); ps->peer_info[logevt.from] = logevt.msg->info; return discard_event(); } diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h index 710a77e9567..56f76081bf8 100644 --- a/src/osd/PeeringState.h +++ b/src/osd/PeeringState.h @@ -1577,14 +1577,14 @@ public: void rewind_divergent_log(ObjectStore::Transaction& t, eversion_t newhead); void merge_log( ObjectStore::Transaction& t, pg_info_t &oinfo, - pg_log_t &olog, pg_shard_t from); + pg_log_t&& olog, pg_shard_t from); void proc_primary_info(ObjectStore::Transaction &t, const pg_info_t &info); void proc_master_log(ObjectStore::Transaction& t, pg_info_t &oinfo, - pg_log_t &olog, pg_missing_t& omissing, + pg_log_t&& olog, pg_missing_t&& omissing, pg_shard_t from); void proc_replica_log(pg_info_t &oinfo, const pg_log_t &olog, - pg_missing_t& omissing, pg_shard_t from); + pg_missing_t&& omissing, pg_shard_t from); void calc_min_last_complete_ondisk() { eversion_t min = last_complete_ondisk; diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h index d523bd3d91a..f1bf86e445c 100644 --- a/src/osd/osd_types.h +++ b/src/osd/osd_types.h @@ -4705,10 +4705,10 @@ public: return it->second.need; } - void claim(pg_missing_set& o) { + void claim(pg_missing_set&& o) { static_assert(!TrackChanges, "Can't use claim with TrackChanges"); - missing.swap(o.missing); - rmissing.swap(o.rmissing); + missing = std::move(o.missing); + rmissing = std::move(o.rmissing); } /* diff --git a/src/test/osd/TestPGLog.cc b/src/test/osd/TestPGLog.cc index 99e22f82df9..3c45d1212e4 100644 --- a/src/test/osd/TestPGLog.cc +++ b/src/test/osd/TestPGLog.cc @@ -289,7 +289,7 @@ public: bool dirty_info = false; bool dirty_big_info = false; merge_log( - oinfo, olog, pg_shard_t(1, shard_id_t(0)), info, + oinfo, std::move(olog), pg_shard_t(1, shard_id_t(0)), info, &h, dirty_info, dirty_big_info); ASSERT_EQ(info.last_update, oinfo.last_update); @@ -890,7 +890,7 @@ TEST_F(PGLogTest, merge_log) { EXPECT_FALSE(dirty_big_info); TestHandler h(remove_snap); - merge_log(oinfo, olog, fromosd, info, &h, + merge_log(oinfo, std::move(olog), fromosd, info, &h, dirty_info, dirty_big_info); EXPECT_FALSE(missing.have_missing()); @@ -940,7 +940,7 @@ TEST_F(PGLogTest, merge_log) { EXPECT_FALSE(dirty_big_info); TestHandler h(remove_snap); - merge_log(oinfo, olog, fromosd, info, &h, + merge_log(oinfo, std::move(olog), fromosd, info, &h, dirty_info, dirty_big_info); EXPECT_FALSE(missing.have_missing()); @@ -1045,7 +1045,7 @@ TEST_F(PGLogTest, merge_log) { EXPECT_FALSE(dirty_big_info); TestHandler h(remove_snap); - merge_log(oinfo, olog, fromosd, info, &h, + merge_log(oinfo, std::move(olog), fromosd, info, &h, dirty_info, dirty_big_info); EXPECT_FALSE(missing.have_missing()); @@ -1154,7 +1154,7 @@ TEST_F(PGLogTest, merge_log) { EXPECT_FALSE(dirty_big_info); TestHandler h(remove_snap); - merge_log(oinfo, olog, fromosd, info, &h, + merge_log(oinfo, std::move(olog), fromosd, info, &h, dirty_info, dirty_big_info); /* When the divergent entry is a DELETE and the authoritative @@ -1273,7 +1273,7 @@ TEST_F(PGLogTest, merge_log) { TestHandler h(remove_snap); missing.may_include_deletes = false; - merge_log(oinfo, olog, fromosd, info, &h, + merge_log(oinfo, std::move(olog), fromosd, info, &h, dirty_info, dirty_big_info); /* When the divergent entry is a DELETE and the authoritative @@ -1375,7 +1375,7 @@ TEST_F(PGLogTest, merge_log) { TestHandler h(remove_snap); missing.may_include_deletes = false; - merge_log(oinfo, olog, fromosd, info, &h, + merge_log(oinfo, std::move(olog), fromosd, info, &h, dirty_info, dirty_big_info); EXPECT_FALSE(missing.have_missing()); diff --git a/src/test/osd/types.cc b/src/test/osd/types.cc index bed33c02d61..c452176bcfc 100644 --- a/src/test/osd/types.cc +++ b/src/test/osd/types.cc @@ -996,7 +996,7 @@ TEST(pg_missing_t, claim) pg_missing_t other; EXPECT_FALSE(other.have_missing()); - other.claim(missing); + other.claim(std::move(missing)); EXPECT_TRUE(other.have_missing()); }