mds: optimize revoking stale caps

For caps that are not being revoked and don't have writeable range
and don't want exclusive caps or file read/write. there is no need
to call Locker::revoke_stale_caps(Capability*). Because these caps
don't need recover and don't affect eval_gather()/try_eval().

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>

diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc
index 0bd1410fec87e73ed92cf91670eca41c25731972..50507635118d2050c1a4542bad30be09c86b3494 100644 (file)
--- a/src/mds/CInode.cc
+++ b/src/mds/CInode.cc
@@ -216,7 +216,7 @@ ostream& operator<<(ostream& out, const CInode& in)
       if (p.second.issued() != p.second.pending())
        out << "/" << ccap_string(p.second.issued());
       out << "/" << ccap_string(p.second.wanted())
-         << "@" << p.second.get_last_sent();
+         << "@" << p.second.get_last_seq();
       first = false;
     }
     out << "}";
@@ -4599,7 +4599,7 @@ void CInode::dump(Formatter *f, int flags) const
       f->dump_string("pending", ccap_string(cap->pending()));
       f->dump_string("issued", ccap_string(cap->issued()));
       f->dump_string("wanted", ccap_string(cap->wanted()));
-      f->dump_int("last_sent", cap->get_last_sent());
+      f->dump_int("last_sent", cap->get_last_seq());
       f->close_section();
     }
     f->close_section();

diff --git a/src/mds/Capability.cc b/src/mds/Capability.cc
index 21be39d6f4e1211f753d36aec638206904db104d..eadbd5cdff883544f77bd1960543b676064df988 100644 (file)
--- a/src/mds/Capability.cc
+++ b/src/mds/Capability.cc
@@ -153,8 +153,10 @@ Capability::Capability(CInode *i, Session *s, uint64_t id) :
   _pending(0), _issued(0), last_sent(0), last_issue(0), mseq(0),
   suppress(0), state(0)
 {
-  if (session)
+  if (session) {
     session->touch_cap_bottom(this);
+    cap_gen = session->get_cap_gen();
+  }
 }
 
 client_t Capability::get_client() const
@@ -167,6 +169,29 @@ bool Capability::is_stale() const
   return session ? session->is_stale() : false;
 }
 
