]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
auth: switch to LockPolicy templatized alternatives
authorKefu Chai <kchai@redhat.com>
Wed, 15 Aug 2018 12:31:36 +0000 (20:31 +0800)
committerKefu Chai <kchai@redhat.com>
Thu, 16 Aug 2018 09:33:48 +0000 (17:33 +0800)
this change

* is the first step to use std::mutex, etc directly instead our Mutex
  wrappers for better performance. see
  https://trello.com/c/aUSezBRH/365-stdmutex-etc-for-release-builds
* pave the road to lockless auth/ subsystem in seastar.

Signed-off-by: Kefu Chai <kchai@redhat.com>
15 files changed:
src/auth/AuthAuthorizeHandler.h
src/auth/AuthClientHandler.cc
src/auth/AuthClientHandler.h
src/auth/RotatingKeyRing.cc
src/auth/RotatingKeyRing.h
src/auth/cephx/CephxClientHandler.cc
src/auth/cephx/CephxClientHandler.h
src/auth/none/AuthNoneClientHandler.h
src/common/shared_cache.hpp
src/mds/MDSDaemon.cc
src/mgr/DaemonServer.cc
src/mon/MonClient.cc
src/mon/MonClient.h
src/mon/Monitor.cc
src/osd/OSD.cc

index f6a0e68c95989db07766466c6c0ea7e6cbdad35c..7473267dc4d46e5f5342146be6cc2860e2af9a74 100644 (file)
@@ -27,7 +27,6 @@
 
 class CephContext;
 class KeyRing;
