From df945a900a90701fd024ff7b537be509937213b2 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 13 Aug 2014 08:30:25 -0700 Subject: [PATCH] osd: fix require_same_peer_instance from fast_dispatch The mark-down of old peers needs to take the session_dispatch_lock in order to safely clear the Session ref cycle. However, for fast dispatch callers, that lock is already held. Pass a flag down from the callers indicating whether we need to take the additional lock. Fixes: #9096 Signed-off-by: Sage Weil --- src/osd/OSD.cc | 47 +++++++++++++++++++++++++++++------------------ src/osd/OSD.h | 6 ++++-- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 5cfbf97cd5fd1..0a9c72bf247f0 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -6694,7 +6694,8 @@ bool OSD::require_self_aliveness(OpRequestRef& op, epoch_t epoch) return true; } -bool OSD::require_same_peer_instance(OpRequestRef& op, OSDMapRef& map) +bool OSD::require_same_peer_instance(OpRequestRef& op, OSDMapRef& map, + bool is_fast_dispatch) { Message *m = op->get_req(); int from = m->get_source().num(); @@ -6710,10 +6711,12 @@ bool OSD::require_same_peer_instance(OpRequestRef& op, OSDMapRef& map) cluster_messenger->mark_down(con.get()); Session *s = static_cast(con->get_priv()); if (s) { - s->session_dispatch_lock.Lock(); + if (!is_fast_dispatch) + s->session_dispatch_lock.Lock(); clear_session_waiting_on_map(s); con->set_priv(NULL); // break ref <-> session cycle, if any - s->session_dispatch_lock.Unlock(); + if (!is_fast_dispatch) + s->session_dispatch_lock.Unlock(); s->put(); } return false; @@ -6726,7 +6729,8 @@ bool OSD::require_same_peer_instance(OpRequestRef& op, OSDMapRef& map) * require that we have same (or newer) map, and that * the source is the pg primary. */ -bool OSD::require_same_or_newer_map(OpRequestRef& op, epoch_t epoch) +bool OSD::require_same_or_newer_map(OpRequestRef& op, epoch_t epoch, + bool is_fast_dispatch) { Message *m = op->get_req(); dout(15) << "require_same_or_newer_map " << epoch << " (i am " << osdmap->get_epoch() << ") " << m << dendl; @@ -6746,7 +6750,7 @@ bool OSD::require_same_or_newer_map(OpRequestRef& op, epoch_t epoch) // ok, our map is same or newer.. do they still exist? if (m->get_connection()->get_messenger() == cluster_messenger && - !require_same_peer_instance(op, osdmap)) { + !require_same_peer_instance(op, osdmap, is_fast_dispatch)) { return false; } @@ -6863,7 +6867,8 @@ void OSD::handle_pg_create(OpRequestRef op) } op->get_req()->put(); - if (!require_same_or_newer_map(op, m->epoch)) return; + if (!require_same_or_newer_map(op, m->epoch, false)) + return; op->mark_started(); @@ -7220,7 +7225,8 @@ void OSD::handle_pg_notify(OpRequestRef op) if (!require_osd_peer(op)) return; - if (!require_same_or_newer_map(op, m->get_epoch())) return; + if (!require_same_or_newer_map(op, m->get_epoch(), false)) + return; op->mark_started(); @@ -7255,7 +7261,8 @@ void OSD::handle_pg_log(OpRequestRef op) return; int from = m->get_source().num(); - if (!require_same_or_newer_map(op, m->get_epoch())) return; + if (!require_same_or_newer_map(op, m->get_epoch(), false)) + return; if (m->info.pgid.preferred() >= 0) { dout(10) << "ignoring localized pg " << m->info.pgid << dendl; @@ -7284,7 +7291,8 @@ void OSD::handle_pg_info(OpRequestRef op) return; int from = m->get_source().num(); - if (!require_same_or_newer_map(op, m->get_epoch())) return; + if (!require_same_or_newer_map(op, m->get_epoch(), false)) + return; op->mark_started(); @@ -7321,7 +7329,8 @@ void OSD::handle_pg_trim(OpRequestRef op) return; int from = m->get_source().num(); - if (!require_same_or_newer_map(op, m->epoch)) return; + if (!require_same_or_newer_map(op, m->epoch, false)) + return; if (m->pgid.preferred() >= 0) { dout(10) << "ignoring localized pg " << m->pgid << dendl; @@ -7374,7 +7383,7 @@ void OSD::handle_pg_backfill_reserve(OpRequestRef op) if (!require_osd_peer(op)) return; - if (!require_same_or_newer_map(op, m->query_epoch)) + if (!require_same_or_newer_map(op, m->query_epoch, false)) return; PG::CephPeeringEvtRef evt; @@ -7423,7 +7432,7 @@ void OSD::handle_pg_recovery_reserve(OpRequestRef op) if (!require_osd_peer(op)) return; - if (!require_same_or_newer_map(op, m->query_epoch)) + if (!require_same_or_newer_map(op, m->query_epoch, false)) return; PG::CephPeeringEvtRef evt; @@ -7483,7 +7492,8 @@ void OSD::handle_pg_query(OpRequestRef op) dout(7) << "handle_pg_query from " << m->get_source() << " epoch " << m->get_epoch() << dendl; int from = m->get_source().num(); - if (!require_same_or_newer_map(op, m->get_epoch())) return; + if (!require_same_or_newer_map(op, m->get_epoch(), false)) + return; op->mark_started(); @@ -7590,7 +7600,8 @@ void OSD::handle_pg_remove(OpRequestRef op) dout(7) << "handle_pg_remove from " << m->get_source() << " on " << m->pg_list.size() << " pgs" << dendl; - if (!require_same_or_newer_map(op, m->get_epoch())) return; + if (!require_same_or_newer_map(op, m->get_epoch(), false)) + return; op->mark_started(); @@ -8060,10 +8071,10 @@ void OSD::handle_replica_op(OpRequestRef& op, OSDMapRef& osdmap) if (!require_self_aliveness(op, m->map_epoch)) return; if (!require_osd_peer(op)) - return false; - if (map->get_epoch() >= m->map_epoch && - !require_same_peer_instance(op, osdmap)) - return false; + return; + if (osdmap->get_epoch() >= m->map_epoch && + !require_same_peer_instance(op, osdmap, true)) + return; // must be a rep op. assert(m->get_source().is_osd()); diff --git a/src/osd/OSD.h b/src/osd/OSD.h index 952d1b8002925..1c91189d48432 100644 --- a/src/osd/OSD.h +++ b/src/osd/OSD.h @@ -1820,9 +1820,11 @@ protected: * address as in the given map. * @pre op was sent by an OSD using the cluster messenger */ - bool require_same_peer_instance(OpRequestRef& op, OSDMapRef& map); + bool require_same_peer_instance(OpRequestRef& op, OSDMapRef& map, + bool is_fast_dispatch); - bool require_same_or_newer_map(OpRequestRef& op, epoch_t e); + bool require_same_or_newer_map(OpRequestRef& op, epoch_t e, + bool is_fast_dispatch); void handle_pg_query(OpRequestRef op); void handle_pg_notify(OpRequestRef op); -- 2.39.5