From: Ronen Friedman Date: Thu, 30 Dec 2021 13:01:24 +0000 (+0000) Subject: osd/scrub: add to PgScrubber a local ref to next-scrub's details X-Git-Tag: v18.0.0~1513^2~2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=2a3483f7d3d25752b381a4c1214a0223120abcd1;p=ceph.git osd/scrub: add to PgScrubber a local ref to next-scrub's details The owner is still the PG. Signed-off-by: Ronen Friedman osd/scrub: removing PgScrubber's ref to the primary scrub map as caching this reference (to an object that is owned by the scrubber backend) creates a dangling-pointer risk. Signed-off-by: Ronen Friedman --- diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 39447c07cda5d..35a693711d64e 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -7548,7 +7548,7 @@ Scrub::schedule_result_t OSDService::initiate_a_scrub(spg_t pgid, return Scrub::schedule_result_t::already_started; } // Skip other kinds of scrubbing if only explicitly requested repairing is allowed - if (allow_requested_repair_only && !pg->m_planned_scrub.must_repair) { + if (allow_requested_repair_only && !pg->get_planned_scrub().must_repair) { pg->unlock(); dout(10) << __func__ << " skip " << pgid << " because repairing is not explicitly requested on it" << dendl; @@ -7573,7 +7573,7 @@ void OSD::resched_all_scrubs() if (!pg) continue; - if (!pg->m_planned_scrub.must_scrub && !pg->m_planned_scrub.need_auto) { + if (!pg->get_planned_scrub().must_scrub && !pg->get_planned_scrub().need_auto) { dout(15) << __func__ << ": reschedule " << job.pgid << dendl; pg->reschedule_scrub(); } diff --git a/src/osd/PG.h b/src/osd/PG.h index 2bd4a5af067b4..4cd845d43161e 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -168,6 +168,7 @@ class ScrubberPasskey { private: friend class Scrub::ReplicaReservations; friend class PrimaryLogScrub; + friend class PgScrubber; ScrubberPasskey() {} ScrubberPasskey(const ScrubberPasskey&) = default; ScrubberPasskey& operator=(const ScrubberPasskey&) = delete; @@ -190,6 +191,10 @@ public: /// flags detailing scheduling/operation characteristics of the next scrub requested_scrub_t m_planned_scrub; + const requested_scrub_t& get_planned_scrub() const { + return m_planned_scrub; + } + /// scrubbing state for both Primary & replicas bool is_scrub_active() const { return m_scrubber->is_scrub_active(); } @@ -1374,6 +1379,10 @@ public: return osd; } + requested_scrub_t& get_planned_scrub(ScrubberPasskey) { + return m_planned_scrub; + } + }; #endif diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index 34d0763e7314a..14f6a67e52015 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -17,6 +17,7 @@ #include "messages/MOSDScrubReserve.h" #include "osd/OSD.h" +#include "osd/PG.h" #include "osd/osd_types_fmt.h" #include "ScrubStore.h" #include "scrub_backend.h" @@ -631,7 +632,7 @@ void PgScrubber::on_applied_when_primary(const eversion_t& applied_version) */ bool PgScrubber::select_range() { - m_primary_scrubmap = m_be->new_chunk(); + m_be->new_chunk(); /* get the start and end of our scrub chunk * @@ -1000,7 +1001,7 @@ int PgScrubber::build_primary_map_chunk() dout(20) << __func__ << ": initiated at epoch " << map_building_since << dendl; - auto ret = build_scrub_map_chunk(*m_primary_scrubmap, + auto ret = build_scrub_map_chunk(m_be->get_primary_scrubmap(), m_primary_scrubmap_pos, m_start, m_end, @@ -1521,7 +1522,7 @@ void PgScrubber::scrub_finish() ceph_assert(m_pg->is_locked()); ceph_assert(is_queued_or_active()); - m_pg->m_planned_scrub = requested_scrub_t{}; + m_planned_scrub = requested_scrub_t{}; // if the repair request comes from auto-repair and large number of errors, // we would like to cancel auto-repair @@ -1590,11 +1591,11 @@ void PgScrubber::scrub_finish() // Deep scrub in order to get corrected error counts m_pg->scrub_after_recovery = true; - m_pg->m_planned_scrub.req_scrub = - m_pg->m_planned_scrub.req_scrub || m_flags.required; + m_planned_scrub.req_scrub = + m_planned_scrub.req_scrub || m_flags.required; dout(20) << __func__ << " Current 'required': " << m_flags.required - << " Planned 'req_scrub': " << m_pg->m_planned_scrub.req_scrub + << " Planned 'req_scrub': " << m_planned_scrub.req_scrub << dendl; } else if (m_shallow_errors || m_deep_errors) { @@ -1683,7 +1684,7 @@ void PgScrubber::scrub_finish() cleanup_on_finish(); if (do_auto_scrub) { - request_rescrubbing(m_pg->m_planned_scrub); + request_rescrubbing(m_planned_scrub); } if (m_pg->is_active() && m_pg->is_primary()) { @@ -1729,7 +1730,7 @@ void PgScrubber::dump_scrubber(ceph::Formatter* f, } else { f->dump_bool("active", false); f->dump_bool("must_scrub", - (m_pg->m_planned_scrub.must_scrub || m_flags.required)); + (m_planned_scrub.must_scrub || m_flags.required)); f->dump_bool("must_deep_scrub", request_flags.must_deep_scrub); f->dump_bool("must_repair", request_flags.must_repair); f->dump_bool("need_auto", request_flags.need_auto); @@ -1818,13 +1819,13 @@ pg_scrubbing_status_t PgScrubber::get_schedule() const // Will next scrub surely be a deep one? note that deep-scrub might be // selected even if we report a regular scrub here. bool deep_expected = (now_is >= m_pg->next_deepscrub_interval()) || - m_pg->m_planned_scrub.must_deep_scrub || - m_pg->m_planned_scrub.need_auto; + m_planned_scrub.must_deep_scrub || + m_planned_scrub.need_auto; scrub_level_t expected_level = deep_expected ? scrub_level_t::deep : scrub_level_t::shallow; - bool periodic = !m_pg->m_planned_scrub.must_scrub && - !m_pg->m_planned_scrub.need_auto && - !m_pg->m_planned_scrub.must_deep_scrub; + bool periodic = !m_planned_scrub.must_scrub && + !m_planned_scrub.need_auto && + !m_planned_scrub.must_deep_scrub; // are we ripe for scrubbing? if (now_is > m_scrub_job->schedule.scheduled_at) { @@ -1884,6 +1885,7 @@ PgScrubber::PgScrubber(PG* pg) , m_pg_id{pg->pg_id} , m_osds{m_pg->osd} , m_pg_whoami{pg->pg_whoami} + , m_planned_scrub{pg->get_planned_scrub(ScrubberPasskey{})} , preemption_data{pg} { m_fsm = std::make_unique(m_pg, this); diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h index 86d774fcd5a43..6fffed57bd3b4 100644 --- a/src/osd/scrubber/pg_scrubber.h +++ b/src/osd/scrubber/pg_scrubber.h @@ -690,6 +690,9 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener { scrub_flags_t m_flags; + /// a reference to the details of the next scrub (as requested and managed by the PG) + requested_scrub_t& m_planned_scrub; + bool m_active{false}; /** @@ -800,7 +803,6 @@ private: void message_all_replicas(int32_t opcode, std::string_view op_text); hobject_t m_max_end; ///< Largest end that may have been sent to replicas - ScrubMap* m_primary_scrubmap{nullptr}; ///< the map is owned by the ScrubBackend ScrubMapBuilder m_primary_scrubmap_pos; void _request_scrub_map(pg_shard_t replica, diff --git a/src/osd/scrubber/scrub_backend.cc b/src/osd/scrubber/scrub_backend.cc index 326dd5bab388e..af7febf0eec5b 100644 --- a/src/osd/scrubber/scrub_backend.cc +++ b/src/osd/scrubber/scrub_backend.cc @@ -108,11 +108,15 @@ void ScrubBackend::update_repair_status(bool should_repair) : (m_depth == scrub_level_t::deep ? "deep-scrub"sv : "scrub"sv)); } -ScrubMap* ScrubBackend::new_chunk() +void ScrubBackend::new_chunk() { dout(15) << __func__ << dendl; this_chunk.emplace(m_pg_whoami); - return &this_chunk->received_maps[m_pg_whoami]; +} + +ScrubMap& ScrubBackend::get_primary_scrubmap() +{ + return this_chunk->received_maps[m_pg_whoami]; } void ScrubBackend::merge_to_authoritative_set() diff --git a/src/osd/scrubber/scrub_backend.h b/src/osd/scrubber/scrub_backend.h index 30a685dfbcaff..573465d5c5037 100644 --- a/src/osd/scrubber/scrub_backend.h +++ b/src/osd/scrubber/scrub_backend.h @@ -219,7 +219,9 @@ class ScrubBackend { * * @returns a pointer to the newly created ScrubMap. */ - ScrubMap* new_chunk(); + void new_chunk(); + + ScrubMap& get_primary_scrubmap(); /** * sets Backend's m_repair flag (setting m_mode_desc to a corresponding