From a2b294133283c3ba26dd118c1cdc3a5961e678fc Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Mon, 3 Jun 2019 21:15:42 +0800 Subject: [PATCH] test/crimson: fix msgr test of ref counter racing Foreign-dispatch in crimson-messenger is not implemented yet, and no need to test this feature now. Fixes: http://tracker.ceph.com/issues/36405 Signed-off-by: Yingxin Cheng --- src/crimson/net/SocketConnection.cc | 2 ++ src/test/crimson/test_messenger.cc | 29 +++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/crimson/net/SocketConnection.cc b/src/crimson/net/SocketConnection.cc index a8f5cac1cb3e4..288f4b576c769 100644 --- a/src/crimson/net/SocketConnection.cc +++ b/src/crimson/net/SocketConnection.cc @@ -57,6 +57,8 @@ seastar::future SocketConnection::is_connected() seastar::future<> SocketConnection::send(MessageRef msg) { logger().debug("{} --> {} === {}", messenger, get_peer_addr(), *msg); + // Cannot send msg from another core now, its ref counter can be contaminated! + ceph_assert(seastar::engine().cpu_id() == shard_id()); return seastar::smp::submit_to(shard_id(), [this, msg=std::move(msg)] { return protocol->send(std::move(msg)); }); diff --git a/src/test/crimson/test_messenger.cc b/src/test/crimson/test_messenger.cc index d1552159e4c66..3e22643b03299 100644 --- a/src/test/crimson/test_messenger.cc +++ b/src/test/crimson/test_messenger.cc @@ -167,7 +167,17 @@ static seastar::future<> test_echo(unsigned rounds, return msgr->shutdown(); } - seastar::future<> dispatch_pingpong(const entity_addr_t& peer_addr, bool foreign_dispatch=true) { + // Note: currently we don't support foreign dispatch a message because: + // 1. it is not effecient because each ref-count modification needs + // a cross-core jump, so it should be discouraged. + // 2. messenger needs to be modified to hold a wrapper for the sending + // message because it can be a nested seastar smart ptr or not. + // 3. in 1:1 mapping OSD, there is no need to do foreign dispatch. + seastar::future<> dispatch_pingpong(const entity_addr_t& peer_addr, + bool foreign_dispatch) { +#ifndef CRIMSON_MSGR_SEND_FOREIGN + ceph_assert(!foreign_dispatch); +#endif mono_time start_time = mono_clock::now(); return msgr->connect(peer_addr, entity_name_t::TYPE_OSD) .then([this, foreign_dispatch, start_time](auto conn) { @@ -269,12 +279,15 @@ static seastar::future<> test_echo(unsigned rounds, // dispatch pingpoing .then([client1, client2, server1, server2] { return seastar::when_all_succeed( - // test connecting in parallel, accepting in parallel, - // and operating the connection reference from a foreign/local core - client1->dispatch_pingpong(server1->msgr->get_myaddr(), true), + // test connecting in parallel, accepting in parallel +#ifdef CRIMSON_MSGR_SEND_FOREIGN + // operate the connection reference from a foreign core + client1->dispatch_pingpong(server1->msgr->get_myaddr(), true), + client2->dispatch_pingpong(server2->msgr->get_myaddr(), true), +#endif + // operate the connection reference from a local core client1->dispatch_pingpong(server2->msgr->get_myaddr(), false), - client2->dispatch_pingpong(server1->msgr->get_myaddr(), false), - client2->dispatch_pingpong(server2->msgr->get_myaddr(), true)); + client2->dispatch_pingpong(server1->msgr->get_myaddr(), false)); // shutdown }).finally([client1] { logger().info("client1 shutdown..."); @@ -331,7 +344,7 @@ static seastar::future<> test_concurrent_dispatch(bool v2) const std::string& lname, const uint64_t nonce, const entity_addr_t& addr) { - return ceph::net::Messenger::create(name, lname, nonce) + return ceph::net::Messenger::create(name, lname, nonce, 0) .then([this, addr](ceph::net::Messenger *messenger) { return container().invoke_on_all([messenger](auto& server) { server.msgr = messenger->get_local_shard(); @@ -363,7 +376,7 @@ static seastar::future<> test_concurrent_dispatch(bool v2) seastar::future<> init(const entity_name_t& name, const std::string& lname, const uint64_t nonce) { - return ceph::net::Messenger::create(name, lname, nonce) + return ceph::net::Messenger::create(name, lname, nonce, 0) .then([this](ceph::net::Messenger *messenger) { return container().invoke_on_all([messenger](auto& client) { client.msgr = messenger->get_local_shard(); -- 2.39.5