osd: fix pg locking flags

Two things we need to handle:

 - callers who already hold map_lock (split_pg())
 - callers who already hold another pg->lock, and want to skip the lockdep
   check for this one.

Signed-off-by: Sage Weil <sage@newdream.net>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 426be49a25f7b93228f9297e49a8dc1cc176b6db..5c83122016fc713577694d08f5898a8c00d37f4f 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -1098,7 +1098,7 @@ void OSD::_put_pool(PGPool *p)
   }
 }
 
-PG *OSD::_open_lock_pg(pg_t pgid, bool no_lockdep_check)
+PG *OSD::_open_lock_pg(pg_t pgid, bool no_lockdep_check, bool hold_map_lock)
 {
   assert(osd_lock.is_locked());
 
@@ -1118,19 +1118,22 @@ PG *OSD::_open_lock_pg(pg_t pgid, bool no_lockdep_check)
   assert(pg_map.count(pgid) == 0);
   pg_map[pgid] = pg;
 
-  pg->lock(no_lockdep_check); // always lock.
+  if (hold_map_lock)
+    pg->lock_with_map_lock_held(no_lockdep_check);
+  else
+    pg->lock(no_lockdep_check);
   pg->get();  // because it's in pg_map
   return pg;
 }
 
-PG *OSD::_create_lock_pg(pg_t pgid, bool newly_created,
+PG *OSD::_create_lock_pg(pg_t pgid, bool newly_created, bool hold_map_lock,
                         int role, vector<int>& up, vector<int>& acting, pg_history_t history,
                         ObjectStore::Transaction& t)
 {
   assert(osd_lock.is_locked());
   dout(20) << "_create_lock_pg pgid " << pgid << dendl;
 
-  PG *pg = _open_lock_pg(pgid, true);
+  PG *pg = _open_lock_pg(pgid, true, hold_map_lock);
 
   assert(!store->collection_exists(coll_t(pgid)));
   t.create_collection(coll_t(pgid));
@@ -1298,7 +1301,7 @@ PG *OSD::get_or_create_pg(const pg_info_t& info, epoch_t epoch, int from, int& c
     // ok, create PG locally using provided Info and History
     *pt = new ObjectStore::Transaction;
     *pfin = new C_Contexts(g_ceph_context);
-    pg = _create_lock_pg(info.pgid, create, role, up, acting, history, **pt);
+    pg = _create_lock_pg(info.pgid, create, false, role, up, acting, history, **pt);
       
     created++;
     dout(10) << *pg << " is new" << dendl;
@@ -4059,8 +4062,8 @@ void OSD::do_split(PG *parent, set<pg_t>& childpgids, ObjectStore::Transaction&
     history.epoch_created = history.same_up_since =
       history.same_interval_since = history.same_primary_since =
       osdmap->get_epoch();
-    PG *pg = _create_lock_pg(*q, true,
-                            0, parent->up, parent->acting, history, t);
+    PG *pg = _create_lock_pg(*q, true, true,
+                            parent->get_role(), parent->up, parent->acting, history, t);
     children[*q] = pg;
     dout(10) << "  child " << *pg << dendl;
   }
@@ -4297,7 +4300,7 @@ void OSD::handle_pg_create(OpRequest *op)
       ObjectStore::Transaction *t = new ObjectStore::Transaction;
       C_Contexts *fin = new C_Contexts(g_ceph_context);
 
-      PG *pg = _create_lock_pg(pgid, true,
+      PG *pg = _create_lock_pg(pgid, true, false,
                               0, creating_pgs[pgid].acting, creating_pgs[pgid].acting, history,
                               *t);
       creating_pgs.erase(pgid);

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index d053821b6d5299828ea3ee4dbe1bc019d1ff91df..e8c24ddfa38ab21ccc6414c9882f6329477d61f1 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -447,8 +447,8 @@ protected:
   bool  _have_pg(pg_t pgid);
   PG   *_lookup_lock_pg(pg_t pgid);
   PG   *_lookup_lock_pg_with_map_lock_held(pg_t pgid);
-  PG   *_open_lock_pg(pg_t pg, bool no_lockdep_check=false);  // create new PG (in memory)
-  PG   *_create_lock_pg(pg_t pgid, bool newly_created,
+  PG   *_open_lock_pg(pg_t pg, bool no_lockdep_check=false, bool hold_map_lock=false);
+  PG   *_create_lock_pg(pg_t pgid, bool newly_created, bool hold_map_lock,
                        int role, vector<int>& up, vector<int>& acting, pg_history_t history,
                        ObjectStore::Transaction& t);
 

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 42948f047189dcfc37a15c5a374b3f5aa56c0d99..2754816be589731e6dc9e821b630db99c702347c 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -54,9 +54,9 @@ void PG::lock(bool no_lockdep)
  * caller holds osd->map_lock, no need to take it to get a valid
  * osdmap reference.
  */
-void PG::lock_with_map_lock_held()
+void PG::lock_with_map_lock_held(bool no_lockdep)
 {
-  _lock.Lock();
+  _lock.Lock(no_lockdep);
   osdmap_ref = osd->osdmap;
 }
 

diff --git a/src/osd/PG.h b/src/osd/PG.h
index 5581d2ddf0c1f286592da8e000a3d95791e9b335..4c90b3daf8f96154aa4b21e5123dceccbcfe1e9e 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -356,7 +356,7 @@ public:
 
   /* During handle_osd_map, the osd holds a write lock to the osdmap.
    * *_with_map_lock_held assume that the map_lock is already held */
-  void lock_with_map_lock_held();
+  void lock_with_map_lock_held(bool no_lockdep = false);
 
   void assert_locked() {
     assert(_lock.is_locked());