osd: lookup_*pg must return PGRef

Otherwise it is fundamentally unsafe, as the PG might get destroyed out
from under us without a reference.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 3014753b5257b191fcace49725fe2a75de201448..85464d29a1622a16982845b3c20875d599d75585 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -3747,7 +3747,7 @@ PGRef OSD::_lookup_pg(spg_t pgid)
   return p->second->pg;
 }
 
-PG *OSD::_lookup_lock_pg(spg_t pgid)
+PGRef OSD::_lookup_lock_pg(spg_t pgid)
 {
   PGRef pg = _lookup_pg(pgid);
   if (!pg) {
@@ -3755,13 +3755,13 @@ PG *OSD::_lookup_lock_pg(spg_t pgid)
   }
   pg->lock();
   if (!pg->is_deleted()) {
-    return pg.get();
+    return pg;
   }
   pg->unlock();
   return nullptr;
 }
 
-PG *OSD::lookup_lock_pg(spg_t pgid)
+PGRef OSD::lookup_lock_pg(spg_t pgid)
 {
   return _lookup_lock_pg(pgid);
 }
@@ -4971,7 +4971,7 @@ void TestOpsSocketHook::test_ops(OSDService *service, ObjectStore *store,
       return;
     }
 
-    PG *pg = service->osd->_lookup_lock_pg(pgid);
+    PGRef pg = service->osd->_lookup_lock_pg(pgid);
     if (pg == nullptr) {
       ss << "Can't find pg " << pgid;
       return;
@@ -5850,7 +5850,7 @@ void OSD::do_command(Connection *con, ceph_tid_t tid, vector<string>& cmd, buffe
       r = -EINVAL;
     } else {
       spg_t pcand;
-      PG *pg = nullptr;
+      PGRef pg;
       if (osdmap->get_primary_shard(pgid, &pcand) &&
          (pg = _lookup_lock_pg(pcand))) {
        if (pg->is_primary()) {
@@ -6853,7 +6853,7 @@ void OSD::sched_scrub()
         continue;
       }
 
-      PG *pg = _lookup_lock_pg(scrub.pgid);
+      PGRef pg = _lookup_lock_pg(scrub.pgid);
       if (!pg)
        continue;
       dout(10) << "sched_scrub scrubbing " << scrub.pgid << " at " << scrub.sched_time

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index f870737916881872843ec7d71f2a05f1fc9027ba..aea1ec0e2941f7fb879c23d0586205f4a0bbebd2 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1800,7 +1800,7 @@ protected:
   PGRecoveryStats pg_recovery_stats;
 
   PGRef _lookup_pg(spg_t pgid);
-  PG   *_lookup_lock_pg(spg_t pgid);
+  PGRef _lookup_lock_pg(spg_t pgid);
   void register_pg(PGRef pg);
   void unregister_pg(PG *pg);
 
@@ -1808,7 +1808,7 @@ protected:
   void _get_pgids(vector<spg_t> *v);
 
 public:
-  PG   *lookup_lock_pg(spg_t pgid);
+  PGRef lookup_lock_pg(spg_t pgid);
 
 protected:
   PG* _make_pg(OSDMapRef createmap, spg_t pgid);

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 9934a5c9bdfc9510e2be24fd4bc624970dc0a074..518ac11701e1ee88b4db372c6975042b8ccc1436 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -4685,7 +4685,7 @@ void PG::scrub(epoch_t queued, ThreadPool::TPHandle &handle)
     int state = scrubber.state;
     auto scrub_requeue_callback =
         new FunctionContext([osds, pgid, state](int r) {
-          PG *pg = osds->osd->lookup_lock_pg(pgid);
+          PGRef pg = osds->osd->lookup_lock_pg(pgid);
           if (pg == nullptr) {
             lgeneric_dout(osds->osd->cct, 20)
                 << "scrub_requeue_callback: Could not find "