From aa81f93537fd8bfdf1da6e9200e7f76bea3af71f Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 27 Jun 2018 08:34:07 -0500 Subject: [PATCH] msg/Messenger: use mutable_item_history<> for my_addrs We read this without a lock. Signed-off-by: Sage Weil --- src/msg/Messenger.h | 7 ++++--- src/msg/async/AsyncMessenger.cc | 20 ++++++++++---------- src/msg/async/AsyncMessenger.h | 2 +- src/msg/simple/Accepter.cc | 4 ++-- src/msg/simple/SimpleMessenger.cc | 14 +++++++------- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/msg/Messenger.h b/src/msg/Messenger.h index 9f8937e6b3256..ebc64c34f6180 100644 --- a/src/msg/Messenger.h +++ b/src/msg/Messenger.h @@ -29,6 +29,7 @@ #include "include/types.h" #include "include/ceph_features.h" #include "auth/Crypto.h" +#include "common/item_history.h" #include #include @@ -53,7 +54,7 @@ protected: entity_name_t my_name; /// my addr - entity_addrvec_t my_addrs; + mutable_item_history my_addrs; int default_send_priority; /// set to true once the Messenger has started, and set to false on shutdown @@ -151,10 +152,10 @@ public: * currently believes to be its own. */ entity_addr_t get_myaddr() { - return my_addrs.front(); + return my_addrs->front(); } const entity_addrvec_t& get_myaddrs() { - return my_addrs; + return *my_addrs; } /** diff --git a/src/msg/async/AsyncMessenger.cc b/src/msg/async/AsyncMessenger.cc index f69579cada04a..d4a74963e9845 100644 --- a/src/msg/async/AsyncMessenger.cc +++ b/src/msg/async/AsyncMessenger.cc @@ -446,7 +446,7 @@ int AsyncMessenger::client_bind(const entity_addr_t &bind_addr) return 0; Mutex::Locker l(lock); if (did_bind) { - assert(my_addrs.legacy_addr() == bind_addr); + assert(my_addrs->legacy_addr() == bind_addr); return 0; } if (started) { @@ -472,7 +472,7 @@ void AsyncMessenger::_finish_bind(const entity_addrvec_t& bind_addrs, if (get_myaddrs().front().get_port() == 0) { set_myaddrs(listen_addrs); } - for (auto& a : my_addrs.v) { + for (auto& a : my_addrs->v) { a.set_nonce(nonce); } @@ -495,7 +495,7 @@ int AsyncMessenger::start() stopped = false; if (!did_bind) { - for (auto& a : my_addrs.v) { + for (auto& a : my_addrs->v) { a.nonce = nonce; } _init_local_connection(); @@ -548,7 +548,7 @@ AsyncConnectionRef AsyncMessenger::create_connect( const entity_addrvec_t& addrs, int type) { assert(lock.is_locked()); - assert(addrs != my_addrs); + assert(addrs != *my_addrs); ldout(cct, 10) << __func__ << " " << addrs << ", creating connection and registering" << dendl; @@ -580,7 +580,7 @@ AsyncConnectionRef AsyncMessenger::create_connect( ConnectionRef AsyncMessenger::get_connection(const entity_inst_t& dest) { Mutex::Locker l(lock); - if (my_addrs.legacy_addr() == dest.addr) { + if (my_addrs->legacy_addr() == dest.addr) { // local return local_connection; } @@ -649,7 +649,7 @@ void AsyncMessenger::submit_message(Message *m, AsyncConnectionRef con, } // local? - if (my_addrs.legacy_addr() == dest_addr) { + if (my_addrs->legacy_addr() == dest_addr) { // local local_connection->send_message(m); return ; @@ -678,7 +678,7 @@ bool AsyncMessenger::set_addr_unknowns(const entity_addrvec_t &addrs) bool ret = false; Mutex::Locker l(lock); - for (auto& a : my_addrs.v) { + for (auto& a : my_addrs->v) { if (a.is_blank_ip()) { int port = a.get_port(); for (auto& b : addrs.v) { @@ -789,14 +789,14 @@ void AsyncMessenger::learned_addr(const entity_addr_t &peer_addr_for_me) lock.Lock(); if (need_addr) { need_addr = false; - if (my_addrs.empty()) { + if (my_addrs->empty()) { auto a = peer_addr_for_me; a.set_nonce(nonce); set_myaddrs(entity_addrvec_t(a)); ldout(cct,10) << __func__ << " had no addrs" << dendl; } else { // fix all addrs of the same family, regardless of type (msgr2 vs legacy) - for (auto& a : my_addrs.v) { + for (auto& a : my_addrs->v) { if (a.get_family() == peer_addr_for_me.get_family()) { entity_addr_t t = peer_addr_for_me; t.set_type(a.get_type()); @@ -807,7 +807,7 @@ void AsyncMessenger::learned_addr(const entity_addr_t &peer_addr_for_me) } } } - ldout(cct, 1) << __func__ << " learned my addr " << my_addrs + ldout(cct, 1) << __func__ << " learned my addr " << *my_addrs << " (peer_addr_for_me " << peer_addr_for_me << ")" << dendl; _init_local_connection(); } diff --git a/src/msg/async/AsyncMessenger.h b/src/msg/async/AsyncMessenger.h index df748fbde713a..bce7bc83517ea 100644 --- a/src/msg/async/AsyncMessenger.h +++ b/src/msg/async/AsyncMessenger.h @@ -324,7 +324,7 @@ private: void _init_local_connection() { assert(lock.is_locked()); - local_connection->peer_addrs = my_addrs; + local_connection->peer_addrs = *my_addrs; local_connection->peer_type = my_name.type(); local_connection->set_features(CEPH_FEATURES_ALL); ms_deliver_handle_fast_connect(local_connection.get()); diff --git a/src/msg/simple/Accepter.cc b/src/msg/simple/Accepter.cc index 4216e59c1b0d5..829535e3eca00 100644 --- a/src/msg/simple/Accepter.cc +++ b/src/msg/simple/Accepter.cc @@ -255,7 +255,7 @@ int Accepter::bind(const entity_addr_t &bind_addr, const set& avoid_ports) return rc; } - ldout(msgr->cct,1) << __func__ << " my_addrs " << msgr->my_addrs + ldout(msgr->cct,1) << __func__ << " my_addrs " << *msgr->my_addrs << " my_addr " << msgr->my_addr << " need_addr=" << msgr->get_need_addr() << dendl; return 0; @@ -273,7 +273,7 @@ int Accepter::rebind(const set& avoid_ports) // adjust the nonce; we want our entity_addr_t to be truly unique. nonce += 1000000; msgr->my_addr.nonce = nonce; - msgr->my_addrs.v[0].nonce = nonce; + msgr->my_addrs->v[0].nonce = nonce; ldout(msgr->cct,10) << __func__ << " new nonce " << nonce << " and addr " << msgr->my_addr << dendl; diff --git a/src/msg/simple/SimpleMessenger.cc b/src/msg/simple/SimpleMessenger.cc index 426a770ab276f..04a1c7812912c 100644 --- a/src/msg/simple/SimpleMessenger.cc +++ b/src/msg/simple/SimpleMessenger.cc @@ -151,7 +151,7 @@ bool SimpleMessenger::set_addr_unknowns(const entity_addrvec_t &addrs) { bool ret = false; auto addr = addrs.legacy_addr(); - assert(my_addr == my_addrs.front()); + assert(my_addr == my_addrs->front()); if (my_addr.is_blank_ip()) { ldout(cct,1) << __func__ << " " << addr << dendl; entity_addr_t t = my_addr; @@ -164,7 +164,7 @@ bool SimpleMessenger::set_addr_unknowns(const entity_addrvec_t &addrs) } else { ldout(cct,1) << __func__ << " " << addr << " no-op" << dendl; } - assert(my_addr == my_addrs.front()); + assert(my_addr == my_addrs->front()); return ret; } @@ -174,11 +174,11 @@ void SimpleMessenger::set_myaddrs(const entity_addrvec_t &av) my_addr.set_nonce(nonce); // do this in a slightly paranoid way because we update this value in a // thread-unsafe way. SimpleMessenger sucks. - if (my_addrs.empty()) { + if (my_addrs->empty()) { Messenger::set_myaddrs(av); } else { - assert(my_addrs.v.size() == av.v.size()); - my_addrs.v[0] = av.front(); + assert(my_addrs->v.size() == av.v.size()); + my_addrs->v[0] = av.front(); set_endpoint_addr(av.front(), my_name); } } @@ -347,7 +347,7 @@ int SimpleMessenger::client_bind(const entity_addr_t &bind_addr) return 0; Mutex::Locker l(lock); if (did_bind) { - assert(my_addrs == entity_addrvec_t(bind_addr)); + assert(*my_addrs == entity_addrvec_t(bind_addr)); return 0; } if (started) { @@ -776,7 +776,7 @@ void SimpleMessenger::learned_addr(const entity_addr_t &peer_addr_for_me) void SimpleMessenger::init_local_connection() { - local_connection->peer_addrs = my_addrs; + local_connection->peer_addrs = *my_addrs; local_connection->peer_type = my_name.type(); local_connection->set_features(CEPH_FEATURES_ALL); ms_deliver_handle_fast_connect(local_connection.get()); -- 2.39.5