From 272afb9bbebfe5b4a46c56e27173680244cac73b Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Tue, 25 Sep 2018 15:42:25 +0800 Subject: [PATCH] auth,common: switch to ceph::mutex, etc in this change, along with LockPolicy, src/common/lock_* are completely removed. instead of using LockPolicy based template specialization of LockMutex, etc, it would be simpler if we can just rely on WITH_SEASTAR preprocessor macro to tell if we are compiling code for crimson or not. but please bear in mind, we cannot link against the plain libceph-common in crimson anymore. Signed-off-by: Kefu Chai --- src/auth/AuthClientHandler.cc | 22 ++------- src/auth/AuthClientHandler.h | 7 +-- src/auth/RotatingKeyRing.cc | 28 +++--------- src/auth/RotatingKeyRing.h | 8 ++-- src/auth/cephx/CephxClientHandler.cc | 31 ++++--------- src/auth/cephx/CephxClientHandler.h | 5 +-- src/common/lock_cond.h | 42 ----------------- src/common/lock_mutex.h | 61 ------------------------- src/common/lock_policy.h | 12 ----- src/common/lock_shared_mutex.h | 67 ---------------------------- src/common/lock_shared_ptr.h | 23 ---------- src/common/shared_cache.hpp | 29 +++++++----- src/mon/MonClient.cc | 6 +-- src/mon/MonClient.h | 9 ++-- 14 files changed, 50 insertions(+), 300 deletions(-) delete mode 100644 src/common/lock_cond.h delete mode 100644 src/common/lock_mutex.h delete mode 100644 src/common/lock_policy.h delete mode 100644 src/common/lock_shared_mutex.h delete mode 100644 src/common/lock_shared_ptr.h diff --git a/src/auth/AuthClientHandler.cc b/src/auth/AuthClientHandler.cc index fe0a5238f5495..7f6804b47ba32 100644 --- a/src/auth/AuthClientHandler.cc +++ b/src/auth/AuthClientHandler.cc @@ -20,32 +20,16 @@ #include "none/AuthNoneClientHandler.h" -template AuthClientHandler* -AuthClientHandler::create(CephContext *cct, int proto, - RotatingKeyRing *rkeys) +AuthClientHandler::create(CephContext* cct, int proto, + RotatingKeyRing* rkeys) { switch (proto) { case CEPH_AUTH_CEPHX: - return new CephxClientHandler(cct, rkeys); + return new CephxClientHandler(cct, rkeys); case CEPH_AUTH_NONE: return new AuthNoneClientHandler{cct}; default: return NULL; } } - -// explicitly instantiate only the classes we need -#ifdef WITH_SEASTAR -template AuthClientHandler* -AuthClientHandler::create( - CephContext *cct, - int proto, - RotatingKeyRing *rkeys); -#else -template AuthClientHandler* -AuthClientHandler::create( - CephContext *cct, - int proto, - RotatingKeyRing *rkeys); -#endif diff --git a/src/auth/AuthClientHandler.h b/src/auth/AuthClientHandler.h index a395c326838f3..7e9d0cc910da5 100644 --- a/src/auth/AuthClientHandler.h +++ b/src/auth/AuthClientHandler.h @@ -17,11 +17,10 @@ #include "auth/Auth.h" -#include "common/lock_policy.h" class CephContext; struct MAuthReply; -template class RotatingKeyRing; +class RotatingKeyRing; class AuthClientHandler { protected: @@ -59,9 +58,7 @@ public: virtual void set_global_id(uint64_t id) = 0; - template - static AuthClientHandler* - create(CephContext *cct, int proto, RotatingKeyRing *rkeys); + static AuthClientHandler* create(CephContext* cct, int proto, RotatingKeyRing* rkeys); protected: virtual void validate_tickets() = 0; }; diff --git a/src/auth/RotatingKeyRing.cc b/src/auth/RotatingKeyRing.cc index 6fade65c8a01d..c2d614a143269 100644 --- a/src/auth/RotatingKeyRing.cc +++ b/src/auth/RotatingKeyRing.cc @@ -9,30 +9,26 @@ #define dout_prefix *_dout << "auth: " -template -bool RotatingKeyRing::need_new_secrets() const +bool RotatingKeyRing::need_new_secrets() const { std::lock_guard l{lock}; return secrets.need_new_secrets(); } -template -bool RotatingKeyRing::need_new_secrets(utime_t now) const +bool RotatingKeyRing::need_new_secrets(utime_t now) const { std::lock_guard l{lock}; return secrets.need_new_secrets(now); } -template -void RotatingKeyRing::set_secrets(RotatingSecrets&& s) +void RotatingKeyRing::set_secrets(RotatingSecrets&& s) { std::lock_guard l{lock}; secrets = std::move(s); dump_rotating(); } -template -void RotatingKeyRing::dump_rotating() const +void RotatingKeyRing::dump_rotating() const { ldout(cct, 10) << "dump_rotating:" << dendl; for (map::const_iterator iter = secrets.secrets.begin(); @@ -41,15 +37,13 @@ void RotatingKeyRing::dump_rotating() const ldout(cct, 10) << " id " << iter->first << " " << iter->second << dendl; } -template -bool RotatingKeyRing::get_secret(const EntityName& name, CryptoKey& secret) const +bool RotatingKeyRing::get_secret(const EntityName& name, CryptoKey& secret) const { std::lock_guard l{lock}; return keyring->get_secret(name, secret); } -template -bool RotatingKeyRing::get_service_secret(uint32_t service_id_, uint64_t secret_id, +bool RotatingKeyRing::get_service_secret(uint32_t service_id_, uint64_t secret_id, CryptoKey& secret) const { std::lock_guard l{lock}; @@ -72,15 +66,7 @@ bool RotatingKeyRing::get_service_secret(uint32_t service_id_, uint64_t secr return true; } -template -KeyRing *RotatingKeyRing::get_keyring() +KeyRing* RotatingKeyRing::get_keyring() { return keyring; } - -// explicitly instantiate only the classes we need -#ifdef WITH_SEASTAR -template class RotatingKeyRing; -#else -template class RotatingKeyRing; -#endif diff --git a/src/auth/RotatingKeyRing.h b/src/auth/RotatingKeyRing.h index 6d094c22ef7be..04277a8be8502 100644 --- a/src/auth/RotatingKeyRing.h +++ b/src/auth/RotatingKeyRing.h @@ -15,7 +15,7 @@ #ifndef CEPH_ROTATINGKEYRING_H #define CEPH_ROTATINGKEYRING_H -#include "common/lock_mutex.h" +#include "common/ceph_mutex.h" #include "auth/Auth.h" /* @@ -25,20 +25,20 @@ class KeyRing; class CephContext; -template class RotatingKeyRing : public KeyStore { CephContext *cct; uint32_t service_id; RotatingSecrets secrets; KeyRing *keyring; - mutable LockMutexT lock; + mutable ceph::mutex lock; public: RotatingKeyRing(CephContext *cct_, uint32_t s, KeyRing *kr) : cct(cct_), service_id(s), keyring(kr), - lock(LockMutex::create("RotatingKeyRing::lock")) {} + lock{ceph::make_mutex("RotatingKeyRing::lock")} + {} bool need_new_secrets() const; bool need_new_secrets(utime_t now) const; diff --git a/src/auth/cephx/CephxClientHandler.cc b/src/auth/cephx/CephxClientHandler.cc index 3559fd7372866..b4bbdc6cc03b0 100644 --- a/src/auth/cephx/CephxClientHandler.cc +++ b/src/auth/cephx/CephxClientHandler.cc @@ -28,8 +28,7 @@ #undef dout_prefix #define dout_prefix *_dout << "cephx client: " -template -int CephxClientHandler::build_request(bufferlist& bl) const +int CephxClientHandler::build_request(bufferlist& bl) const { ldout(cct, 10) << "build_request" << dendl; @@ -97,8 +96,7 @@ int CephxClientHandler::build_request(bufferlist& bl) const return 0; } -template -bool CephxClientHandler::_need_tickets() const +bool CephxClientHandler::_need_tickets() const { // do not bother (re)requesting tickets if we *only* need the MGR // ticket; that can happen during an upgrade and we want to avoid a @@ -107,8 +105,7 @@ bool CephxClientHandler::_need_tickets() const return need && need != CEPH_ENTITY_TYPE_MGR; } -template -int CephxClientHandler::handle_response(int ret, bufferlist::const_iterator& indata) +int CephxClientHandler::handle_response(int ret, bufferlist::const_iterator& indata) { ldout(cct, 10) << "handle_response ret = " << ret << dendl; @@ -201,16 +198,14 @@ int CephxClientHandler::handle_response(int ret, bufferlist::const_iterator& } -template -AuthAuthorizer *CephxClientHandler::build_authorizer(uint32_t service_id) const +AuthAuthorizer *CephxClientHandler::build_authorizer(uint32_t service_id) const { ldout(cct, 10) << "build_authorizer for service " << ceph_entity_type_name(service_id) << dendl; return tickets.build_authorizer(service_id); } -template -bool CephxClientHandler::build_rotating_request(bufferlist& bl) const +bool CephxClientHandler::build_rotating_request(bufferlist& bl) const { ldout(cct, 10) << "build_rotating_request" << dendl; CephXRequestHeader header; @@ -219,8 +214,7 @@ bool CephxClientHandler::build_rotating_request(bufferlist& bl) const return true; } -template -void CephxClientHandler::prepare_build_request() +void CephxClientHandler::prepare_build_request() { ldout(cct, 10) << "validate_tickets: want=" << want << " need=" << need << " have=" << have << dendl; @@ -231,15 +225,13 @@ void CephxClientHandler::prepare_build_request() ticket_handler = &(tickets.get_handler(CEPH_ENTITY_TYPE_AUTH)); } -template -void CephxClientHandler::validate_tickets() +void CephxClientHandler::validate_tickets() { // lock should be held for write tickets.validate_tickets(want, have, need); } -template -bool CephxClientHandler::need_tickets() +bool CephxClientHandler::need_tickets() { validate_tickets(); @@ -250,10 +242,3 @@ bool CephxClientHandler::need_tickets() return _need_tickets(); } - -// explicitly instantiate only the classes we need -#ifdef WITH_SEASTAR -template class CephxClientHandler; -#else -template class CephxClientHandler; -#endif diff --git a/src/auth/cephx/CephxClientHandler.h b/src/auth/cephx/CephxClientHandler.h index 7a91e0322c07c..9d821281588d4 100644 --- a/src/auth/cephx/CephxClientHandler.h +++ b/src/auth/cephx/CephxClientHandler.h @@ -22,7 +22,6 @@ class CephContext; class KeyRing; -template class CephxClientHandler : public AuthClientHandler { bool starting; @@ -32,12 +31,12 @@ class CephxClientHandler : public AuthClientHandler { CephXTicketManager tickets; CephXTicketHandler* ticket_handler; - RotatingKeyRing *rotating_secrets; + RotatingKeyRing* rotating_secrets; KeyRing *keyring; public: CephxClientHandler(CephContext *cct_, - RotatingKeyRing *rsecrets) + RotatingKeyRing *rsecrets) : AuthClientHandler(cct_), starting(false), server_challenge(0), diff --git a/src/common/lock_cond.h b/src/common/lock_cond.h deleted file mode 100644 index 2cf067c80c556..0000000000000 --- a/src/common/lock_cond.h +++ /dev/null @@ -1,42 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- - -#pragma once - -#include "lock_policy.h" -#include "lock_mutex.h" -#ifdef NDEBUG -#include -#else -#include "common/condition_variable_debug.h" -#endif - -class SharedLRUTest; - -namespace ceph { - -// empty helper class except when the template argument is LockPolicy::MUTEX -template -class LockCond { - // lockless condition_variable cannot be represented using - // std::condition_variables interfaces. -}; - -#ifdef NDEBUG -template<> -class LockCond -{ -public: - using type = std::condition_variable; -}; -#else -template<> -class LockCond { -public: - using type = ceph::condition_variable_debug; -}; - -#endif // NDEBUG - -template using LockCondT = typename LockCond::type; - -} // namespace ceph diff --git a/src/common/lock_mutex.h b/src/common/lock_mutex.h deleted file mode 100644 index 8203d8d3edfcf..0000000000000 --- a/src/common/lock_mutex.h +++ /dev/null @@ -1,61 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- - -#pragma once - -#include "lock_policy.h" -#ifdef NDEBUG -#include -#else -#include "mutex_debug.h" -#endif - -namespace ceph { - -// empty helper class except when the template argument is LockPolicy::MUTEX -template -class LockMutex { - struct Dummy { - void lock() {} - bool try_lock() { - return true; - } - void unlock() {} - bool is_locked() const { - return true; - } - }; -public: - using type = Dummy; - // discard the constructor params - template - static Dummy create(Args&& ...) { - return Dummy{}; - } -}; - -#ifdef NDEBUG -template<> -class LockMutex { -public: - using type = std::mutex; - // discard the constructor params - template - static std::mutex create(Args&& ...) { - return std::mutex{}; - } -}; -#else -template<> -class LockMutex { -public: - using type = ceph::mutex_debug; - template - static ceph::mutex_debug create(Args&& ...args) { - return ceph::mutex_debug{std::forward(args)...}; - } -}; -#endif // NDEBUG - -template using LockMutexT = typename LockMutex::type; - -} // namespace ceph diff --git a/src/common/lock_policy.h b/src/common/lock_policy.h deleted file mode 100644 index 0be74a8f9508c..0000000000000 --- a/src/common/lock_policy.h +++ /dev/null @@ -1,12 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- - -#pragma once - -namespace ceph { - -enum class LockPolicy { - SINGLE, - MUTEX, -}; - -} diff --git a/src/common/lock_shared_mutex.h b/src/common/lock_shared_mutex.h deleted file mode 100644 index a5b0be1299f9a..0000000000000 --- a/src/common/lock_shared_mutex.h +++ /dev/null @@ -1,67 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- -// vim: ts=8 sw=2 smarttab - -#pragma once - -#include "lock_policy.h" -#ifdef NDEBUG -#include -#else -#include "shared_mutex_debug.h" -#endif - -namespace ceph { - -// empty helper class except when the template argument is LockPolicy::MUTEX -template -class SharedMutex { - struct Dummy { - // exclusive lock - void lock() {} - bool try_lock() { - return true; - } - void unlock() {} - // shared lock - void lock_shared() {} - bool try_lock_shared() { - return true; - } - void unlock_shared() {} - }; -public: - using type = Dummy; - template - static Dummy create(Args&& ...) { - return Dummy{}; - } -}; - -#ifdef NDEBUG -template<> -class SharedMutex -{ -public: - using type = std::shared_mutex; - // discard the constructor params - template - static std::shared_mutex create(Args&&... args) { - return std::shared_mutex{}; - } -}; -#else -template<> -class SharedMutex { -public: - using type = ceph::shared_mutex_debug; - template - static ceph::shared_mutex_debug create(Args&&... args) { - return ceph::shared_mutex_debug{std::forward(args)...}; - } -}; -#endif // NDEBUG - -template using SharedMutexT = typename SharedMutex::type; - -} // namespace ceph - diff --git a/src/common/lock_shared_ptr.h b/src/common/lock_shared_ptr.h deleted file mode 100644 index e915d7b78473b..0000000000000 --- a/src/common/lock_shared_ptr.h +++ /dev/null @@ -1,23 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- - -#pragma once - -#include "lock_policy.h" -#include -#include - -namespace ceph { - -template -struct SharedPtrTrait { - template using shared_ptr = boost::local_shared_ptr; - template using weak_ptr = boost::weak_ptr; -}; - -template<> -struct SharedPtrTrait { - template using shared_ptr = std::shared_ptr; - template using weak_ptr = std::weak_ptr; -}; - -} diff --git a/src/common/shared_cache.hpp b/src/common/shared_cache.hpp index 5aa4963722d0a..283bff401c5aa 100644 --- a/src/common/shared_cache.hpp +++ b/src/common/shared_cache.hpp @@ -17,26 +17,31 @@ #include #include +#ifdef WITH_SEASTAR +#include +#else +#include +#endif +#include "common/ceph_mutex.h" #include "common/dout.h" -#include "common/lock_cond.h" -#include "common/lock_mutex.h" -#include "common/lock_policy.h" -#include "common/lock_shared_ptr.h" #include "include/unordered_map.h" // re-include our assert to clobber the system one; fix dout: #include "include/ceph_assert.h" -template +template class SharedLRU { CephContext *cct; - using shared_ptr_trait_t = SharedPtrTrait; - using VPtr = typename shared_ptr_trait_t::template shared_ptr; - using WeakVPtr = typename shared_ptr_trait_t::template weak_ptr; - LockMutexT lock; +#ifdef WITH_SEASTAR + using VPtr = boost::local_shared_ptr; + using WeakVPtr = boost::weak_ptr; +#else + using VPtr = std::shared_ptr; + using WeakVPtr = std::weak_ptr; +#endif + ceph::mutex lock; size_t max_size; - LockCondT cond; + ceph::condition_variable cond; unsigned size; public: int waiting; @@ -99,7 +104,7 @@ private: public: SharedLRU(CephContext *cct = NULL, size_t max_size = 20) : cct(cct), - lock{LockMutex::create("SharedLRU::lock")}, + lock{ceph::make_mutex("SharedLRU::lock")}, max_size(max_size), size(0), waiting(0) { contents.rehash(max_size); diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc index 356e0cff9237d..058da24d48ba5 100644 --- a/src/mon/MonClient.cc +++ b/src/mon/MonClient.cc @@ -428,7 +428,7 @@ int MonClient::init() } rotating_secrets.reset( - new RotatingKeyRing(cct, cct->get_module_type(), keyring.get())); + new RotatingKeyRing(cct, cct->get_module_type(), keyring.get())); initialized = true; @@ -1230,7 +1230,7 @@ void MonConnection::start(epoch_t epoch, int MonConnection::handle_auth(MAuthReply* m, const EntityName& entity_name, uint32_t want_keys, - RotatingKeyRing* keyring) + RotatingKeyRing* keyring) { if (state == State::NEGOTIATING) { int r = _negotiate(m, entity_name, want_keys, keyring); @@ -1249,7 +1249,7 @@ int MonConnection::handle_auth(MAuthReply* m, int MonConnection::_negotiate(MAuthReply *m, const EntityName& entity_name, uint32_t want_keys, - RotatingKeyRing* keyring) + RotatingKeyRing* keyring) { if (auth && (int)m->protocol == auth->get_protocol()) { // good, negotiation completed diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h index fd3339a788e22..baeedf5d60f61 100644 --- a/src/mon/MonClient.h +++ b/src/mon/MonClient.h @@ -21,7 +21,6 @@ #include "MonMap.h" #include "MonSub.h" -#include "common/lock_policy.h" #include "common/Timer.h" #include "common/Finisher.h" #include "common/config.h" @@ -38,7 +37,7 @@ class AuthAuthorizer; class AuthMethodList; class AuthClientHandler; class KeyRing; -template class RotatingKeyRing; +class RotatingKeyRing; struct MonClientPinger : public Dispatcher { @@ -108,7 +107,7 @@ public: int handle_auth(MAuthReply *m, const EntityName& entity_name, uint32_t want_keys, - RotatingKeyRing* keyring); + RotatingKeyRing* keyring); int authenticate(MAuthReply *m); void start(epoch_t epoch, const EntityName& entity_name, @@ -128,7 +127,7 @@ private: int _negotiate(MAuthReply *m, const EntityName& entity_name, uint32_t want_keys, - RotatingKeyRing* keyring); + RotatingKeyRing* keyring); private: CephContext *cct; @@ -276,7 +275,7 @@ public: } std::unique_ptr keyring; - std::unique_ptr> rotating_secrets; + std::unique_ptr rotating_secrets; public: explicit MonClient(CephContext *cct_); -- 2.39.5