+bool Capability::is_valid() const
+{
+  return !session || session->get_cap_gen() == cap_gen;
+}
+
+void Capability::revalidate()
+{
+  if (is_valid())
+    return;
+
+  if (_pending & ~CEPH_CAP_PIN)
+    inc_last_seq();
+
+  bool was_revoking = _issued & ~_pending;
+  _pending = _issued = CEPH_CAP_PIN;
+  _revokes.clear();
+
+  cap_gen = session->get_cap_gen();
+
+  if (was_revoking)
+    maybe_clear_notable();
+}
+
 void Capability::mark_notable()
 {
   state |= STATE_NOTABLE;

diff --git a/src/mds/Capability.h b/src/mds/Capability.h
index 2081e055e1c4d8a4f8bc272c6a8b26acc00902c9..38681d95becc908ff16e2d03c9a6337a147088e6 100644 (file)
--- a/src/mds/Capability.h
+++ b/src/mds/Capability.h
@@ -121,11 +121,16 @@ public:
 
   const Capability& operator=(const Capability& other) = delete;
 
-  int pending() const { return _pending; }
-  int issued() const { return _issued; }
-  bool is_null() const { return !_pending && _revokes.empty(); }
+  int pending() const {
+    return is_valid() ? _pending : (_pending & CEPH_CAP_PIN);
+  }
+  int issued() const {
+    return is_valid() ? _issued : (_issued & CEPH_CAP_PIN);
+  }
 
   ceph_seq_t issue(unsigned c) {
+    revalidate();
+
     if (_pending & ~c) {
       // revoking (and maybe adding) bits.  note caps prior to this revocation
       _revokes.emplace_back(_pending, last_sent, last_issue);
@@ -150,6 +155,8 @@ public:
     return last_sent;
   }
   ceph_seq_t issue_norevoke(unsigned c) {
+    revalidate();
+
     _pending |= c;
     _issued |= c;
     //check_rdcaps_list();
@@ -205,7 +212,6 @@ public:
   ceph_seq_t get_mseq() const { return mseq; }
   void inc_mseq() { mseq++; }
 
-  ceph_seq_t get_last_sent() const { return last_sent; }
   utime_t get_last_issue_stamp() const { return last_issue_stamp; }
   utime_t get_last_revoke_stamp() const { return last_revoke_stamp; }
 
@@ -265,7 +271,11 @@ public:
   void set_wanted(int w);
 
   void inc_last_seq() { last_sent++; }
-  ceph_seq_t get_last_seq() const { return last_sent; }
+  ceph_seq_t get_last_seq() const {
+    if (!is_valid() && (_pending & ~CEPH_CAP_PIN))
+      return last_sent + 1;
+    return last_sent;
+  }
   ceph_seq_t get_last_issue() const { return last_issue; }
 
   void reset_seq() {
@@ -275,7 +285,7 @@ public:
   
   // -- exports --
   Export make_export() const {
-    return Export(cap_id, _wanted, issued(), pending(), client_follows, last_sent, mseq+1, last_issue_stamp);
+    return Export(cap_id, wanted(), issued(), pending(), client_follows, get_last_seq(), mseq+1, last_issue_stamp);
   }
   void merge(const Export& other, bool auth_cap) {
     if (!is_stale()) {
@@ -341,6 +351,7 @@ private:
   Session *session;
 
   uint64_t cap_id;
+  uint32_t cap_gen;
 
   __u32 _wanted;     // what the client wants (ideally)
 
@@ -368,6 +379,9 @@ private:
     }
   }
 
+  bool is_valid() const;
+  void revalidate();
+
   void mark_notable();
   void maybe_clear_notable();
 };

diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index b902b737f7c669b54c64896a5e76792130064fcb..c9ac1853dc53d287eafc0c77de25f4fffa497eee 100644 (file)
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -2110,19 +2110,27 @@ void Locker::issue_truncate(CInode *in)
     check_inode_max_size(in);
 }
 
-
-void Locker::revoke_stale_caps(Capability *cap)
+void Locker::revoke_stale_caps(Session *session)
 {
-  CInode *in = cap->get_inode();
-  if (in->state_test(CInode::STATE_EXPORTINGCAPS)) {
-    // if export succeeds, the cap will be removed. if export fails, we need to
-    // revoke the cap if it's still stale.
-    in->state_set(CInode::STATE_EVALSTALECAPS);
-    return;
-  }
+  dout(10) << "revoke_stale_caps for " << session->info.inst.name << dendl;
 
-  int issued = cap->issued();
-  if (issued & ~CEPH_CAP_PIN) {
+  std::vector<CInode*> to_eval;
+
+  for (auto p = session->caps.begin(); !p.end(); ) {
+    Capability *cap = *p;
+    ++p;
+    if (!cap->is_notable()) {
+      // the rest ones are not being revoked and don't have writeable range
+      // and don't want exclusive caps or want file read/write. They don't
+      // need recover, they don't affect eval_gather()/try_eval()
+      break;
+    }
+
+    int issued = cap->issued();
+    if (!(issued & ~CEPH_CAP_PIN))
+      continue;
+
+    CInode *in = cap->get_inode();
     dout(10) << " revoking " << ccap_string(issued) << " on " << *in << dendl;
     cap->revoke();
 
@@ -2130,26 +2138,31 @@ void Locker::revoke_stale_caps(Capability *cap)
        in->inode.client_ranges.count(cap->get_client()))
       in->state_set(CInode::STATE_NEEDSRECOVER);
 
-    if (!in->filelock.is_stable()) eval_gather(&in->filelock);
-    if (!in->linklock.is_stable()) eval_gather(&in->linklock);
-    if (!in->authlock.is_stable()) eval_gather(&in->authlock);
-    if (!in->xattrlock.is_stable()) eval_gather(&in->xattrlock);
-
-    if (in->is_auth()) {
-      try_eval(in, CEPH_CAP_LOCKS);
-    } else {
-      request_inode_file_caps(in);
-    }
+    // eval lock/inode may finish contexts, which may modify other cap's position
+    // in the session->caps.
+    to_eval.push_back(in);
   }
-}
 
-void Locker::revoke_stale_caps(Session *session)
-{
-  dout(10) << "revoke_stale_caps for " << session->info.inst.name << dendl;
+  // invalidate the rest
+  session->inc_cap_gen();
 
-  for (xlist<Capability*>::iterator p = session->caps.begin(); !p.end(); ++p) {
-    Capability *cap = *p;
-    revoke_stale_caps(cap);
+  for (auto in : to_eval) {
+    if (in->state_test(CInode::STATE_EXPORTINGCAPS))
+      continue;
+
+    if (!in->filelock.is_stable())
+      eval_gather(&in->filelock);
+    if (!in->linklock.is_stable())
+      eval_gather(&in->linklock);
+    if (!in->authlock.is_stable())
+      eval_gather(&in->authlock);
+    if (!in->xattrlock.is_stable())
+      eval_gather(&in->xattrlock);
+
+    if (in->is_auth())
+      try_eval(in, CEPH_CAP_LOCKS);
+    else
+      request_inode_file_caps(in);
   }
 }
 
@@ -2308,7 +2321,7 @@ void Locker::calc_new_client_ranges(CInode *in, uint64_t size, bool update,
   // increase ranges as appropriate.
   // shrink to 0 if no WR|BUFFER caps issued.
   for (auto &p : in->client_caps) {
-    if ((p.second.issued() | p.second.wanted()) & (CEPH_CAP_FILE_WR|CEPH_CAP_FILE_BUFFER)) {
+    if ((p.second.issued() | p.second.wanted()) & CEPH_CAP_ANY_FILE_WR) {
       client_writeable_range_t& nr = (*new_ranges)[p.first];
       nr.range.first = 0;
       if (latest->client_ranges.count(p.first)) {
@@ -3056,7 +3069,7 @@ public:
 void Locker::kick_issue_caps(CInode *in, client_t client, ceph_seq_t seq)
 {
   Capability *cap = in->get_client_cap(client);
-  if (!cap || cap->get_last_sent() != seq)
+  if (!cap || cap->get_last_seq() != seq)
     return;
   if (in->is_frozen()) {
     dout(10) << "kick_issue_caps waiting for unfreeze on " << *in << dendl;
@@ -3538,16 +3551,20 @@ void Locker::_do_cap_release(client_t client, inodeno_t ino, uint64_t cap_id,
     eval_cap_gather(in);
     return;
   }
-  remove_client_cap(in, client);
+  remove_client_cap(in, cap);
 }
 
-void Locker::remove_client_cap(CInode *in, client_t client)
+void Locker::remove_client_cap(CInode *in, Capability *cap)
 {
+  client_t client = cap->get_client();
   // clean out any pending snapflush state
   if (!in->client_need_snapflush.empty())
     _do_null_snapflush(in, client);
 
+  bool notable = cap->is_notable();
   in->remove_client_cap(client);
+  if (!notable)
+    return;
 
   if (in->is_auth()) {
     // make sure we clear out the client byte range

diff --git a/src/mds/Locker.h b/src/mds/Locker.h
index 015a87993e7e5f61e45f3a0f7d5a99877d1fb140..5e935b5c8276a901acd7e2e891fd9698ea567a88 100644 (file)
--- a/src/mds/Locker.h
+++ b/src/mds/Locker.h
@@ -179,7 +179,7 @@ public:
   void kick_cap_releases(MDRequestRef& mdr);
   void kick_issue_caps(CInode *in, client_t client, ceph_seq_t seq);
 
-  void remove_client_cap(CInode *in, client_t client);
+  void remove_client_cap(CInode *in, Capability *cap);
 
   void get_late_revoking_clients(std::list<client_t> *result, double timeout) const;
 
@@ -244,7 +244,6 @@ public:
   void issue_caps_set(std::set<CInode*>& inset);
   void issue_truncate(CInode *in);
   void revoke_stale_caps(Session *session);
-  void revoke_stale_caps(Capability *cap);
   void resume_stale_caps(Session *session);
   void remove_stale_leases(Session *session);
 

diff --git a/src/mds/Migrator.cc b/src/mds/Migrator.cc
index e0de6be9f56d913c32f5be9b053b602f19eca695..1d777ff6e4e6b8c0a2efc8a4d999e0f0e29304b7 100644 (file)
--- a/src/mds/Migrator.cc
+++ b/src/mds/Migrator.cc
@@ -1999,10 +1999,9 @@ void Migrator::export_reverse(CDir *dir, export_state_t& stat)
     bool need_issue = false;
     for (auto &p : in->client_caps) {
       Capability *cap = &p.second;
-      if (cap->is_stale()) {
-       mds->locker->revoke_stale_caps(cap);
-      } else {
+      if (!cap->is_stale()) {
        need_issue = true;
+       break;
       }
     }
     if (need_issue &&

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index b864505a9ca50c6ebd9cb3bac14e1502f468902c..5482f37cf57b4a158f3d960266cbf0efbfee25db 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -710,7 +710,7 @@ void Server::_session_logged(Session *session, uint64_t state_seq, bool open, ve
       Capability *cap = session->caps.front();
       CInode *in = cap->get_inode();
       dout(20) << " killing capability " << ccap_string(cap->issued()) << " on " << *in << dendl;
-      mds->locker->remove_client_cap(in, session->get_client());
+      mds->locker->remove_client_cap(in, cap);
     }
     while (!session->leases.empty()) {
       ClientLease *r = session->leases.front();

diff --git a/src/mds/SessionMap.h b/src/mds/SessionMap.h
index 870fb3e9ea496aeb7374716f0673e8da30c5fdb4..6d8478e4e0af6f6daf79749b9cf0e0ef14a9e52d 100644 (file)
--- a/src/mds/SessionMap.h
+++ b/src/mds/SessionMap.h
@@ -243,6 +243,7 @@ public:
 
   // -- caps --
 private:
+  uint32_t cap_gen;
   version_t cap_push_seq;        // cap push seq #
   map<version_t, MDSInternalContextBase::vec > waitfor_flush; // flush session messages
 
@@ -252,7 +253,9 @@ public:
   time last_cap_renew = clock::zero();
   time last_seen = clock::zero();
 
-public:
+  void inc_cap_gen() { ++cap_gen; }
+  uint32_t get_cap_gen() const { return cap_gen; }
+
   version_t inc_push_seq() { return ++cap_push_seq; }
   version_t get_push_seq() const { return cap_push_seq; }
 
@@ -373,7 +376,7 @@ public:
     recall_release_count(0), auth_caps(g_ceph_context),
     connection(NULL), item_session_list(this),
     requests(0),  // member_offset passed to front() manually
-    cap_push_seq(0),
+    cap_gen(0), cap_push_seq(0),
     lease_seq(0),
     completed_requests_dirty(false),
     num_trim_flushes_warnings(0),