* [PATCH v9 1/5] ceph: add global dentry lease metric support
2020-03-09 7:37 [PATCH v9 0/5] ceph: add perf metrics support xiubli
@ 2020-03-09 7:37 ` xiubli
2020-03-09 7:37 ` [PATCH v9 2/5] ceph: add caps perf metric for each session xiubli
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: xiubli @ 2020-03-09 7:37 UTC (permalink / raw)
To: jlayton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel, Xiubo Li
From: Xiubo Li <xiubli@redhat.com>
For the dentry lease we will only count the hit/miss info triggered
from the vfs calls, for the cases like request reply handling and
perodically ceph_trim_dentries() we will ignore them.
Currently only the debugfs is support:
The output will be:
item total miss hit
-------------------------------------------------
d_lease 11 7 141
URL: https://tracker.ceph.com/issues/43215
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
fs/ceph/debugfs.c | 32 ++++++++++++++++++++++++++++----
fs/ceph/dir.c | 12 ++++++++++++
fs/ceph/mds_client.c | 37 +++++++++++++++++++++++++++++++++++--
fs/ceph/mds_client.h | 4 ++++
fs/ceph/metric.h | 11 +++++++++++
fs/ceph/super.h | 1 +
6 files changed, 91 insertions(+), 6 deletions(-)
create mode 100644 fs/ceph/metric.h
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 481ac97..15975ba 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -124,6 +124,22 @@ static int mdsc_show(struct seq_file *s, void *p)
return 0;
}
+static int metric_show(struct seq_file *s, void *p)
+{
+ struct ceph_fs_client *fsc = s->private;
+ struct ceph_mds_client *mdsc = fsc->mdsc;
+
+ seq_printf(s, "item total miss hit\n");
+ seq_printf(s, "-------------------------------------------------\n");
+
+ seq_printf(s, "%-14s%-16lld%-16lld%lld\n", "d_lease",
+ atomic64_read(&mdsc->metric.total_dentries),
+ percpu_counter_sum(&mdsc->metric.d_lease_mis),
+ percpu_counter_sum(&mdsc->metric.d_lease_hit));
+
+ return 0;
+}
+
static int caps_show_cb(struct inode *inode, struct ceph_cap *cap, void *p)
{
struct seq_file *s = p;
@@ -222,6 +238,7 @@ static int mds_sessions_show(struct seq_file *s, void *ptr)
DEFINE_SHOW_ATTRIBUTE(mdsc);
DEFINE_SHOW_ATTRIBUTE(caps);
DEFINE_SHOW_ATTRIBUTE(mds_sessions);
+DEFINE_SHOW_ATTRIBUTE(metric);
/*
@@ -255,6 +272,7 @@ void ceph_fs_debugfs_cleanup(struct ceph_fs_client *fsc)
debugfs_remove(fsc->debugfs_mdsmap);
debugfs_remove(fsc->debugfs_mds_sessions);
debugfs_remove(fsc->debugfs_caps);
+ debugfs_remove(fsc->debugfs_metric);
debugfs_remove(fsc->debugfs_mdsc);
}
@@ -295,11 +313,17 @@ void ceph_fs_debugfs_init(struct ceph_fs_client *fsc)
fsc,
&mdsc_fops);
+ fsc->debugfs_metric = debugfs_create_file("metrics",
+ 0400,
+ fsc->client->debugfs_dir,
+ fsc,
+ &metric_fops);
+
fsc->debugfs_caps = debugfs_create_file("caps",
- 0400,
- fsc->client->debugfs_dir,
- fsc,
- &caps_fops);
+ 0400,
+ fsc->client->debugfs_dir,
+ fsc,
+ &caps_fops);
}
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index d594c26..8097a86 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -38,6 +38,8 @@
static int ceph_d_init(struct dentry *dentry)
{
struct ceph_dentry_info *di;
+ struct ceph_fs_client *fsc = ceph_sb_to_client(dentry->d_sb);
+ struct ceph_mds_client *mdsc = fsc->mdsc;
di = kmem_cache_zalloc(ceph_dentry_cachep, GFP_KERNEL);
if (!di)
@@ -48,6 +50,9 @@ static int ceph_d_init(struct dentry *dentry)
di->time = jiffies;
dentry->d_fsdata = di;
INIT_LIST_HEAD(&di->lease_list);
+
+ atomic64_inc(&mdsc->metric.total_dentries);
+
return 0;
}
@@ -1709,6 +1714,8 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
if (flags & LOOKUP_RCU)
return -ECHILD;
+ percpu_counter_inc(&mdsc->metric.d_lease_mis);
+
op = ceph_snap(dir) == CEPH_SNAPDIR ?
CEPH_MDS_OP_LOOKUPSNAP : CEPH_MDS_OP_LOOKUP;
req = ceph_mdsc_create_request(mdsc, op, USE_ANY_MDS);
@@ -1740,6 +1747,8 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
dout("d_revalidate %p lookup result=%d\n",
dentry, err);
}
+ } else {
+ percpu_counter_inc(&mdsc->metric.d_lease_hit);
}
dout("d_revalidate %p %s\n", dentry, valid ? "valid" : "invalid");
@@ -1782,9 +1791,12 @@ static int ceph_d_delete(const struct dentry *dentry)
static void ceph_d_release(struct dentry *dentry)
{
struct ceph_dentry_info *di = ceph_dentry(dentry);
+ struct ceph_fs_client *fsc = ceph_sb_to_client(dentry->d_sb);
dout("d_release %p\n", dentry);
+ atomic64_dec(&fsc->mdsc->metric.total_dentries);
+
spin_lock(&dentry->d_lock);
__dentry_lease_unlist(di);
dentry->d_fsdata = NULL;
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 486f91f..0e2557b 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -4321,10 +4321,31 @@ static void delayed_work(struct work_struct *work)
schedule_delayed(mdsc);
}
+static int ceph_mdsc_metric_init(struct ceph_client_metric *metric)
+{
+ int ret;
+
+ if (!metric)
+ return -EINVAL;
+
+ atomic64_set(&metric->total_dentries, 0);
+ ret = percpu_counter_init(&metric->d_lease_hit, 0, GFP_KERNEL);
+ if (ret)
+ return ret;
+ ret = percpu_counter_init(&metric->d_lease_mis, 0, GFP_KERNEL);
+ if (ret) {
+ percpu_counter_destroy(&metric->d_lease_hit);
+ return ret;
+ }
+
+ return 0;
+}
+
int ceph_mdsc_init(struct ceph_fs_client *fsc)
{
struct ceph_mds_client *mdsc;
+ int err;
mdsc = kzalloc(sizeof(struct ceph_mds_client), GFP_NOFS);
if (!mdsc)
@@ -4333,8 +4354,8 @@ int ceph_mdsc_init(struct ceph_fs_client *fsc)
mutex_init(&mdsc->mutex);
mdsc->mdsmap = kzalloc(sizeof(*mdsc->mdsmap), GFP_NOFS);
if (!mdsc->mdsmap) {
- kfree(mdsc);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto err_mdsc;
}
fsc->mdsc = mdsc;
@@ -4373,6 +4394,9 @@ int ceph_mdsc_init(struct ceph_fs_client *fsc)
init_waitqueue_head(&mdsc->cap_flushing_wq);
INIT_WORK(&mdsc->cap_reclaim_work, ceph_cap_reclaim_work);
atomic_set(&mdsc->cap_reclaim_pending, 0);
+ err = ceph_mdsc_metric_init(&mdsc->metric);
+ if (err)
+ goto err_mdsmap;
spin_lock_init(&mdsc->dentry_list_lock);
INIT_LIST_HEAD(&mdsc->dentry_leases);
@@ -4391,6 +4415,12 @@ int ceph_mdsc_init(struct ceph_fs_client *fsc)
strscpy(mdsc->nodename, utsname()->nodename,
sizeof(mdsc->nodename));
return 0;
+
+err_mdsmap:
+ kfree(mdsc->mdsmap);
+err_mdsc:
+ kfree(mdsc);
+ return err;
}
/*
@@ -4648,6 +4678,9 @@ void ceph_mdsc_destroy(struct ceph_fs_client *fsc)
ceph_mdsc_stop(mdsc);
+ percpu_counter_destroy(&mdsc->metric.d_lease_mis);
+ percpu_counter_destroy(&mdsc->metric.d_lease_hit);
+
fsc->mdsc = NULL;
kfree(mdsc);
dout("mdsc_destroy %p done\n", mdsc);
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 4e5be79b..ae1d01c 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -16,6 +16,8 @@
#include <linux/ceph/mdsmap.h>
#include <linux/ceph/auth.h>
+#include "metric.h"
+
/* The first 8 bits are reserved for old ceph releases */
enum ceph_feature_type {
CEPHFS_FEATURE_MIMIC = 8,
@@ -454,6 +456,8 @@ struct ceph_mds_client {
struct list_head dentry_leases; /* fifo list */
struct list_head dentry_dir_leases; /* lru list */
+ struct ceph_client_metric metric;
+
spinlock_t snapid_map_lock;
struct rb_root snapid_map_tree;
struct list_head snapid_map_lru;
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
new file mode 100644
index 0000000..998fe2a
--- /dev/null
+++ b/fs/ceph/metric.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _FS_CEPH_MDS_METRIC_H
+#define _FS_CEPH_MDS_METRIC_H
+
+/* This is the global metrics */
+struct ceph_client_metric {
+ atomic64_t total_dentries;
+ struct percpu_counter d_lease_hit;
+ struct percpu_counter d_lease_mis;
+};
+#endif /* _FS_CEPH_MDS_METRIC_H */
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 60aac3a..5c73cf1 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -128,6 +128,7 @@ struct ceph_fs_client {
struct dentry *debugfs_congestion_kb;
struct dentry *debugfs_bdi;
struct dentry *debugfs_mdsc, *debugfs_mdsmap;
+ struct dentry *debugfs_metric;
struct dentry *debugfs_mds_sessions;
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v9 2/5] ceph: add caps perf metric for each session
2020-03-09 7:37 [PATCH v9 0/5] ceph: add perf metrics support xiubli
2020-03-09 7:37 ` [PATCH v9 1/5] ceph: add global dentry lease metric support xiubli
@ 2020-03-09 7:37 ` xiubli
2020-03-09 11:51 ` Jeff Layton
2020-03-09 7:37 ` [PATCH v9 3/5] ceph: add global read latency metric support xiubli
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: xiubli @ 2020-03-09 7:37 UTC (permalink / raw)
To: jlayton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel, Xiubo Li
From: Xiubo Li <xiubli@redhat.com>
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@redhat.com>
---
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);
+ 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;
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 15975ba..c83e52b 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -128,6 +128,7 @@ static int metric_show(struct seq_file *s, void *p)
{
struct ceph_fs_client *fsc = s->private;
struct ceph_mds_client *mdsc = fsc->mdsc;
+ int i, nr_caps = 0;
seq_printf(s, "item total miss hit\n");
seq_printf(s, "-------------------------------------------------\n");
@@ -137,6 +138,21 @@ static int metric_show(struct seq_file *s, void *p)
percpu_counter_sum(&mdsc->metric.d_lease_mis),
percpu_counter_sum(&mdsc->metric.d_lease_hit));
+ mutex_lock(&mdsc->mutex);
+ for (i = 0; i < mdsc->max_sessions; i++) {
+ struct ceph_mds_session *s;
+
+ s = __ceph_lookup_mds_session(mdsc, i);
+ if (!s)
+ continue;
+ nr_caps += s->s_nr_caps;
+ ceph_put_mds_session(s);
+ }
+ mutex_unlock(&mdsc->mutex);
+ seq_printf(s, "%-14s%-16d%-16lld%lld\n", "caps", nr_caps,
+ percpu_counter_sum(&mdsc->metric.i_caps_mis),
+ percpu_counter_sum(&mdsc->metric.i_caps_hit));
+
return 0;
}
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 8097a86..10d528a 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -349,8 +349,9 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
!ceph_test_mount_opt(fsc, NOASYNCREADDIR) &&
ceph_snap(inode) != CEPH_SNAPDIR &&
__ceph_dir_is_complete_ordered(ci) &&
- __ceph_caps_issued_mask(ci, CEPH_CAP_FILE_SHARED, 1)) {
+ __ceph_caps_issued_mask_metric(ci, CEPH_CAP_FILE_SHARED, 1)) {
int shared_gen = atomic_read(&ci->i_shared_gen);
+
spin_unlock(&ci->i_ceph_lock);
err = __dcache_readdir(file, ctx, shared_gen);
if (err != -EAGAIN)
@@ -767,7 +768,7 @@ static struct dentry *ceph_lookup(struct inode *dir, struct dentry *dentry,
!is_root_ceph_dentry(dir, dentry) &&
ceph_test_mount_opt(fsc, DCACHE) &&
__ceph_dir_is_complete(ci) &&
- (__ceph_caps_issued_mask(ci, CEPH_CAP_FILE_SHARED, 1))) {
+ __ceph_caps_issued_mask_metric(ci, CEPH_CAP_FILE_SHARED, 1)) {
__ceph_touch_fmode(ci, mdsc, CEPH_FILE_MODE_RD);
spin_unlock(&ci->i_ceph_lock);
dout(" dir %p complete, -ENOENT\n", dir);
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index ee40ba7..b5a30ca6 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2284,8 +2284,8 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,
dout("do_getattr inode %p mask %s mode 0%o\n",
inode, ceph_cap_string(mask), inode->i_mode);
- if (!force && ceph_caps_issued_mask(ceph_inode(inode), mask, 1))
- return 0;
+ if (!force && ceph_caps_issued_mask_metric(ceph_inode(inode), mask, 1))
+ return 0;
mode = (mask & CEPH_STAT_RSTAT) ? USE_AUTH_MDS : USE_ANY_MDS;
req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETATTR, mode);
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 0e2557b..ba54fd2 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -4332,13 +4332,29 @@ static int ceph_mdsc_metric_init(struct ceph_client_metric *metric)
ret = percpu_counter_init(&metric->d_lease_hit, 0, GFP_KERNEL);
if (ret)
return ret;
+
ret = percpu_counter_init(&metric->d_lease_mis, 0, GFP_KERNEL);
- if (ret) {
- percpu_counter_destroy(&metric->d_lease_hit);
- return ret;
- }
+ if (ret)
+ goto err_d_lease_mis;
+
+ ret = percpu_counter_init(&metric->i_caps_hit, 0, GFP_KERNEL);
+ if (ret)
+ goto err_i_caps_hit;
+
+ ret = percpu_counter_init(&metric->i_caps_mis, 0, GFP_KERNEL);
+ if (ret)
+ goto err_i_caps_mis;
return 0;
+
+err_i_caps_mis:
+ percpu_counter_destroy(&metric->i_caps_hit);
+err_i_caps_hit:
+ percpu_counter_destroy(&metric->d_lease_mis);
+err_d_lease_mis:
+ percpu_counter_destroy(&metric->d_lease_hit);
+
+ return ret;
}
int ceph_mdsc_init(struct ceph_fs_client *fsc)
@@ -4678,6 +4694,8 @@ void ceph_mdsc_destroy(struct ceph_fs_client *fsc)
ceph_mdsc_stop(mdsc);
+ percpu_counter_destroy(&mdsc->metric.i_caps_mis);
+ percpu_counter_destroy(&mdsc->metric.i_caps_hit);
percpu_counter_destroy(&mdsc->metric.d_lease_mis);
percpu_counter_destroy(&mdsc->metric.d_lease_hit);
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index 998fe2a..f620f72 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -7,5 +7,18 @@ struct ceph_client_metric {
atomic64_t total_dentries;
struct percpu_counter d_lease_hit;
struct percpu_counter d_lease_mis;
+
+ struct percpu_counter i_caps_hit;
+ struct percpu_counter i_caps_mis;
};
+
+static inline void ceph_update_cap_hit(struct ceph_client_metric *m)
+{
+ percpu_counter_inc(&m->i_caps_hit);
+}
+
+static inline void ceph_update_cap_mis(struct ceph_client_metric *m)
+{
+ percpu_counter_inc(&m->i_caps_mis);
+}
#endif /* _FS_CEPH_MDS_METRIC_H */
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 5c73cf1..47cfd89 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -645,6 +645,8 @@ static inline bool __ceph_is_any_real_caps(struct ceph_inode_info *ci)
extern int __ceph_caps_issued(struct ceph_inode_info *ci, int *implemented);
extern int __ceph_caps_issued_mask(struct ceph_inode_info *ci, int mask, int t);
+extern int __ceph_caps_issued_mask_metric(struct ceph_inode_info *ci, int mask,
+ int t);
extern int __ceph_caps_issued_other(struct ceph_inode_info *ci,
struct ceph_cap *cap);
@@ -657,12 +659,12 @@ static inline int ceph_caps_issued(struct ceph_inode_info *ci)
return issued;
}
-static inline int ceph_caps_issued_mask(struct ceph_inode_info *ci, int mask,
- int touch)
+static inline int ceph_caps_issued_mask_metric(struct ceph_inode_info *ci,
+ int mask, int touch)
{
int r;
spin_lock(&ci->i_ceph_lock);
- r = __ceph_caps_issued_mask(ci, mask, touch);
+ r = __ceph_caps_issued_mask_metric(ci, mask, touch);
spin_unlock(&ci->i_ceph_lock);
return r;
}
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 7b8a070..71ee34d 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -856,7 +856,7 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
if (ci->i_xattrs.version == 0 ||
!((req_mask & CEPH_CAP_XATTR_SHARED) ||
- __ceph_caps_issued_mask(ci, CEPH_CAP_XATTR_SHARED, 1))) {
+ __ceph_caps_issued_mask_metric(ci, CEPH_CAP_XATTR_SHARED, 1))) {
spin_unlock(&ci->i_ceph_lock);
/* security module gets xattr while filling trace */
@@ -914,7 +914,7 @@ ssize_t ceph_listxattr(struct dentry *dentry, char *names, size_t size)
ci->i_xattrs.version, ci->i_xattrs.index_version);
if (ci->i_xattrs.version == 0 ||
- !__ceph_caps_issued_mask(ci, CEPH_CAP_XATTR_SHARED, 1)) {
+ !__ceph_caps_issued_mask_metric(ci, CEPH_CAP_XATTR_SHARED, 1)) {
spin_unlock(&ci->i_ceph_lock);
err = ceph_do_getattr(inode, CEPH_STAT_CAP_XATTR, true);
if (err)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v9 2/5] ceph: add caps perf metric for each session
2020-03-09 7:37 ` [PATCH v9 2/5] ceph: add caps perf metric for each session xiubli
@ 2020-03-09 11:51 ` Jeff Layton
2020-03-09 12:05 ` Jeff Layton
2020-03-09 12:26 ` Xiubo Li
0 siblings, 2 replies; 14+ messages in thread
From: Jeff Layton @ 2020-03-09 11:51 UTC (permalink / raw)
To: xiubli, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel
On Mon, 2020-03-09 at 03:37 -0400, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
>
> 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@redhat.com>
> ---
> 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?
> + 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;
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v9 2/5] ceph: add caps perf metric for each session
2020-03-09 11:51 ` Jeff Layton
@ 2020-03-09 12:05 ` Jeff Layton
2020-03-09 12:36 ` Xiubo Li
2020-03-09 12:26 ` Xiubo Li
1 sibling, 1 reply; 14+ messages in thread
From: Jeff Layton @ 2020-03-09 12:05 UTC (permalink / raw)
To: xiubli, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel
On Mon, 2020-03-09 at 07:51 -0400, Jeff Layton wrote:
> On Mon, 2020-03-09 at 03:37 -0400, xiubli@redhat.com wrote:
> > From: Xiubo Li <xiubli@redhat.com>
> >
> > 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@redhat.com>
> > ---
> > 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?
>
> > + 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;
Here's what I'd propose on top. If this looks ok, then I can just fold
this patch into yours before merging. No need to resend just for this.
----------------8<----------------
[PATCH] SQUASH: count negative error code as miss in try_get_cap_refs
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/ceph/caps.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index efaeb67d584c..3be928782b45 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -2694,9 +2694,9 @@ static int try_get_cap_refs(struct inode *inode,
int need, int want,
if (snap_rwsem_locked)
up_read(&mdsc->snap_rwsem);
- if (!ret)
+ if (ret <= 0)
ceph_update_cap_mis(&mdsc->metric);
- else if (ret == 1)
+ else
ceph_update_cap_hit(&mdsc->metric);
dout("get_cap_refs %p ret %d got %s\n", inode,
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v9 2/5] ceph: add caps perf metric for each session
2020-03-09 12:05 ` Jeff Layton
@ 2020-03-09 12:36 ` Xiubo Li
2020-03-09 18:22 ` Jeff Layton
0 siblings, 1 reply; 14+ messages in thread
From: Xiubo Li @ 2020-03-09 12:36 UTC (permalink / raw)
To: Jeff Layton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel
On 2020/3/9 20:05, Jeff Layton wrote:
> On Mon, 2020-03-09 at 07:51 -0400, Jeff Layton wrote:
>> On Mon, 2020-03-09 at 03:37 -0400, xiubli@redhat.com wrote:
>>> From: Xiubo Li <xiubli@redhat.com>
>>>
>>> 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@redhat.com>
>>> ---
>>> 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?
>>
>>> + 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;
> Here's what I'd propose on top. If this looks ok, then I can just fold
> this patch into yours before merging. No need to resend just for this.
>
> ----------------8<----------------
>
> [PATCH] SQUASH: count negative error code as miss in try_get_cap_refs
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/ceph/caps.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index efaeb67d584c..3be928782b45 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2694,9 +2694,9 @@ static int try_get_cap_refs(struct inode *inode,
> int need, int want,
> if (snap_rwsem_locked)
> up_read(&mdsc->snap_rwsem);
>
> - if (!ret)
> + if (ret <= 0)
> ceph_update_cap_mis(&mdsc->metric);
> - else if (ret == 1)
> + else
> ceph_update_cap_hit(&mdsc->metric);
>
> dout("get_cap_refs %p ret %d got %s\n", inode,
For the try_get_cap_refs(), maybe this is the correct approach to count
hit/miss as the function comments.
Thanks
Brs
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v9 2/5] ceph: add caps perf metric for each session
2020-03-09 12:36 ` Xiubo Li
@ 2020-03-09 18:22 ` Jeff Layton
2020-03-10 0:25 ` Xiubo Li
0 siblings, 1 reply; 14+ messages in thread
From: Jeff Layton @ 2020-03-09 18:22 UTC (permalink / raw)
To: Xiubo Li, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel
On Mon, 2020-03-09 at 20:36 +0800, Xiubo Li wrote:
> On 2020/3/9 20:05, Jeff Layton wrote:
> > On Mon, 2020-03-09 at 07:51 -0400, Jeff Layton wrote:
> > > On Mon, 2020-03-09 at 03:37 -0400, xiubli@redhat.com wrote:
> > > > From: Xiubo Li <xiubli@redhat.com>
> > > >
> > > > 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@redhat.com>
> > > > ---
> > > > 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?
> > >
> > > > + 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;
> > Here's what I'd propose on top. If this looks ok, then I can just fold
> > this patch into yours before merging. No need to resend just for this.
> >
> > ----------------8<----------------
> >
> > [PATCH] SQUASH: count negative error code as miss in try_get_cap_refs
> >
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > fs/ceph/caps.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > index efaeb67d584c..3be928782b45 100644
> > --- a/fs/ceph/caps.c
> > +++ b/fs/ceph/caps.c
> > @@ -2694,9 +2694,9 @@ static int try_get_cap_refs(struct inode *inode,
> > int need, int want,
> > if (snap_rwsem_locked)
> > up_read(&mdsc->snap_rwsem);
> >
> > - if (!ret)
> > + if (ret <= 0)
> > ceph_update_cap_mis(&mdsc->metric);
> > - else if (ret == 1)
> > + else
> > ceph_update_cap_hit(&mdsc->metric);
> >
> > dout("get_cap_refs %p ret %d got %s\n", inode,
>
> For the try_get_cap_refs(), maybe this is the correct approach to count
> hit/miss as the function comments.
>
I decided to just merge your patches as-is. Given that those are error
conditions, and some of them may occur before we ever check the caps, I
think we should just opt to not count those cases.
I did clean up the changelogs a bit, so please have a look and let me
know if they look ok to you.
Thanks!
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v9 2/5] ceph: add caps perf metric for each session
2020-03-09 18:22 ` Jeff Layton
@ 2020-03-10 0:25 ` Xiubo Li
0 siblings, 0 replies; 14+ messages in thread
From: Xiubo Li @ 2020-03-10 0:25 UTC (permalink / raw)
To: Jeff Layton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel
On 2020/3/10 2:22, Jeff Layton wrote:
> On Mon, 2020-03-09 at 20:36 +0800, Xiubo Li wrote:
>> On 2020/3/9 20:05, Jeff Layton wrote:
>>> On Mon, 2020-03-09 at 07:51 -0400, Jeff Layton wrote:
>>>> On Mon, 2020-03-09 at 03:37 -0400, xiubli@redhat.com wrote:
>>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>>
>>>>> 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@redhat.com>
>>>>> ---
>>>>> 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?
>>>>
>>>>> + 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;
>>> Here's what I'd propose on top. If this looks ok, then I can just fold
>>> this patch into yours before merging. No need to resend just for this.
>>>
>>> ----------------8<----------------
>>>
>>> [PATCH] SQUASH: count negative error code as miss in try_get_cap_refs
>>>
>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>> ---
>>> fs/ceph/caps.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
>>> index efaeb67d584c..3be928782b45 100644
>>> --- a/fs/ceph/caps.c
>>> +++ b/fs/ceph/caps.c
>>> @@ -2694,9 +2694,9 @@ static int try_get_cap_refs(struct inode *inode,
>>> int need, int want,
>>> if (snap_rwsem_locked)
>>> up_read(&mdsc->snap_rwsem);
>>>
>>> - if (!ret)
>>> + if (ret <= 0)
>>> ceph_update_cap_mis(&mdsc->metric);
>>> - else if (ret == 1)
>>> + else
>>> ceph_update_cap_hit(&mdsc->metric);
>>>
>>> dout("get_cap_refs %p ret %d got %s\n", inode,
>> For the try_get_cap_refs(), maybe this is the correct approach to count
>> hit/miss as the function comments.
>>
> I decided to just merge your patches as-is. Given that those are error
> conditions, and some of them may occur before we ever check the caps, I
> think we should just opt to not count those cases.
>
> I did clean up the changelogs a bit, so please have a look and let me
> know if they look ok to you.
Cool, it looks nice to me.
Thanks Jeff.
BRs
> Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v9 2/5] ceph: add caps perf metric for each session
2020-03-09 11:51 ` Jeff Layton
2020-03-09 12:05 ` Jeff Layton
@ 2020-03-09 12:26 ` Xiubo Li
1 sibling, 0 replies; 14+ messages in thread
From: Xiubo Li @ 2020-03-09 12:26 UTC (permalink / raw)
To: Jeff Layton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel
On 2020/3/9 19:51, Jeff Layton wrote:
> On Mon, 2020-03-09 at 03:37 -0400, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> 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@redhat.com>
>> ---
>> 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;
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v9 3/5] ceph: add global read latency metric support
2020-03-09 7:37 [PATCH v9 0/5] ceph: add perf metrics support xiubli
2020-03-09 7:37 ` [PATCH v9 1/5] ceph: add global dentry lease metric support xiubli
2020-03-09 7:37 ` [PATCH v9 2/5] ceph: add caps perf metric for each session xiubli
@ 2020-03-09 7:37 ` xiubli
2020-03-09 7:37 ` [PATCH v9 4/5] ceph: add global write " xiubli
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: xiubli @ 2020-03-09 7:37 UTC (permalink / raw)
To: jlayton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel, Xiubo Li
From: Xiubo Li <xiubli@redhat.com>
It will calculate the latency for the read osd requests:
item total sum_lat(us) avg_lat(us)
-----------------------------------------------------
read 1036 848000 818
URL: https://tracker.ceph.com/issues/43215
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
fs/ceph/addr.c | 8 ++++++++
fs/ceph/debugfs.c | 11 +++++++++++
fs/ceph/file.c | 16 ++++++++++++++++
fs/ceph/mds_client.c | 14 ++++++++++++++
fs/ceph/metric.h | 15 +++++++++++++++
include/linux/ceph/osd_client.h | 1 +
net/ceph/osd_client.c | 2 ++
7 files changed, 67 insertions(+)
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 6f4678d..55008a3 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -216,6 +216,9 @@ static int ceph_sync_readpages(struct ceph_fs_client *fsc,
if (!rc)
rc = ceph_osdc_wait_request(osdc, req);
+ ceph_update_read_latency(&fsc->mdsc->metric, req->r_start_stamp,
+ req->r_end_stamp, rc);
+
ceph_osdc_put_request(req);
dout("readpages result %d\n", rc);
return rc;
@@ -299,6 +302,7 @@ static int ceph_readpage(struct file *filp, struct page *page)
static void finish_read(struct ceph_osd_request *req)
{
struct inode *inode = req->r_inode;
+ struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
struct ceph_osd_data *osd_data;
int rc = req->r_result <= 0 ? req->r_result : 0;
int bytes = req->r_result >= 0 ? req->r_result : 0;
@@ -336,6 +340,10 @@ static void finish_read(struct ceph_osd_request *req)
put_page(page);
bytes -= PAGE_SIZE;
}
+
+ ceph_update_read_latency(&fsc->mdsc->metric, req->r_start_stamp,
+ req->r_end_stamp, rc);
+
kfree(osd_data->pages);
}
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index c83e52b..d814a3a 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -129,7 +129,18 @@ static int metric_show(struct seq_file *s, void *p)
struct ceph_fs_client *fsc = s->private;
struct ceph_mds_client *mdsc = fsc->mdsc;
int i, nr_caps = 0;
+ s64 total, sum, avg = 0;
+ seq_printf(s, "item total sum_lat(us) avg_lat(us)\n");
+ seq_printf(s, "-----------------------------------------------------\n");
+
+ total = percpu_counter_sum(&mdsc->metric.total_reads);
+ sum = percpu_counter_sum(&mdsc->metric.read_latency_sum);
+ sum = jiffies_to_usecs(sum);
+ avg = total ? sum / total : 0;
+ seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "read", total, sum, avg);
+
+ seq_printf(s, "\n");
seq_printf(s, "item total miss hit\n");
seq_printf(s, "-------------------------------------------------\n");
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index ba46ba74..3dce2a0 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -906,6 +906,10 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
ret = ceph_osdc_start_request(osdc, req, false);
if (!ret)
ret = ceph_osdc_wait_request(osdc, req);
+
+ ceph_update_read_latency(&fsc->mdsc->metric, req->r_start_stamp,
+ req->r_end_stamp, ret);
+
ceph_osdc_put_request(req);
i_size = i_size_read(inode);
@@ -1044,6 +1048,8 @@ static void ceph_aio_complete_req(struct ceph_osd_request *req)
struct inode *inode = req->r_inode;
struct ceph_aio_request *aio_req = req->r_priv;
struct ceph_osd_data *osd_data = osd_req_op_extent_osd_data(req, 0);
+ struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
+ struct ceph_client_metric *metric = &fsc->mdsc->metric;
BUG_ON(osd_data->type != CEPH_OSD_DATA_TYPE_BVECS);
BUG_ON(!osd_data->num_bvecs);
@@ -1051,6 +1057,11 @@ static void ceph_aio_complete_req(struct ceph_osd_request *req)
dout("ceph_aio_complete_req %p rc %d bytes %u\n",
inode, rc, osd_data->bvec_pos.iter.bi_size);
+ /* r_start_stamp == 0 means the request was not submitted */
+ if (req->r_start_stamp && !aio_req->write)
+ ceph_update_read_latency(metric, req->r_start_stamp,
+ req->r_end_stamp, rc);
+
if (rc == -EOLDSNAPC) {
struct ceph_aio_work *aio_work;
BUG_ON(!aio_req->write);
@@ -1179,6 +1190,7 @@ static void ceph_aio_retry_work(struct work_struct *work)
struct inode *inode = file_inode(file);
struct ceph_inode_info *ci = ceph_inode(inode);
struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
+ struct ceph_client_metric *metric = &fsc->mdsc->metric;
struct ceph_vino vino;
struct ceph_osd_request *req;
struct bio_vec *bvecs;
@@ -1295,6 +1307,10 @@ static void ceph_aio_retry_work(struct work_struct *work)
if (!ret)
ret = ceph_osdc_wait_request(&fsc->client->osdc, req);
+ if (!write)
+ ceph_update_read_latency(metric, req->r_start_stamp,
+ req->r_end_stamp, ret);
+
size = i_size_read(inode);
if (!write) {
if (ret == -ENOENT)
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index ba54fd2..94f6e53 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -4345,8 +4345,20 @@ static int ceph_mdsc_metric_init(struct ceph_client_metric *metric)
if (ret)
goto err_i_caps_mis;
+ ret = percpu_counter_init(&metric->total_reads, 0, GFP_KERNEL);
+ if (ret)
+ goto err_total_reads;
+
+ ret = percpu_counter_init(&metric->read_latency_sum, 0, GFP_KERNEL);
+ if (ret)
+ goto err_read_latency_sum;
+
return 0;
+err_read_latency_sum:
+ percpu_counter_destroy(&metric->total_reads);
+err_total_reads:
+ percpu_counter_destroy(&metric->i_caps_mis);
err_i_caps_mis:
percpu_counter_destroy(&metric->i_caps_hit);
err_i_caps_hit:
@@ -4694,6 +4706,8 @@ void ceph_mdsc_destroy(struct ceph_fs_client *fsc)
ceph_mdsc_stop(mdsc);
+ percpu_counter_destroy(&mdsc->metric.read_latency_sum);
+ percpu_counter_destroy(&mdsc->metric.total_reads);
percpu_counter_destroy(&mdsc->metric.i_caps_mis);
percpu_counter_destroy(&mdsc->metric.i_caps_hit);
percpu_counter_destroy(&mdsc->metric.d_lease_mis);
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index f620f72..0fe3eee 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -10,6 +10,9 @@ struct ceph_client_metric {
struct percpu_counter i_caps_hit;
struct percpu_counter i_caps_mis;
+
+ struct percpu_counter total_reads;
+ struct percpu_counter read_latency_sum;
};
static inline void ceph_update_cap_hit(struct ceph_client_metric *m)
@@ -21,4 +24,16 @@ static inline void ceph_update_cap_mis(struct ceph_client_metric *m)
{
percpu_counter_inc(&m->i_caps_mis);
}
+
+static inline void ceph_update_read_latency(struct ceph_client_metric *m,
+ unsigned long r_start,
+ unsigned long r_end,
+ int rc)
+{
+ if (rc < 0 && rc != -ENOENT && rc != -ETIMEDOUT)
+ return;
+
+ percpu_counter_inc(&m->total_reads);
+ percpu_counter_add(&m->read_latency_sum, r_end - r_start);
+}
#endif /* _FS_CEPH_MDS_METRIC_H */
diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
index 9d9f745..02ff3a3 100644
--- a/include/linux/ceph/osd_client.h
+++ b/include/linux/ceph/osd_client.h
@@ -213,6 +213,7 @@ struct ceph_osd_request {
/* internal */
unsigned long r_stamp; /* jiffies, send or check time */
unsigned long r_start_stamp; /* jiffies */
+ unsigned long r_end_stamp; /* jiffies */
int r_attempts;
u32 r_map_dne_bound;
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index 51810db..4106db6 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -2389,6 +2389,8 @@ static void finish_request(struct ceph_osd_request *req)
WARN_ON(lookup_request_mc(&osdc->map_checks, req->r_tid));
dout("%s req %p tid %llu\n", __func__, req, req->r_tid);
+ req->r_end_stamp = jiffies;
+
if (req->r_osd)
unlink_request(req->r_osd, req);
atomic_dec(&osdc->num_requests);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v9 4/5] ceph: add global write latency metric support
2020-03-09 7:37 [PATCH v9 0/5] ceph: add perf metrics support xiubli
` (2 preceding siblings ...)
2020-03-09 7:37 ` [PATCH v9 3/5] ceph: add global read latency metric support xiubli
@ 2020-03-09 7:37 ` xiubli
2020-03-09 7:37 ` [PATCH v9 5/5] ceph: add global metadata perf " xiubli
2020-03-09 12:09 ` [PATCH v9 0/5] ceph: add perf metrics support Jeff Layton
5 siblings, 0 replies; 14+ messages in thread
From: xiubli @ 2020-03-09 7:37 UTC (permalink / raw)
To: jlayton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel, Xiubo Li
From: Xiubo Li <xiubli@redhat.com>
It will calculate the latency for the write osd requests:
item total sum_lat(us) avg_lat(us)
-----------------------------------------------------
write 1048 8778000 8375
URL: https://tracker.ceph.com/issues/43215
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
fs/ceph/addr.c | 10 ++++++++++
fs/ceph/debugfs.c | 6 ++++++
fs/ceph/file.c | 18 ++++++++++++++----
fs/ceph/mds_client.c | 14 ++++++++++++++
fs/ceph/metric.h | 15 +++++++++++++++
5 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 55008a3..f359619 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -651,6 +651,9 @@ static int ceph_sync_writepages(struct ceph_fs_client *fsc,
if (!rc)
rc = ceph_osdc_wait_request(osdc, req);
+ ceph_update_write_latency(&fsc->mdsc->metric, req->r_start_stamp,
+ req->r_end_stamp, rc);
+
ceph_osdc_put_request(req);
if (rc == 0)
rc = len;
@@ -802,6 +805,9 @@ static void writepages_finish(struct ceph_osd_request *req)
ceph_clear_error_write(ci);
}
+ ceph_update_write_latency(&fsc->mdsc->metric, req->r_start_stamp,
+ req->r_end_stamp, rc);
+
/*
* We lost the cache cap, need to truncate the page before
* it is unlocked, otherwise we'd truncate it later in the
@@ -1860,6 +1866,10 @@ int ceph_uninline_data(struct file *filp, struct page *locked_page)
err = ceph_osdc_start_request(&fsc->client->osdc, req, false);
if (!err)
err = ceph_osdc_wait_request(&fsc->client->osdc, req);
+
+ ceph_update_write_latency(&fsc->mdsc->metric, req->r_start_stamp,
+ req->r_end_stamp, err);
+
out_put:
ceph_osdc_put_request(req);
if (err == -ECANCELED)
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index d814a3a..464bfbd 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -140,6 +140,12 @@ static int metric_show(struct seq_file *s, void *p)
avg = total ? sum / total : 0;
seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "read", total, sum, avg);
+ total = percpu_counter_sum(&mdsc->metric.total_writes);
+ sum = percpu_counter_sum(&mdsc->metric.write_latency_sum);
+ sum = jiffies_to_usecs(sum);
+ avg = total ? sum / total : 0;
+ seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "write", total, sum, avg);
+
seq_printf(s, "\n");
seq_printf(s, "item total miss hit\n");
seq_printf(s, "-------------------------------------------------\n");
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 3dce2a0..aa08fdf 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1058,9 +1058,14 @@ static void ceph_aio_complete_req(struct ceph_osd_request *req)
inode, rc, osd_data->bvec_pos.iter.bi_size);
/* r_start_stamp == 0 means the request was not submitted */
- if (req->r_start_stamp && !aio_req->write)
- ceph_update_read_latency(metric, req->r_start_stamp,
- req->r_end_stamp, rc);
+ if (req->r_start_stamp) {
+ if (aio_req->write)
+ ceph_update_write_latency(metric, req->r_start_stamp,
+ req->r_end_stamp, rc);
+ else
+ ceph_update_read_latency(metric, req->r_start_stamp,
+ req->r_end_stamp, rc);
+ }
if (rc == -EOLDSNAPC) {
struct ceph_aio_work *aio_work;
@@ -1307,7 +1312,10 @@ static void ceph_aio_retry_work(struct work_struct *work)
if (!ret)
ret = ceph_osdc_wait_request(&fsc->client->osdc, req);
- if (!write)
+ if (write)
+ ceph_update_write_latency(metric, req->r_start_stamp,
+ req->r_end_stamp, ret);
+ else
ceph_update_read_latency(metric, req->r_start_stamp,
req->r_end_stamp, ret);
@@ -1482,6 +1490,8 @@ static void ceph_aio_retry_work(struct work_struct *work)
if (!ret)
ret = ceph_osdc_wait_request(&fsc->client->osdc, req);
+ ceph_update_write_latency(&fsc->mdsc->metric, req->r_start_stamp,
+ req->r_end_stamp, ret);
out:
ceph_osdc_put_request(req);
if (ret != 0) {
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 94f6e53..43f5d59 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -4353,8 +4353,20 @@ static int ceph_mdsc_metric_init(struct ceph_client_metric *metric)
if (ret)
goto err_read_latency_sum;
+ ret = percpu_counter_init(&metric->total_writes, 0, GFP_KERNEL);
+ if (ret)
+ goto err_total_writes;
+
+ ret = percpu_counter_init(&metric->write_latency_sum, 0, GFP_KERNEL);
+ if (ret)
+ goto err_write_latency_sum;
+
return 0;
+err_write_latency_sum:
+ percpu_counter_destroy(&metric->total_writes);
+err_total_writes:
+ percpu_counter_destroy(&metric->read_latency_sum);
err_read_latency_sum:
percpu_counter_destroy(&metric->total_reads);
err_total_reads:
@@ -4706,6 +4718,8 @@ void ceph_mdsc_destroy(struct ceph_fs_client *fsc)
ceph_mdsc_stop(mdsc);
+ percpu_counter_destroy(&mdsc->metric.write_latency_sum);
+ percpu_counter_destroy(&mdsc->metric.total_writes);
percpu_counter_destroy(&mdsc->metric.read_latency_sum);
percpu_counter_destroy(&mdsc->metric.total_reads);
percpu_counter_destroy(&mdsc->metric.i_caps_mis);
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index 0fe3eee..13698aa 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -13,6 +13,9 @@ struct ceph_client_metric {
struct percpu_counter total_reads;
struct percpu_counter read_latency_sum;
+
+ struct percpu_counter total_writes;
+ struct percpu_counter write_latency_sum;
};
static inline void ceph_update_cap_hit(struct ceph_client_metric *m)
@@ -36,4 +39,16 @@ static inline void ceph_update_read_latency(struct ceph_client_metric *m,
percpu_counter_inc(&m->total_reads);
percpu_counter_add(&m->read_latency_sum, r_end - r_start);
}
+
+static inline void ceph_update_write_latency(struct ceph_client_metric *m,
+ unsigned long r_start,
+ unsigned long r_end,
+ int rc)
+{
+ if (rc && rc != -ETIMEDOUT)
+ return;
+
+ percpu_counter_inc(&m->total_writes);
+ percpu_counter_add(&m->write_latency_sum, r_end - r_start);
+}
#endif /* _FS_CEPH_MDS_METRIC_H */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v9 5/5] ceph: add global metadata perf metric support
2020-03-09 7:37 [PATCH v9 0/5] ceph: add perf metrics support xiubli
` (3 preceding siblings ...)
2020-03-09 7:37 ` [PATCH v9 4/5] ceph: add global write " xiubli
@ 2020-03-09 7:37 ` xiubli
2020-03-09 12:09 ` [PATCH v9 0/5] ceph: add perf metrics support Jeff Layton
5 siblings, 0 replies; 14+ messages in thread
From: xiubli @ 2020-03-09 7:37 UTC (permalink / raw)
To: jlayton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel, Xiubo Li
From: Xiubo Li <xiubli@redhat.com>
It will calculate the latency for the metedata requests:
item total sum_lat(us) avg_lat(us)
-----------------------------------------------------
metadata 113 220000 1946
URL: https://tracker.ceph.com/issues/43215
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
fs/ceph/debugfs.c | 6 ++++++
fs/ceph/mds_client.c | 19 +++++++++++++++++++
fs/ceph/mds_client.h | 3 ++-
fs/ceph/metric.h | 15 +++++++++++++++
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 464bfbd..60f3e307 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -146,6 +146,12 @@ static int metric_show(struct seq_file *s, void *p)
avg = total ? sum / total : 0;
seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "write", total, sum, avg);
+ total = percpu_counter_sum(&mdsc->metric.total_metadatas);
+ sum = percpu_counter_sum(&mdsc->metric.metadata_latency_sum);
+ sum = jiffies_to_usecs(sum);
+ avg = total ? sum / total : 0;
+ seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "metadata", total, sum, avg);
+
seq_printf(s, "\n");
seq_printf(s, "item total miss hit\n");
seq_printf(s, "-------------------------------------------------\n");
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 43f5d59..5c03ed3 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2547,6 +2547,8 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
static void complete_request(struct ceph_mds_client *mdsc,
struct ceph_mds_request *req)
{
+ req->r_ended = jiffies;
+
if (req->r_callback)
req->r_callback(mdsc, req);
complete_all(&req->r_completion);
@@ -3155,6 +3157,9 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
/* kick calling process */
complete_request(mdsc, req);
+
+ ceph_update_metadata_latency(&mdsc->metric, req->r_started,
+ req->r_ended, err);
out:
ceph_mdsc_put_request(req);
return;
@@ -4361,8 +4366,20 @@ static int ceph_mdsc_metric_init(struct ceph_client_metric *metric)
if (ret)
goto err_write_latency_sum;
+ ret = percpu_counter_init(&metric->total_metadatas, 0, GFP_KERNEL);
+ if (ret)
+ goto err_total_metadatas;
+
+ ret = percpu_counter_init(&metric->metadata_latency_sum, 0, GFP_KERNEL);
+ if (ret)
+ goto err_metadata_latency_sum;
+
return 0;
+err_metadata_latency_sum:
+ percpu_counter_destroy(&metric->total_metadatas);
+err_total_metadatas:
+ percpu_counter_destroy(&metric->write_latency_sum);
err_write_latency_sum:
percpu_counter_destroy(&metric->total_writes);
err_total_writes:
@@ -4718,6 +4735,8 @@ void ceph_mdsc_destroy(struct ceph_fs_client *fsc)
ceph_mdsc_stop(mdsc);
+ percpu_counter_destroy(&mdsc->metric.metadata_latency_sum);
+ percpu_counter_destroy(&mdsc->metric.total_metadatas);
percpu_counter_destroy(&mdsc->metric.write_latency_sum);
percpu_counter_destroy(&mdsc->metric.total_writes);
percpu_counter_destroy(&mdsc->metric.read_latency_sum);
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index ae1d01c..9018fa7 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -298,7 +298,8 @@ struct ceph_mds_request {
u32 r_readdir_offset;
unsigned long r_timeout; /* optional. jiffies, 0 is "wait forever" */
- unsigned long r_started; /* start time to measure timeout against */
+ unsigned long r_started; /* start time to measure timeout against and latency */
+ unsigned long r_ended; /* finish time to measure latency */
unsigned long r_request_started; /* start time for mds request only,
used to measure lease durations */
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index 13698aa..faba142 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -16,6 +16,9 @@ struct ceph_client_metric {
struct percpu_counter total_writes;
struct percpu_counter write_latency_sum;
+
+ struct percpu_counter total_metadatas;
+ struct percpu_counter metadata_latency_sum;
};
static inline void ceph_update_cap_hit(struct ceph_client_metric *m)
@@ -51,4 +54,16 @@ static inline void ceph_update_write_latency(struct ceph_client_metric *m,
percpu_counter_inc(&m->total_writes);
percpu_counter_add(&m->write_latency_sum, r_end - r_start);
}
+
+static inline void ceph_update_metadata_latency(struct ceph_client_metric *m,
+ unsigned long r_start,
+ unsigned long r_end,
+ int rc)
+{
+ if (rc && rc != -ENOENT)
+ return;
+
+ percpu_counter_inc(&m->total_metadatas);
+ percpu_counter_add(&m->metadata_latency_sum, r_end - r_start);
+}
#endif /* _FS_CEPH_MDS_METRIC_H */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v9 0/5] ceph: add perf metrics support
2020-03-09 7:37 [PATCH v9 0/5] ceph: add perf metrics support xiubli
` (4 preceding siblings ...)
2020-03-09 7:37 ` [PATCH v9 5/5] ceph: add global metadata perf " xiubli
@ 2020-03-09 12:09 ` Jeff Layton
2020-03-09 12:35 ` Xiubo Li
5 siblings, 1 reply; 14+ messages in thread
From: Jeff Layton @ 2020-03-09 12:09 UTC (permalink / raw)
To: xiubli, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel
On Mon, 2020-03-09 at 03:37 -0400, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
>
> Changed in V9:
> - add an r_ended field to the mds request struct and use that to calculate the metric
> - fix some commit comments
>
> We can get the metrics from the debugfs:
>
> $ cat /sys/kernel/debug/ceph/0c93a60d-5645-4c46-8568-4c8f63db4c7f.client4267/metrics
> item total sum_lat(us) avg_lat(us)
> -----------------------------------------------------
> read 13 417000 32076
> write 42 131205000 3123928
> metadata 104 493000 4740
>
> item total miss hit
> -------------------------------------------------
> d_lease 204 0 918
> caps 204 213 368218
>
Thanks Xiubo! This looks good. One minor issue with the cap patch, but I
can just fix that up before merging if you're ok with my proposed
change.
Beyond this...while average latency is a good metric, it's often not
enough to help diagnose problems. I wonder if we ought to be at least
tracking min/max latency for all calls too. I wonder if there's way to
track standard deviation too? That would be really nice to have.
Cheers,
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v9 0/5] ceph: add perf metrics support
2020-03-09 12:09 ` [PATCH v9 0/5] ceph: add perf metrics support Jeff Layton
@ 2020-03-09 12:35 ` Xiubo Li
0 siblings, 0 replies; 14+ messages in thread
From: Xiubo Li @ 2020-03-09 12:35 UTC (permalink / raw)
To: Jeff Layton, idryomov; +Cc: sage, zyan, pdonnell, ceph-devel
On 2020/3/9 20:09, Jeff Layton wrote:
> On Mon, 2020-03-09 at 03:37 -0400, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> Changed in V9:
>> - add an r_ended field to the mds request struct and use that to calculate the metric
>> - fix some commit comments
>>
>> We can get the metrics from the debugfs:
>>
>> $ cat /sys/kernel/debug/ceph/0c93a60d-5645-4c46-8568-4c8f63db4c7f.client4267/metrics
>> item total sum_lat(us) avg_lat(us)
>> -----------------------------------------------------
>> read 13 417000 32076
>> write 42 131205000 3123928
>> metadata 104 493000 4740
>>
>> item total miss hit
>> -------------------------------------------------
>> d_lease 204 0 918
>> caps 204 213 368218
>>
> Thanks Xiubo! This looks good. One minor issue with the cap patch, but I
> can just fix that up before merging if you're ok with my proposed
> change.
>
> Beyond this...while average latency is a good metric, it's often not
> enough to help diagnose problems. I wonder if we ought to be at least
> tracking min/max latency for all calls too. I wonder if there's way to
> track standard deviation too? That would be really nice to have.
yeah, the min/max latencies here make sense, it is on my todo list and I
will do it after this patch series.
And for the standard deviation I will try to have a investigate of it.
Thanks
> Cheers,
^ permalink raw reply [flat|nested] 14+ messages in thread