From 98d50889f65bdfcce793e2d3acf3352fe8b5c90f Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Fri, 21 Jul 2023 13:05:15 -0400 Subject: [PATCH] mds: drop MDRequestImpl::msg_lock TrackedOp::lock is already suitable for this purpose. Signed-off-by: Patrick Donnelly --- src/mds/Mutation.cc | 107 +++++++++++++++++++------------------------- src/mds/Mutation.h | 2 - 2 files changed, 45 insertions(+), 64 deletions(-) diff --git a/src/mds/Mutation.cc b/src/mds/Mutation.cc index b009bb38ffc8a..e51348e638d5f 100644 --- a/src/mds/Mutation.cc +++ b/src/mds/Mutation.cc @@ -446,22 +446,17 @@ int MDRequestImpl::compare_paths() cref_t MDRequestImpl::release_client_request() { - msg_lock.lock(); + std::lock_guard l(lock); cref_t req; req.swap(client_request); client_request = req; - msg_lock.unlock(); return req; } void MDRequestImpl::reset_peer_request(const cref_t& req) { - msg_lock.lock(); - cref_t old; - old.swap(peer_request); + std::lock_guard l(lock); peer_request = req; - msg_lock.unlock(); - old.reset(); } void MDRequestImpl::print(ostream &out) const @@ -484,50 +479,43 @@ void MDRequestImpl::_dump(Formatter *f) const std::lock_guard l(lock); f->dump_string("flag_point", _get_state_string()); f->dump_stream("reqid") << reqid; - { - msg_lock.lock(); - auto _client_request = client_request; - auto _peer_request =peer_request; - msg_lock.unlock(); - - if (_client_request) { - f->dump_string("op_type", "client_request"); - f->open_object_section("client_info"); - f->dump_stream("client") << _client_request->get_orig_source(); - f->dump_int("tid", _client_request->get_tid()); - f->close_section(); // client_info - } else if (is_peer()) { // replies go to an existing mdr - f->dump_string("op_type", "peer_request"); - f->open_object_section("leader_info"); - f->dump_stream("leader") << peer_to_mds; - f->close_section(); // leader_info - - if (_peer_request) { - f->open_object_section("request_info"); - f->dump_int("attempt", _peer_request->get_attempt()); - f->dump_string("op_type", - MMDSPeerRequest::get_opname(_peer_request->get_op())); - f->dump_int("lock_type", _peer_request->get_lock_type()); - f->dump_stream("object_info") << _peer_request->get_object_info(); - f->dump_stream("srcdnpath") << _peer_request->srcdnpath; - f->dump_stream("destdnpath") << _peer_request->destdnpath; - f->dump_stream("witnesses") << _peer_request->witnesses; - f->dump_bool("has_inode_export", - _peer_request->inode_export_v != 0); - f->dump_int("inode_export_v", _peer_request->inode_export_v); - f->dump_stream("op_stamp") << _peer_request->op_stamp; - f->close_section(); // request_info - } - } - else if (internal_op != -1) { // internal request - f->dump_string("op_type", "internal_op"); - f->dump_int("internal_op", internal_op); - f->dump_string("op_name", ceph_mds_op_name(internal_op)); - } - else { - f->dump_string("op_type", "no_available_op_found"); + if (client_request) { + f->dump_string("op_type", "client_request"); + f->open_object_section("client_info"); + f->dump_stream("client") << client_request->get_orig_source(); + f->dump_int("tid", client_request->get_tid()); + f->close_section(); // client_info + } else if (is_peer()) { // replies go to an existing mdr + f->dump_string("op_type", "peer_request"); + f->open_object_section("leader_info"); + f->dump_stream("leader") << peer_to_mds; + f->close_section(); // leader_info + + if (peer_request) { + f->open_object_section("request_info"); + f->dump_int("attempt", peer_request->get_attempt()); + f->dump_string("op_type", + MMDSPeerRequest::get_opname(peer_request->get_op())); + f->dump_int("lock_type", peer_request->get_lock_type()); + f->dump_stream("object_info") << peer_request->get_object_info(); + f->dump_stream("srcdnpath") << peer_request->srcdnpath; + f->dump_stream("destdnpath") << peer_request->destdnpath; + f->dump_stream("witnesses") << peer_request->witnesses; + f->dump_bool("has_inode_export", + peer_request->inode_export_v != 0); + f->dump_int("inode_export_v", peer_request->inode_export_v); + f->dump_stream("op_stamp") << peer_request->op_stamp; + f->close_section(); // request_info } } + else if (internal_op != -1) { // internal request + f->dump_string("op_type", "internal_op"); + f->dump_int("internal_op", internal_op); + f->dump_string("op_name", ceph_mds_op_name(internal_op)); + } + else { + f->dump_string("op_type", "no_available_op_found"); + } { f->open_array_section("events"); @@ -538,25 +526,20 @@ void MDRequestImpl::_dump(Formatter *f) const } } -void MDRequestImpl::_dump_op_descriptor(ostream& stream) const +void MDRequestImpl::_dump_op_descriptor(ostream& os) const { - msg_lock.lock(); - auto _client_request = client_request; - auto _peer_request = peer_request; - msg_lock.unlock(); - - if (_client_request) { - _client_request->print(stream); - } else if (_peer_request) { - _peer_request->print(stream); + if (client_request) { + client_request->print(os); + } else if (peer_request) { + peer_request->print(os); } else if (is_peer()) { - stream << "peer_request:" << reqid; + os << "peer_request:" << reqid; } else if (internal_op >= 0) { - stream << "internal op " << ceph_mds_op_name(internal_op) << ":" << reqid; + os << "internal op " << ceph_mds_op_name(internal_op) << ":" << reqid; } else { // drat, it's triggered by a peer request, but we don't have a message // FIXME - stream << "rejoin:" << reqid; + os << "rejoin:" << reqid; } } diff --git a/src/mds/Mutation.h b/src/mds/Mutation.h index 3bfff9691205b..c2afb073bb5bd 100644 --- a/src/mds/Mutation.h +++ b/src/mds/Mutation.h @@ -455,8 +455,6 @@ struct MDRequestImpl : public MutationImpl { protected: void _dump(ceph::Formatter *f) const override; void _dump_op_descriptor(std::ostream& stream) const override; -private: - mutable ceph::spinlock msg_lock; }; struct MDPeerUpdate { -- 2.39.5