Re: [PATCH v9 2/5] ceph: add caps perf metric for each session

Xiubo Li <xiubli@xxxxxxxxxx> · Mon, 9 Mar 2020 20:26:59 +0800

On 2020/3/9 19:51, Jeff Layton wrote:
On Mon, 2020-03-09 at 03:37 -0400, xiubli@xxxxxxxxxx wrote:
From: Xiubo Li <xiubli@xxxxxxxxxx>

This will fulfill the cap hit/mis metric stuff per-superblock,
it will count the hit/mis counters based each inode, and if one
inode's 'issued & ~revoking == mask' will mean a hit, or a miss.

item          total           miss            hit
-------------------------------------------------
caps          295             107             4119

URL: https://tracker.ceph.com/issues/43215
Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
---
  fs/ceph/acl.c        |  2 +-
  fs/ceph/caps.c       | 19 +++++++++++++++++++
  fs/ceph/debugfs.c    | 16 ++++++++++++++++
  fs/ceph/dir.c        |  5 +++--
  fs/ceph/inode.c      |  4 ++--
  fs/ceph/mds_client.c | 26 ++++++++++++++++++++++----
  fs/ceph/metric.h     | 13 +++++++++++++
  fs/ceph/super.h      |  8 +++++---
  fs/ceph/xattr.c      |  4 ++--
  9 files changed, 83 insertions(+), 14 deletions(-)

diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
index 26be652..e046574 100644
--- a/fs/ceph/acl.c
+++ b/fs/ceph/acl.c
@@ -22,7 +22,7 @@ static inline void ceph_set_cached_acl(struct inode *inode,
  	struct ceph_inode_info *ci = ceph_inode(inode);
  
  	spin_lock(&ci->i_ceph_lock);
-	if (__ceph_caps_issued_mask(ci, CEPH_CAP_XATTR_SHARED, 0))
+	if (__ceph_caps_issued_mask_metric(ci, CEPH_CAP_XATTR_SHARED, 0))
  		set_cached_acl(inode, type, acl);
  	else
  		forget_cached_acl(inode, type);
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 342a32c..efaeb67 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -912,6 +912,20 @@ int __ceph_caps_issued_mask(struct ceph_inode_info *ci, int mask, int touch)
  	return 0;
  }
  
+int __ceph_caps_issued_mask_metric(struct ceph_inode_info *ci, int mask,
+				   int touch)
+{
+	struct ceph_fs_client *fsc = ceph_sb_to_client(ci->vfs_inode.i_sb);
+	int r;
+
+	r = __ceph_caps_issued_mask(ci, mask, touch);
+	if (r)
+		ceph_update_cap_hit(&fsc->mdsc->metric);
+	else
+		ceph_update_cap_mis(&fsc->mdsc->metric);
+	return r;
+}
+
  /*
   * Return true if mask caps are currently being revoked by an MDS.
   */
@@ -2680,6 +2694,11 @@ static int try_get_cap_refs(struct inode *inode, int need, int want,
  	if (snap_rwsem_locked)
  		up_read(&mdsc->snap_rwsem);
  
+	if (!ret)
+		ceph_update_cap_mis(&mdsc->metric);
Should a negative error code here also mean a miss?

A negative error code could also from the case that if have & need == 
need, but it may fail with ret = -EAGAIN.

Or maybe could we just move this to :

if ((have & need) == need){

    hit()

} else {

   miss()

}

Thanks

BRs


+	else if (ret == 1)
+		ceph_update_cap_hit(&mdsc->metric);
+
  	dout("get_cap_refs %p ret %d got %s\n", inode,
  	     ret, ceph_cap_string(*got));
  	return ret;