-class RotatingKeyRing;
 
 struct AuthAuthorizeHandler {
   virtual ~AuthAuthorizeHandler() {}
index 8a75a30428930e7384f1cc9d92b3b04a62acdd8f..320d2faba4e153968811ca597590d819567f0364 100644 (file)
 #include "none/AuthNoneClientHandler.h"
 
 
-AuthClientHandler* AuthClientHandler::create(CephContext *cct, int proto,
-                                            RotatingKeyRing *rkeys)
+template<ceph::LockPolicy lp>
+AuthClientHandler<lp>*
+AuthClientHandler<lp>::create(CephContext *cct, int proto,
+                             RotatingKeyRing<lp> *rkeys)
 {
   switch (proto) {
   case CEPH_AUTH_CEPHX:
-    return new CephxClientHandler(cct, rkeys);
+    return new CephxClientHandler<lp>(cct, rkeys);
   case CEPH_AUTH_NONE:
-    return new AuthNoneClientHandler(cct, rkeys);
+    return new AuthNoneClientHandler<lp>(cct, rkeys);
   default:
     return NULL;
   }
 }
 
+// explicitly instantiate only the classes we need
+template class AuthClientHandler<ceph::LockPolicy::MUTEX>;
index be1ad642bad6a7dcfaedfa1d4dd165cb2bce1e60..140dbcc63b4d8b9a795a3551b2c4587e761cc9e6 100644 (file)
 
 
 #include "auth/Auth.h"
-#include "common/RWLock.h"
+#include "common/lock_policy.h"
+#include "common/lock_shared_mutex.h"
 
 class CephContext;
 struct MAuthReply;
-class RotatingKeyRing;
+template<ceph::LockPolicy> class RotatingKeyRing;
 
+template<ceph::LockPolicy lock_policy>
 class AuthClientHandler {
 protected:
   CephContext *cct;
@@ -31,18 +33,19 @@ protected:
   uint32_t want;
   uint32_t have;
   uint32_t need;
-  RWLock lock;
+  mutable SharedMutexT<lock_policy> lock;
 
 public:
   explicit AuthClientHandler(CephContext *cct_)
     : cct(cct_), global_id(0), want(CEPH_ENTITY_TYPE_AUTH), have(0), need(0),
-      lock("AuthClientHandler::lock") {}
+      lock{SharedMutex<lock_policy>::create("AuthClientHandler::lock")}
+  {}
   virtual ~AuthClientHandler() {}
 
   void init(const EntityName& n) { name = n; }
   
   void set_want_keys(__u32 keys) {
-    RWLock::WLocker l(lock);
+    std::unique_lock l{lock};
     want = keys | CEPH_ENTITY_TYPE_AUTH;
     validate_tickets();
   }
@@ -61,8 +64,8 @@ public:
 
   virtual void set_global_id(uint64_t id) = 0;
 
-  static AuthClientHandler* create(CephContext *cct,
-                                  int proto, RotatingKeyRing *rkeys);
+  static AuthClientHandler<lock_policy>*
+  create(CephContext *cct, int proto, RotatingKeyRing<lock_policy> *rkeys);
 protected:
   virtual void validate_tickets() = 0;
 };
index e48127aa64b85b059e5210c7720e45764e95a1af..196b1a120521871d88ad3a20d4b99e36dc78bee4 100644 (file)
@@ -9,26 +9,30 @@
 #define dout_prefix *_dout << "auth: "
 
 
-bool RotatingKeyRing::need_new_secrets() const
+template<LockPolicy lp>
+bool RotatingKeyRing<lp>::need_new_secrets() const
 {
-  Mutex::Locker l(lock);
+  std::lock_guard l{lock};
   return secrets.need_new_secrets();
 }
 
-bool RotatingKeyRing::need_new_secrets(utime_t now) const
+template<LockPolicy lp>
+bool RotatingKeyRing<lp>::need_new_secrets(utime_t now) const
 {
-  Mutex::Locker l(lock);
+  std::lock_guard l{lock};
   return secrets.need_new_secrets(now);
 }
 
-void RotatingKeyRing::set_secrets(RotatingSecrets&& s)
+template<LockPolicy lp>
+void RotatingKeyRing<lp>::set_secrets(RotatingSecrets&& s)
 {
-  Mutex::Locker l(lock);
+  std::lock_guard l{lock};
   secrets = std::move(s);
   dump_rotating();
 }
 
-void RotatingKeyRing::dump_rotating() const
+template<LockPolicy lp>
+void RotatingKeyRing<lp>::dump_rotating() const
 {
   ldout(cct, 10) << "dump_rotating:" << dendl;
   for (map<uint64_t, ExpiringCryptoKey>::const_iterator iter = secrets.secrets.begin();
@@ -37,16 +41,18 @@ void RotatingKeyRing::dump_rotating() const
     ldout(cct, 10) << " id " << iter->first << " " << iter->second << dendl;
 }
 
-bool RotatingKeyRing::get_secret(const EntityName& name, CryptoKey& secret) const
+template<LockPolicy lp>
+bool RotatingKeyRing<lp>::get_secret(const EntityName& name, CryptoKey& secret) const
 {
-  Mutex::Locker l(lock);
+  std::lock_guard l{lock};
   return keyring->get_secret(name, secret);
 }
 
-bool RotatingKeyRing::get_service_secret(uint32_t service_id_, uint64_t secret_id,
+template<LockPolicy lp>
+bool RotatingKeyRing<lp>::get_service_secret(uint32_t service_id_, uint64_t secret_id,
                                         CryptoKey& secret) const
 {
-  Mutex::Locker l(lock);
+  std::lock_guard l{lock};
 
   if (service_id_ != this->service_id) {
     ldout(cct, 0) << "do not have service " << ceph_entity_type_name(service_id_)
@@ -66,8 +72,11 @@ bool RotatingKeyRing::get_service_secret(uint32_t service_id_, uint64_t secret_i
   return true;
 }
 
-KeyRing *RotatingKeyRing::
-get_keyring()
+template<LockPolicy lp>
+KeyRing *RotatingKeyRing<lp>::get_keyring()
 {
   return keyring;
 }
+
+// explicitly instantiate only the classes we need
+template class RotatingKeyRing<LockPolicy::MUTEX>;
index 729f5c52021277cb34d3a676fa50d35e2a5c888a..6d094c22ef7beb46da17d490b231529c091d8af9 100644 (file)
@@ -15,7 +15,7 @@
 #ifndef CEPH_ROTATINGKEYRING_H
 #define CEPH_ROTATINGKEYRING_H
 
-#include "common/Mutex.h"
+#include "common/lock_mutex.h"
 #include "auth/Auth.h"
 
 /*
 class KeyRing;
 class CephContext;
 
+template<LockPolicy lock_policy>
 class RotatingKeyRing : public KeyStore {
   CephContext *cct;
   uint32_t service_id;
   RotatingSecrets secrets;
   KeyRing *keyring;
-  mutable Mutex lock;
+  mutable LockMutexT<lock_policy> lock;
 
 public:
   RotatingKeyRing(CephContext *cct_, uint32_t s, KeyRing *kr) :
     cct(cct_),
     service_id(s),
     keyring(kr),
-    lock("RotatingKeyRing::lock") {}
+    lock(LockMutex<lock_policy>::create("RotatingKeyRing::lock")) {}
 
   bool need_new_secrets() const;
   bool need_new_secrets(utime_t now) const;
index 88eec0b2a41bc18c57c1eca1ab1cde631f92f9b4..ea0b67cebbb56c3f91d935b34308042ae212968f 100644 (file)
 #undef dout_prefix
 #define dout_prefix *_dout << "cephx client: "
 
-
-int CephxClientHandler::build_request(bufferlist& bl) const
+template<LockPolicy lp>
+int CephxClientHandler<lp>::build_request(bufferlist& bl) const
 {
   ldout(cct, 10) << "build_request" << dendl;
 
-  RWLock::RLocker l(lock);
+  std::shared_lock l{lock};
 
   if (need & CEPH_ENTITY_TYPE_AUTH) {
     /* authenticate */
@@ -98,7 +98,8 @@ int CephxClientHandler::build_request(bufferlist& bl) const
   return 0;
 }
 
-bool CephxClientHandler::_need_tickets() const
+template<LockPolicy lp>
+bool CephxClientHandler<lp>::_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,10 +108,11 @@ bool CephxClientHandler::_need_tickets() const
   return need && need != CEPH_ENTITY_TYPE_MGR;
 }
 
-int CephxClientHandler::handle_response(int ret, bufferlist::const_iterator& indata)
+template<LockPolicy lp>
+int CephxClientHandler<lp>::handle_response(int ret, bufferlist::const_iterator& indata)
 {
   ldout(cct, 10) << "handle_response ret = " << ret << dendl;
-  RWLock::WLocker l(lock);
+  std::unique_lock l{lock};
   
   if (ret < 0)
     return ret; // hrm!
@@ -201,16 +203,17 @@ int CephxClientHandler::handle_response(int ret, bufferlist::const_iterator& ind
 }
 
 
-
-AuthAuthorizer *CephxClientHandler::build_authorizer(uint32_t service_id) const
+template<LockPolicy lp>
+AuthAuthorizer *CephxClientHandler<lp>::build_authorizer(uint32_t service_id) const
 {
-  RWLock::RLocker l(lock);
+  std::shared_lock l{lock};
   ldout(cct, 10) << "build_authorizer for service " << ceph_entity_type_name(service_id) << dendl;
   return tickets.build_authorizer(service_id);
 }
 
 
-bool CephxClientHandler::build_rotating_request(bufferlist& bl) const
+template<LockPolicy lp>
+bool CephxClientHandler<lp>::build_rotating_request(bufferlist& bl) const
 {
   ldout(cct, 10) << "build_rotating_request" << dendl;
   CephXRequestHeader header;
@@ -219,9 +222,10 @@ bool CephxClientHandler::build_rotating_request(bufferlist& bl) const
   return true;
 }
 
-void CephxClientHandler::prepare_build_request()
+template<LockPolicy lp>
+void CephxClientHandler<lp>::prepare_build_request()
 {
-  RWLock::WLocker l(lock);
+  std::unique_lock l{lock};
   ldout(cct, 10) << "validate_tickets: want=" << want << " need=" << need
                 << " have=" << have << dendl;
   validate_tickets();
@@ -231,15 +235,17 @@ void CephxClientHandler::prepare_build_request()
   ticket_handler = &(tickets.get_handler(CEPH_ENTITY_TYPE_AUTH));
 }
 
-void CephxClientHandler::validate_tickets()
+template<LockPolicy lp>
+void CephxClientHandler<lp>::validate_tickets()
 {
   // lock should be held for write
   tickets.validate_tickets(want, have, need);
 }
 
-bool CephxClientHandler::need_tickets()
+template<LockPolicy lp>
+bool CephxClientHandler<lp>::need_tickets()
 {
-  RWLock::WLocker l(lock);
+  std::unique_lock l{lock};
   validate_tickets();
 
   ldout(cct, 20) << "need_tickets: want=" << want
@@ -249,3 +255,6 @@ bool CephxClientHandler::need_tickets()
 
   return _need_tickets();
 }
+
+// explicitly instantiate only the classes we need
+template class CephxClientHandler<LockPolicy::MUTEX>;
index 5d7b489b204669ced289e4818c61ad5c6c5ca0f2..eb6055c739ee5260e4647c9577f30bb63a95011f 100644 (file)
@@ -22,7 +22,8 @@
 class CephContext;
 class KeyRing;
 
-class CephxClientHandler : public AuthClientHandler {
+template<LockPolicy lock_policy>
+class CephxClientHandler : public AuthClientHandler<lock_policy> {
   bool starting;
 
   /* envelope protocol parameters */
@@ -31,12 +32,20 @@ class CephxClientHandler : public AuthClientHandler {
   CephXTicketManager tickets;
   CephXTicketHandler* ticket_handler;
 
-  RotatingKeyRing *rotating_secrets;
+  RotatingKeyRing<lock_policy> *rotating_secrets;
   KeyRing *keyring;
 
+  using AuthClientHandler<lock_policy>::cct;
+  using AuthClientHandler<lock_policy>::global_id;
+  using AuthClientHandler<lock_policy>::want;
+  using AuthClientHandler<lock_policy>::have;
+  using AuthClientHandler<lock_policy>::need;
+  using AuthClientHandler<lock_policy>::lock;
+
 public:
-  CephxClientHandler(CephContext *cct_, RotatingKeyRing *rsecrets) 
-    : AuthClientHandler(cct_),
+  CephxClientHandler(CephContext *cct_,
+                    RotatingKeyRing<lock_policy> *rsecrets)
+    : AuthClientHandler<lock_policy>(cct_),
       starting(false),
       server_challenge(0),
       tickets(cct_),
@@ -48,7 +57,7 @@ public:
   }
 
   void reset() override {
-    RWLock::WLocker l(lock);
+    std::unique_lock l{lock};
     starting = true;
     server_challenge = 0;
   }
@@ -64,7 +73,7 @@ public:
   bool need_tickets() override;
 
   void set_global_id(uint64_t id) override {
-    RWLock::WLocker l(lock);
+    std::unique_lock l{lock};
     global_id = id;
     tickets.global_id = id;
   }
index ba843979e09beaeb08f013614203ee4b3877c5af..32a410ab76daa2123b580fd767dd9a183aaf5364 100644 (file)
 #include "AuthNoneProtocol.h"
 #include "common/ceph_context.h"
 #include "common/config.h"
-class AuthNoneClientHandler : public AuthClientHandler {
+
+template<LockPolicy lock_policy>
+class AuthNoneClientHandler : public AuthClientHandler<lock_policy> {
+  using AuthClientHandler<lock_policy>::cct;
+  using AuthClientHandler<lock_policy>::global_id;
+  using AuthClientHandler<lock_policy>::lock;
+
 public:
-  AuthNoneClientHandler(CephContext *cct_, RotatingKeyRing *rkeys) 
-    : AuthClientHandler(cct_) {}
+  AuthNoneClientHandler(CephContext *cct_,
+                       RotatingKeyRing<lock_policy> *rkeys)
+    : AuthClientHandler<lock_policy>(cct_) {}
 
   void reset() override { }
 
@@ -35,7 +41,7 @@ public:
   int get_protocol() const override { return CEPH_AUTH_NONE; }
   
   AuthAuthorizer *build_authorizer(uint32_t service_id) const override {
-    RWLock::RLocker l(lock);
+    std::shared_lock l{lock};
     AuthNoneAuthorizer *auth = new AuthNoneAuthorizer();
     if (auth) {
       auth->build_authorizer(cct->_conf->name, global_id);
@@ -46,7 +52,7 @@ public:
   bool need_tickets() override { return false; }
 
   void set_global_id(uint64_t id) override {
-    RWLock::WLocker l(lock);
+    std::unique_lock l{lock};
     global_id = id;
   }
 private:
index 2fdfff581360751146017ae618fa50246ca30be4..1375e4e57a955092e63ffc7366bee1b9fc7e3b67 100644 (file)
@@ -35,7 +35,7 @@ class SharedLRU {
   using WeakVPtr = typename shared_ptr_trait_t::template weak_ptr<V>;
   LockMutexT<lock_policy> lock;
   size_t max_size;
-  LockCond<lock_policy> cond;
+  LockCondT<lock_policy> cond;
   unsigned size;
 public:
   int waiting;
index 98c524556d763535fdc0f895189fffb5cb20e8c6..a64dad905b21f49065406b2607351be3cfc4ee4a 100644 (file)
@@ -1323,8 +1323,7 @@ bool MDSDaemon::ms_verify_authorizer(Connection *con, int peer_type,
   EntityName name;
   uint64_t global_id;
 
-  RotatingKeyRing *keys = monc->rotating_secrets.get();
-  if (keys) {
+  if (auto keys = monc->rotating_secrets.get(); keys) {
     is_valid = authorize_handler->verify_authorizer(
       cct, keys,
       authorizer_data, authorizer_reply, name, global_id, caps_info,
index ebdacaf69011a675e04421302e04e9ee9d63b352..702bf1629b56a4c7fe72a2f293efb62bcd64c68a 100644 (file)
@@ -188,8 +188,7 @@ bool DaemonServer::ms_verify_authorizer(
   s->inst.addr = con->get_peer_addr();
   AuthCapsInfo caps_info;
 
-  RotatingKeyRing *keys = monc->rotating_secrets.get();
-  if (keys) {
+  if (auto keys = monc->rotating_secrets.get(); keys) {
     is_valid = handler->verify_authorizer(
       cct, keys,
       authorizer_data,
index bbbf0ab99e427e06ad1ca8d8bd659561407e068c..0907c2a8ea0337eb5323405f4348d0d00a198f22 100644 (file)
@@ -427,7 +427,7 @@ int MonClient::init()
   }
 
   rotating_secrets.reset(
-    new RotatingKeyRing(cct, cct->get_module_type(), keyring.get()));
+    new RotatingKeyRing<LockPolicy::MUTEX>(cct, cct->get_module_type(), keyring.get()));
 
   initialized = true;
 
@@ -1250,7 +1250,7 @@ void MonConnection::start(epoch_t epoch,
 int MonConnection::handle_auth(MAuthReply* m,
                               const EntityName& entity_name,
                               uint32_t want_keys,
-                              RotatingKeyRing* keyring)
+                              RotatingKeyRing<LockPolicy::MUTEX>* keyring)
 {
   if (state == State::NEGOTIATING) {
     int r = _negotiate(m, entity_name, want_keys, keyring);
@@ -1269,7 +1269,7 @@ int MonConnection::handle_auth(MAuthReply* m,
 int MonConnection::_negotiate(MAuthReply *m,
                              const EntityName& entity_name,
                              uint32_t want_keys,
-                             RotatingKeyRing* keyring)
+                             RotatingKeyRing<LockPolicy::MUTEX>* keyring)
 {
   if (auth && (int)m->protocol == auth->get_protocol()) {
     // good, negotiation completed
@@ -1277,7 +1277,8 @@ int MonConnection::_negotiate(MAuthReply *m,
     return 0;
   }
 
-  auth.reset(AuthClientHandler::create(cct, m->protocol, keyring));
+  auth.reset(
+    AuthClientHandler<LockPolicy::MUTEX>::create(cct,m->protocol, keyring));
   if (!auth) {
     ldout(cct, 10) << "no handler for protocol " << m->protocol << dendl;
     if (m->result == -ENOTSUP) {
index 2c0f21805934873bcc4297c1e952da8ec99a7666..bffd2d916e67c83a150d52e8bfa161cb81253b34 100644 (file)
@@ -34,9 +34,9 @@ class MAuthRotating;
 class LogClient;
 class AuthAuthorizer;
 class AuthMethodList;
-class AuthClientHandler;
+template<LockPolicy> class AuthClientHandler;
 class KeyRing;
-class RotatingKeyRing;
+template<LockPolicy> class RotatingKeyRing;
 
 struct MonClientPinger : public Dispatcher {
 
@@ -106,7 +106,7 @@ public:
   int handle_auth(MAuthReply *m,
                  const EntityName& entity_name,
                  uint32_t want_keys,
-                 RotatingKeyRing* keyring);
+                 RotatingKeyRing<LockPolicy::MUTEX>* keyring);
   int authenticate(MAuthReply *m);
   void start(epoch_t epoch,
              const EntityName& entity_name,
@@ -118,7 +118,7 @@ public:
   ConnectionRef get_con() {
     return con;
   }
-  std::unique_ptr<AuthClientHandler>& get_auth() {
+  std::unique_ptr<AuthClientHandler<LockPolicy::MUTEX>>& get_auth() {
     return auth;
   }
 
@@ -126,7 +126,7 @@ private:
   int _negotiate(MAuthReply *m,
                 const EntityName& entity_name,
                 uint32_t want_keys,
-                RotatingKeyRing* keyring);
+                RotatingKeyRing<LockPolicy::MUTEX>* keyring);
 
 private:
   CephContext *cct;
@@ -139,7 +139,7 @@ private:
   State state = State::NONE;
   ConnectionRef con;
 
-  std::unique_ptr<AuthClientHandler> auth;
+  std::unique_ptr<AuthClientHandler<LockPolicy::MUTEX>> auth;
   uint64_t global_id;
 };
 
@@ -192,7 +192,7 @@ private:
   bool got_config = false;
 
   // authenticate
-  std::unique_ptr<AuthClientHandler> auth;
+  std::unique_ptr<AuthClientHandler<LockPolicy::MUTEX>> auth;
   uint32_t want_keys = 0;
   uint64_t global_id = 0;
   Cond auth_cond;
@@ -337,7 +337,7 @@ public:
   }
   
   std::unique_ptr<KeyRing> keyring;
-  std::unique_ptr<RotatingKeyRing> rotating_secrets;
+  std::unique_ptr<RotatingKeyRing<LockPolicy::MUTEX>> rotating_secrets;
 
  public:
   explicit MonClient(CephContext *cct_);
index bf93a7566020b765dee2043d35f9401161ae43f5..8da36176c0a6232a4d83876fb1bbc515f62a6c4d 100644 (file)
@@ -5713,7 +5713,7 @@ bool Monitor::ms_get_authorizer(int service_id, AuthAuthorizer **authorizer,
   if (!auth_cluster_required.is_supported_auth(CEPH_AUTH_CEPHX)) {
     // auth_none
     dout(20) << __func__ << " building auth_none authorizer" << dendl;
-    AuthNoneClientHandler handler(g_ceph_context, nullptr);
+    AuthNoneClientHandler<LockPolicy::MUTEX> handler(g_ceph_context, nullptr);
     handler.set_global_id(0);
     *authorizer = handler.build_authorizer(service_id);
     return true;
index e83e29b8bb6a2594eafe49fc82e1ddcf3a970881..26da1801d20043c8b1c43bf7755770e36d5e777d 100644 (file)
@@ -6618,7 +6618,7 @@ bool OSD::ms_verify_authorizer(
   uint64_t global_id;
   uint64_t auid = CEPH_AUTH_UID_DEFAULT;
 
-  RotatingKeyRing *keys = monc->rotating_secrets.get();
+  auto keys = monc->rotating_secrets.get();
   if (keys) {
     isvalid = authorize_handler->verify_authorizer(
       cct, keys,