osd/scrub: add to PgScrubber a local ref to next-scrub's details

The owner is still the PG.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
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 <rfriedma@redhat.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 39447c07cda5dd9b2a523d8d3e8805f96166d2f3..35a693711d64efb52036d692d9d161c8b4b6f886 100644 (file)
--- 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 2bd4a5af067b48f47cf3d36200795e8f01ebd389..4cd845d43161ef64eb04e7b5baad168502e03554 100644 (file)
--- 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 34d0763e7314a0293104116b2fd8b0be2cd80dcc..14f6a67e5201569958c2f4bccd50d13ed890704f 100644 (file)
--- 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<ScrubMachine>(m_pg, this);

diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h
index 86d774fcd5a431649a5a7da086b0e208db5b643f..6fffed57bd3b45a708e76dcca026449d54dc0408 100644 (file)
--- 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 326dd5bab388e802e41349da89ad1757250577f9..af7febf0eec5ba2db032d6029d467dbaf754a59f 100644 (file)
--- 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 30a685dfbcaffae6072b028a52ad9e2c05dfcb74..573465d5c5037119bbfe346ff0f8b0283778a443 100644 (file)
--- 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