From 3c8e115e93c4b909eb1f72a33963f35557e5c087 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 3 Jan 2021 12:30:12 +0800 Subject: [PATCH] osd/scrub_machine: pass an instance of event to post_event() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit instead of passing an intrusive_ptr<> to it, pass an instance of event to post_event(). for couple reasons: * post_event() is able to create an intrusive_ptr<> from the event passed to it using event.intrusive_from_this() which creates a clone of the original event. but since the overhead of creating scrub events is relative low. and presumably, it is not the bottleneck of the overall performance of Ceph, so it should be fine. * simpler this way. so we don't need to repeat `boost::intrusive_ptr(new EventName{})` * more consistent this way. we use the same variant of `post_event()` elsewhere. * for silencing the warning from GCC-11, like: ../src/osd/scrub_machine.cc:323:64: note: returned from ‘static void* boost::statechart::event::operator new(std::size_t) [with MostDerived = Scrub::GotReplicas; Allocator = std::allocator]’ ../src/osd/scrub_machine.cc: In constructor ‘Scrub::WaitReplicas::WaitReplicas(boost::statechart::state::my_context)’: ../src/osd/scrub_machine.cc:346:64: warning: ‘static void boost::statechart::event::operator delete(void*) [with MostDerived = Scrub::GotReplicas; Allocator = std::allocator]’ called on pointer returned from a mismatched allocation function [-Wmismatched-new-delete] 346 | post_event(boost::intrusive_ptr(new GotReplicas{})); | this warning is a false alarm. but it is distracting. Signed-off-by: Kefu Chai --- src/osd/scrub_machine.cc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/osd/scrub_machine.cc b/src/osd/scrub_machine.cc index f9c3a0495fa..42133450d7e 100644 --- a/src/osd/scrub_machine.cc +++ b/src/osd/scrub_machine.cc @@ -190,11 +190,11 @@ NewChunk::NewChunk(my_context ctx) : my_base(ctx) bool got_a_chunk = scrbr->select_range(); if (got_a_chunk) { dout(15) << __func__ << " selection OK" << dendl; - post_event(boost::intrusive_ptr(new SelectedChunkFree{})); + post_event(SelectedChunkFree{}); } else { dout(10) << __func__ << " selected chunk is busy" << dendl; // wait until we are available (transitioning to Blocked) - post_event(boost::intrusive_ptr(new ChunkIsBusy{})); + post_event(ChunkIsBusy{}); } } @@ -212,7 +212,7 @@ sc::result NewChunk::react(const SelectedChunkFree&) WaitPushes::WaitPushes(my_context ctx) : my_base(ctx) { dout(10) << " -- state -->> Act/WaitPushes" << dendl; - post_event(boost::intrusive_ptr(new ActivePushesUpd{})); + post_event(ActivePushesUpd{}); } /* @@ -237,7 +237,7 @@ sc::result WaitPushes::react(const ActivePushesUpd&) WaitLastUpdate::WaitLastUpdate(my_context ctx) : my_base(ctx) { dout(10) << " -- state -->> Act/WaitLastUpdate" << dendl; - post_event(boost::intrusive_ptr(new UpdatesApplied{})); + post_event(UpdatesApplied{}); } void WaitLastUpdate::on_new_updates(const UpdatesApplied&) @@ -246,7 +246,7 @@ void WaitLastUpdate::on_new_updates(const UpdatesApplied&) dout(10) << "WaitLastUpdate::on_new_updates(const UpdatesApplied&)" << dendl; if (scrbr->has_pg_marked_new_updates()) { - post_event(boost::intrusive_ptr(new InternalAllUpdates{})); + post_event(InternalAllUpdates{}); } else { // will be requeued by op_applied dout(10) << "wait for EC read/modify/writes to queue" << dendl; @@ -280,7 +280,7 @@ BuildMap::BuildMap(my_context ctx) : my_base(ctx) // we were preempted, either directly or by a replica dout(10) << __func__ << " preempted!!!" << dendl; scrbr->mark_local_map_ready(); - post_event(boost::intrusive_ptr(new IntBmPreempted{})); + post_event(IntBmPreempted{}); } else { @@ -295,12 +295,12 @@ BuildMap::BuildMap(my_context ctx) : my_base(ctx) dout(10) << "BuildMap::BuildMap() Error! Aborting. Ret: " << ret << dendl; // scrbr->mark_local_map_ready(); - post_event(boost::intrusive_ptr(new InternalError{})); + post_event(InternalError{}); } else { // the local map was created - post_event(boost::intrusive_ptr(new IntLocalMapDone{})); + post_event(IntLocalMapDone{}); } } } @@ -320,7 +320,7 @@ DrainReplMaps::DrainReplMaps(my_context ctx) : my_base(ctx) { dout(10) << "-- state -->> Act/DrainReplMaps" << dendl; // we may have received all maps already. Send the event that will make us check. - post_event(boost::intrusive_ptr(new GotReplicas{})); + post_event(GotReplicas{}); } sc::result DrainReplMaps::react(const GotReplicas&) @@ -343,7 +343,7 @@ sc::result DrainReplMaps::react(const GotReplicas&) WaitReplicas::WaitReplicas(my_context ctx) : my_base(ctx) { dout(10) << "-- state -->> Act/WaitReplicas" << dendl; - post_event(boost::intrusive_ptr(new GotReplicas{})); + post_event(GotReplicas{}); } sc::result WaitReplicas::react(const GotReplicas&) @@ -379,7 +379,7 @@ WaitDigestUpdate::WaitDigestUpdate(my_context ctx) : my_base(ctx) // perform an initial check: maybe we already // have all the updates we need: // (note that DigestUpdate is usually an external event) - post_event(boost::intrusive_ptr(new DigestUpdate{})); + post_event(DigestUpdate{}); } sc::result WaitDigestUpdate::react(const DigestUpdate&) @@ -473,7 +473,7 @@ ActiveReplica::ActiveReplica(my_context ctx) : my_base(ctx) DECLARE_LOCALS; // 'scrbr' & 'pg_id' aliases dout(10) << "-- state -->> ActiveReplica" << dendl; scrbr->on_replica_init(); // as we might have skipped ReplicaWaitUpdates - post_event(boost::intrusive_ptr(new SchedReplica{})); + post_event(SchedReplica{}); } sc::result ActiveReplica::react(const SchedReplica&) -- 2.39.5