[PATCH v4 0/4] ceph: add min/max/stdev latency support

[PATCH v4 0/4] ceph: add min/max/stdev latency support

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

Changed in V4:
- fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4]
- rebase and combine the stdev patch series [3/4][4/4]
- remove the sum latency showing, which makes no sense for debugging, if it
  is really needed in some case then just do (avg * total) in userland. [4/4]
- switch {read/write/metadata}_latency_sum to atomic type since it will be
  readed very time when updating the latencies to calculate the stdev. [4/4]

Changed in V2:
- switch spin lock to cmpxchg [1/4]

Changed in V3:
- add the __update_min/max_latency helpers [1/4]



# cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics
item          total       avg_lat(us)     min_lat(us)     max_lat(us)     stdev(us)
-----------------------------------------------------------------------------------
read          2312        9000            1000            100000          607.4
write         21777       925000          2000            44551000        29700.3
metadata      6           4179000         1000            21414000        19590.8

item          total           miss            hit
-------------------------------------------------
d_lease       2               0               11
caps          2               14              398418



Xiubo Li (4):
  ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches
  ceph: add min/max latency support for read/write/metadata metrics
  ceph: move the metric helpers into one separate file
  ceph: add standard deviation support for read/write/metadata perf
    metric

 fs/ceph/Makefile     |   2 +-
 fs/ceph/debugfs.c    |  89 ++++++++++++++++++------
 fs/ceph/mds_client.c |  83 +---------------------
 fs/ceph/metric.c     | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ceph/metric.h     |  79 +++++++++++----------
 5 files changed, 297 insertions(+), 146 deletions(-)
 create mode 100644 fs/ceph/metric.c

-- 
1.8.3.1

[PATCH v4 1/4] ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

fs/ceph/debugfs.c:140: undefined reference to `__divdi3'
Use math64 helpers to avoid 64-bit div on 32-bit arches.

Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/debugfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 60f3e307..95e8693 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -137,19 +137,19 @@ static int metric_show(struct seq_file *s, void *p)
 	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;
+	avg = total ? DIV64_U64_ROUND_CLOSEST(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;
+	avg = total ? DIV64_U64_ROUND_CLOSEST(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;
+	avg = total ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
 	seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "metadata", total, sum, avg);
 
 	seq_printf(s, "\n");
-- 
1.8.3.1

[PATCH v4 2/4] ceph: add min/max latency support for read/write/metadata metrics

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

These will be very useful help diagnose problems.

URL: https://tracker.ceph.com/issues/44533
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/debugfs.c    | 27 +++++++++++++++++++++------
 fs/ceph/mds_client.c |  9 +++++++++
 fs/ceph/metric.h     | 51 ++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 95e8693..01b95fe 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -129,28 +129,43 @@ 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;
+	s64 total, sum, avg = 0, min, max;
 
-	seq_printf(s, "item          total       sum_lat(us)     avg_lat(us)\n");
-	seq_printf(s, "-----------------------------------------------------\n");
+	seq_printf(s, "item          total       sum_lat(us)     avg_lat(us)     min_lat(us)     max_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 ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
-	seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "read", total, sum, avg);
+	min = atomic64_read(&mdsc->metric.read_latency_min);
+	min = jiffies_to_usecs(min == S64_MAX ? 0 : min);
+	max = atomic64_read(&mdsc->metric.read_latency_max);
+	max = jiffies_to_usecs(max);
+	seq_printf(s, "%-14s%-12lld%-16lld%-16lld%-16lld%lld\n", "read",
+		   total, sum, avg, min, max);
 
 	total = percpu_counter_sum(&mdsc->metric.total_writes);
 	sum = percpu_counter_sum(&mdsc->metric.write_latency_sum);
 	sum = jiffies_to_usecs(sum);
 	avg = total ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
-	seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "write", total, sum, avg);
+	min = atomic64_read(&mdsc->metric.write_latency_min);
+	min = jiffies_to_usecs(min == S64_MAX ? 0 : min);
+	max = atomic64_read(&mdsc->metric.write_latency_max);
+	max = jiffies_to_usecs(max);
+	seq_printf(s, "%-14s%-12lld%-16lld%-16lld%-16lld%lld\n", "write",
+		   total, sum, avg, min, max);
 
 	total = percpu_counter_sum(&mdsc->metric.total_metadatas);
 	sum = percpu_counter_sum(&mdsc->metric.metadata_latency_sum);
 	sum = jiffies_to_usecs(sum);
 	avg = total ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
-	seq_printf(s, "%-14s%-12lld%-16lld%lld\n", "metadata", total, sum, avg);
+	min = atomic64_read(&mdsc->metric.metadata_latency_min);
+	min = jiffies_to_usecs(min == S64_MAX ? 0 : min);
+	max = atomic64_read(&mdsc->metric.metadata_latency_max);
+	max = jiffies_to_usecs(max);
+	seq_printf(s, "%-14s%-12lld%-16lld%-16lld%-16lld%lld\n", "metadata",
+		   total, sum, avg, min, max);
 
 	seq_printf(s, "\n");
 	seq_printf(s, "item          total           miss            hit\n");
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 5c03ed3..a3b2810 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -4358,6 +4358,9 @@ static int ceph_mdsc_metric_init(struct ceph_client_metric *metric)
 	if (ret)
 		goto err_read_latency_sum;
 
+	atomic64_set(&metric->read_latency_min, S64_MAX);
+	atomic64_set(&metric->read_latency_max, 0);
+
 	ret = percpu_counter_init(&metric->total_writes, 0, GFP_KERNEL);
 	if (ret)
 		goto err_total_writes;
@@ -4366,6 +4369,9 @@ static int ceph_mdsc_metric_init(struct ceph_client_metric *metric)
 	if (ret)
 		goto err_write_latency_sum;
 
+	atomic64_set(&metric->write_latency_min, S64_MAX);
+	atomic64_set(&metric->write_latency_max, 0);
+
 	ret = percpu_counter_init(&metric->total_metadatas, 0, GFP_KERNEL);
 	if (ret)
 		goto err_total_metadatas;
@@ -4374,6 +4380,9 @@ static int ceph_mdsc_metric_init(struct ceph_client_metric *metric)
 	if (ret)
 		goto err_metadata_latency_sum;
 
+	atomic64_set(&metric->metadata_latency_min, S64_MAX);
+	atomic64_set(&metric->metadata_latency_max, 0);
+
 	return 0;
 
 err_metadata_latency_sum:
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index faba142..b36f7f9 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -2,6 +2,9 @@
 #ifndef _FS_CEPH_MDS_METRIC_H
 #define _FS_CEPH_MDS_METRIC_H
 
+#include <linux/atomic.h>
+#include <linux/percpu.h>
+
 /* This is the global metrics */
 struct ceph_client_metric {
 	atomic64_t            total_dentries;
@@ -13,12 +16,18 @@ struct ceph_client_metric {
 
 	struct percpu_counter total_reads;
 	struct percpu_counter read_latency_sum;
+	atomic64_t read_latency_min;
+	atomic64_t read_latency_max;
 
 	struct percpu_counter total_writes;
 	struct percpu_counter write_latency_sum;
+	atomic64_t write_latency_min;
+	atomic64_t write_latency_max;
 
 	struct percpu_counter total_metadatas;
 	struct percpu_counter metadata_latency_sum;
+	atomic64_t metadata_latency_min;
+	atomic64_t metadata_latency_max;
 };
 
 static inline void ceph_update_cap_hit(struct ceph_client_metric *m)
@@ -31,16 +40,44 @@ static inline void ceph_update_cap_mis(struct ceph_client_metric *m)
 	percpu_counter_inc(&m->i_caps_mis);
 }
 
+static inline void __update_min_latency(atomic64_t *min, unsigned long lat)
+{
+	unsigned long cur, old;
+
+	cur = atomic64_read(min);
+	do {
+		old = cur;
+		if (likely(lat >= old))
+			break;
+	} while (unlikely((cur = atomic64_cmpxchg(min, old, lat)) != old));
+}
+
+static inline void __update_max_latency(atomic64_t *max, unsigned long lat)
+{
+	unsigned long cur, old;
+
+	cur = atomic64_read(max);
+	do {
+		old = cur;
+		if (likely(lat <= old))
+			break;
+	} while (unlikely((cur = atomic64_cmpxchg(max, old, lat)) != old));
+}
+
 static inline void ceph_update_read_latency(struct ceph_client_metric *m,
 					    unsigned long r_start,
 					    unsigned long r_end,
 					    int rc)
 {
+	unsigned long lat = r_end - r_start;
+
 	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);
+	percpu_counter_add(&m->read_latency_sum, lat);
+	__update_min_latency(&m->read_latency_min, lat);
+	__update_max_latency(&m->read_latency_max, lat);
 }
 
 static inline void ceph_update_write_latency(struct ceph_client_metric *m,
@@ -48,11 +85,15 @@ static inline void ceph_update_write_latency(struct ceph_client_metric *m,
 					     unsigned long r_end,
 					     int rc)
 {
+	unsigned long lat = r_end - r_start;
+
 	if (rc && rc != -ETIMEDOUT)
 		return;
 
 	percpu_counter_inc(&m->total_writes);
-	percpu_counter_add(&m->write_latency_sum, r_end - r_start);
+	percpu_counter_add(&m->write_latency_sum, lat);
+	__update_min_latency(&m->write_latency_min, lat);
+	__update_max_latency(&m->write_latency_max, lat);
 }
 
 static inline void ceph_update_metadata_latency(struct ceph_client_metric *m,
@@ -60,10 +101,14 @@ static inline void ceph_update_metadata_latency(struct ceph_client_metric *m,
 						unsigned long r_end,
 						int rc)
 {
+	unsigned long lat = r_end - r_start;
+
 	if (rc && rc != -ENOENT)
 		return;
 
 	percpu_counter_inc(&m->total_metadatas);
-	percpu_counter_add(&m->metadata_latency_sum, r_end - r_start);
+	percpu_counter_add(&m->metadata_latency_sum, lat);
+	__update_min_latency(&m->metadata_latency_min, lat);
+	__update_max_latency(&m->metadata_latency_max, lat);
 }
 #endif /* _FS_CEPH_MDS_METRIC_H */
-- 
1.8.3.1

[PATCH v4 3/4] ceph: move the metric helpers into one separate file

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

The inline is not proper any more dues to the helpers becomes
larger.

URL: https://tracker.ceph.com/issues/44534
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/Makefile     |   2 +-
 fs/ceph/mds_client.c |  92 +--------------------------
 fs/ceph/metric.c     | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ceph/metric.h     |  87 +++++---------------------
 4 files changed, 190 insertions(+), 164 deletions(-)
 create mode 100644 fs/ceph/metric.c

diff --git a/fs/ceph/Makefile b/fs/ceph/Makefile
index 0a0823d..50c635d 100644
--- a/fs/ceph/Makefile
+++ b/fs/ceph/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_CEPH_FS) += ceph.o
 ceph-y := super.o inode.o dir.o file.o locks.o addr.o ioctl.o \
 	export.o caps.o snap.o xattr.o quota.o io.o \
 	mds_client.o mdsmap.o strings.o ceph_frag.o \
-	debugfs.o util.o
+	debugfs.o util.o metric.o
 
 ceph-$(CONFIG_CEPH_FSCACHE) += cache.o
 ceph-$(CONFIG_CEPH_FS_POSIX_ACL) += acl.o
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index a3b2810..eb2657e 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -4326,87 +4326,6 @@ 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)
-		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;
-
-	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;
-
-	atomic64_set(&metric->read_latency_min, S64_MAX);
-	atomic64_set(&metric->read_latency_max, 0);
-
-	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;
-
-	atomic64_set(&metric->write_latency_min, S64_MAX);
-	atomic64_set(&metric->write_latency_max, 0);
-
-	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;
-
-	atomic64_set(&metric->metadata_latency_min, S64_MAX);
-	atomic64_set(&metric->metadata_latency_max, 0);
-
-	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:
-	percpu_counter_destroy(&metric->read_latency_sum);
-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:
-	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)
 
 {
@@ -4744,16 +4663,7 @@ 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);
-	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);
-	percpu_counter_destroy(&mdsc->metric.d_lease_hit);
+	ceph_mdsc_metric_destroy(&mdsc->metric);
 
 	fsc->mdsc = NULL;
 	kfree(mdsc);
diff --git a/fs/ceph/metric.c b/fs/ceph/metric.c
new file mode 100644
index 0000000..1b764df
--- /dev/null
+++ b/fs/ceph/metric.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/atomic.h>
+#include <linux/percpu_counter.h>
+
+#include "metric.h"
+
+int ceph_mdsc_metric_init(struct ceph_client_metric *m)
+{
+	int ret;
+
+	if (!m)
+		return -EINVAL;
+
+	atomic64_set(&m->total_dentries, 0);
+	ret = percpu_counter_init(&m->d_lease_hit, 0, GFP_KERNEL);
+	if (ret)
+		return ret;
+
+	ret = percpu_counter_init(&m->d_lease_mis, 0, GFP_KERNEL);
+	if (ret)
+		goto err_d_lease_mis;
+
+	ret = percpu_counter_init(&m->i_caps_hit, 0, GFP_KERNEL);
+	if (ret)
+		goto err_i_caps_hit;
+
+	ret = percpu_counter_init(&m->i_caps_mis, 0, GFP_KERNEL);
+	if (ret)
+		goto err_i_caps_mis;
+
+	ret = percpu_counter_init(&m->total_reads, 0, GFP_KERNEL);
+	if (ret)
+		goto err_total_reads;
+
+	ret = percpu_counter_init(&m->read_latency_sum, 0, GFP_KERNEL);
+	if (ret)
+		goto err_read_latency_sum;
+
+	atomic64_set(&m->read_latency_min, S64_MAX);
+	atomic64_set(&m->read_latency_max, 0);
+
+	ret = percpu_counter_init(&m->total_writes, 0, GFP_KERNEL);
+	if (ret)
+		goto err_total_writes;
+
+	ret = percpu_counter_init(&m->write_latency_sum, 0, GFP_KERNEL);
+	if (ret)
+		goto err_write_latency_sum;
+
+	atomic64_set(&m->write_latency_min, S64_MAX);
+	atomic64_set(&m->write_latency_max, 0);
+
+	ret = percpu_counter_init(&m->total_metadatas, 0, GFP_KERNEL);
+	if (ret)
+		goto err_total_metadatas;
+
+	ret = percpu_counter_init(&m->metadata_latency_sum, 0, GFP_KERNEL);
+	if (ret)
+		goto err_metadata_latency_sum;
+
+	atomic64_set(&m->metadata_latency_min, S64_MAX);
+	atomic64_set(&m->metadata_latency_max, 0);
+
+	return 0;
+
+err_metadata_latency_sum:
+	percpu_counter_destroy(&m->total_metadatas);
+err_total_metadatas:
+	percpu_counter_destroy(&m->write_latency_sum);
+err_write_latency_sum:
+	percpu_counter_destroy(&m->total_writes);
+err_total_writes:
+	percpu_counter_destroy(&m->read_latency_sum);
+err_read_latency_sum:
+	percpu_counter_destroy(&m->total_reads);
+err_total_reads:
+	percpu_counter_destroy(&m->i_caps_mis);
+err_i_caps_mis:
+	percpu_counter_destroy(&m->i_caps_hit);
+err_i_caps_hit:
+	percpu_counter_destroy(&m->d_lease_mis);
+err_d_lease_mis:
+	percpu_counter_destroy(&m->d_lease_hit);
+
+	return ret;
+}
+
+void ceph_mdsc_metric_destroy(struct ceph_client_metric *m)
+{
+	percpu_counter_destroy(&m->metadata_latency_sum);
+	percpu_counter_destroy(&m->total_metadatas);
+	percpu_counter_destroy(&m->write_latency_sum);
+	percpu_counter_destroy(&m->total_writes);
+	percpu_counter_destroy(&m->read_latency_sum);
+	percpu_counter_destroy(&m->total_reads);
+	percpu_counter_destroy(&m->i_caps_mis);
+	percpu_counter_destroy(&m->i_caps_hit);
+	percpu_counter_destroy(&m->d_lease_mis);
+	percpu_counter_destroy(&m->d_lease_hit);
+}
+
+static inline void __update_min_latency(atomic64_t *min, unsigned long lat)
+{
+	unsigned long cur, old;
+
+	cur = atomic64_read(min);
+	do {
+		old = cur;
+		if (likely(lat >= old))
+			break;
+	} while (unlikely((cur = atomic64_cmpxchg(min, old, lat)) != old));
+}
+
+static inline void __update_max_latency(atomic64_t *max, unsigned long lat)
+{
+	unsigned long cur, old;
+
+	cur = atomic64_read(max);
+	do {
+		old = cur;
+		if (likely(lat <= old))
+			break;
+	} while (unlikely((cur = atomic64_cmpxchg(max, old, lat)) != old));
+}
+
+void ceph_update_read_latency(struct ceph_client_metric *m,
+			      unsigned long r_start,
+			      unsigned long r_end,
+			      int rc)
+{
+	unsigned long lat = r_end - r_start;
+
+	if (rc < 0 && rc != -ENOENT && rc != -ETIMEDOUT)
+		return;
+
+	percpu_counter_inc(&m->total_reads);
+	percpu_counter_add(&m->read_latency_sum, lat);
+	__update_min_latency(&m->read_latency_min, lat);
+	__update_max_latency(&m->read_latency_max, lat);
+}
+
+void ceph_update_write_latency(struct ceph_client_metric *m,
+			       unsigned long r_start,
+			       unsigned long r_end,
+			       int rc)
+{
+	unsigned long lat = r_end - r_start;
+
+	if (rc && rc != -ETIMEDOUT)
+		return;
+
+	percpu_counter_inc(&m->total_writes);
+	percpu_counter_add(&m->write_latency_sum, lat);
+	__update_min_latency(&m->write_latency_min, lat);
+	__update_max_latency(&m->write_latency_max, lat);
+}
+
+void ceph_update_metadata_latency(struct ceph_client_metric *m,
+				  unsigned long r_start,
+				  unsigned long r_end,
+				  int rc)
+{
+	unsigned long lat = r_end - r_start;
+
+	if (rc && rc != -ENOENT)
+		return;
+
+	percpu_counter_inc(&m->total_metadatas);
+	percpu_counter_add(&m->metadata_latency_sum, lat);
+	__update_min_latency(&m->metadata_latency_min, lat);
+	__update_max_latency(&m->metadata_latency_max, lat);
+}
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index b36f7f9..f139aff 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -3,7 +3,7 @@
 #define _FS_CEPH_MDS_METRIC_H
 
 #include <linux/atomic.h>
-#include <linux/percpu.h>
+#include <linux/percpu_counter.h>
 
 /* This is the global metrics */
 struct ceph_client_metric {
@@ -40,75 +40,18 @@ static inline void ceph_update_cap_mis(struct ceph_client_metric *m)
 	percpu_counter_inc(&m->i_caps_mis);
 }
 
-static inline void __update_min_latency(atomic64_t *min, unsigned long lat)
-{
-	unsigned long cur, old;
-
-	cur = atomic64_read(min);
-	do {
-		old = cur;
-		if (likely(lat >= old))
-			break;
-	} while (unlikely((cur = atomic64_cmpxchg(min, old, lat)) != old));
-}
-
-static inline void __update_max_latency(atomic64_t *max, unsigned long lat)
-{
-	unsigned long cur, old;
-
-	cur = atomic64_read(max);
-	do {
-		old = cur;
-		if (likely(lat <= old))
-			break;
-	} while (unlikely((cur = atomic64_cmpxchg(max, old, lat)) != old));
-}
-
-static inline void ceph_update_read_latency(struct ceph_client_metric *m,
-					    unsigned long r_start,
-					    unsigned long r_end,
-					    int rc)
-{
-	unsigned long lat = r_end - r_start;
-
-	if (rc < 0 && rc != -ENOENT && rc != -ETIMEDOUT)
-		return;
-
-	percpu_counter_inc(&m->total_reads);
-	percpu_counter_add(&m->read_latency_sum, lat);
-	__update_min_latency(&m->read_latency_min, lat);
-	__update_max_latency(&m->read_latency_max, lat);
-}
-
-static inline void ceph_update_write_latency(struct ceph_client_metric *m,
-					     unsigned long r_start,
-					     unsigned long r_end,
-					     int rc)
-{
-	unsigned long lat = r_end - r_start;
-
-	if (rc && rc != -ETIMEDOUT)
-		return;
-
-	percpu_counter_inc(&m->total_writes);
-	percpu_counter_add(&m->write_latency_sum, lat);
-	__update_min_latency(&m->write_latency_min, lat);
-	__update_max_latency(&m->write_latency_max, lat);
-}
-
-static inline void ceph_update_metadata_latency(struct ceph_client_metric *m,
-						unsigned long r_start,
-						unsigned long r_end,
-						int rc)
-{
-	unsigned long lat = r_end - r_start;
-
-	if (rc && rc != -ENOENT)
-		return;
-
-	percpu_counter_inc(&m->total_metadatas);
-	percpu_counter_add(&m->metadata_latency_sum, lat);
-	__update_min_latency(&m->metadata_latency_min, lat);
-	__update_max_latency(&m->metadata_latency_max, lat);
-}
+extern int ceph_mdsc_metric_init(struct ceph_client_metric *m);
+extern void ceph_mdsc_metric_destroy(struct ceph_client_metric *m);
+extern void ceph_update_read_latency(struct ceph_client_metric *m,
+				     unsigned long r_start,
+				     unsigned long r_end,
+				     int rc);
+extern void ceph_update_write_latency(struct ceph_client_metric *m,
+				      unsigned long r_start,
+				      unsigned long r_end,
+				      int rc);
+extern void ceph_update_metadata_latency(struct ceph_client_metric *m,
+					 unsigned long r_start,
+					 unsigned long r_end,
+					 int rc);
 #endif /* _FS_CEPH_MDS_METRIC_H */
-- 
1.8.3.1

[PATCH v4 4/4] ceph: add standard deviation support for read/write/metadata perf metric

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

Switch {read/write/metadata}_latency_sum to atomic type and remove
{read/write/metadata}_latency_sum showing in the debugfs, which makes
no sense.

URL: https://tracker.ceph.com/issues/44534
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/debugfs.c | 88 +++++++++++++++++++++++++++++++++----------------
 fs/ceph/metric.c  | 99 ++++++++++++++++++++++++++++++++-----------------------
 fs/ceph/metric.h  | 18 ++++++----
 3 files changed, 129 insertions(+), 76 deletions(-)

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 01b95fe..21f5663 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/ceph/ceph_debug.h>
 
+#include <linux/kernel.h>
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -124,48 +125,77 @@ static int mdsc_show(struct seq_file *s, void *p)
 	return 0;
 }
 
+static s64 get_avg(atomic64_t *totalp, atomic64_t *sump, spinlock_t *lockp,
+		   s64 *total)
+{
+	s64 n, sum, avg = 0;
+
+	spin_lock(lockp);
+	n = atomic64_read(totalp);
+	sum = atomic64_read(sump);
+	spin_unlock(lockp);
+
+	if (likely(n))
+		avg = DIV64_U64_ROUND_CLOSEST(sum, n);
+
+	*total = n;
+	return avg;
+}
+
+#define METRIC(name, total, avg, min, max, sq)	{			\
+	s64 _total, _avg, _min, _max, _sq, _st, _re = 0;		\
+	_avg = jiffies_to_usecs(avg);					\
+	_min = jiffies_to_usecs(min == S64_MAX ? 0 : min);		\
+	_max = jiffies_to_usecs(max);					\
+	_total = total - 1;						\
+	_sq = _total > 0 ? DIV64_U64_ROUND_CLOSEST(sq, _total) : 0;	\
+	_sq = jiffies_to_usecs(_sq);					\
+	_st = int_sqrt64(_sq);						\
+	if (_st > 0) {							\
+		_re = 5 * (_sq - (_st * _st));				\
+		_re = _re > 0 ? _re - 1 : 0;				\
+		_re = _st > 0 ? div64_s64(_re, _st) : 0;		\
+	}								\
+	seq_printf(s, "%-14s%-12lld%-16lld%-16lld%-16lld%lld.%lld\n",	\
+		   name, total, _avg, _min, _max, _st, _re);		\
+}
+
 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, min, max;
+	s64 total, avg, min, max, sq;
 
-	seq_printf(s, "item          total       sum_lat(us)     avg_lat(us)     min_lat(us)     max_lat(us)\n");
-	seq_printf(s, "-------------------------------------------------------------------------------------\n");
+	seq_printf(s, "item          total       avg_lat(us)     min_lat(us)     max_lat(us)     stdev(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 ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
+	avg = get_avg(&mdsc->metric.total_reads,
+		      &mdsc->metric.read_latency_sum,
+		      &mdsc->metric.read_latency_lock,
+		      &total);
 	min = atomic64_read(&mdsc->metric.read_latency_min);
-	min = jiffies_to_usecs(min == S64_MAX ? 0 : min);
 	max = atomic64_read(&mdsc->metric.read_latency_max);
-	max = jiffies_to_usecs(max);
-	seq_printf(s, "%-14s%-12lld%-16lld%-16lld%-16lld%lld\n", "read",
-		   total, sum, avg, min, max);
-
-	total = percpu_counter_sum(&mdsc->metric.total_writes);
-	sum = percpu_counter_sum(&mdsc->metric.write_latency_sum);
-	sum = jiffies_to_usecs(sum);
-	avg = total ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
+	sq = percpu_counter_sum(&mdsc->metric.read_latency_sq_sum);
+	METRIC("read", total, avg, min, max, sq);
+
+	avg = get_avg(&mdsc->metric.total_writes,
+		      &mdsc->metric.write_latency_sum,
+		      &mdsc->metric.write_latency_lock,
+		      &total);
 	min = atomic64_read(&mdsc->metric.write_latency_min);
-	min = jiffies_to_usecs(min == S64_MAX ? 0 : min);
 	max = atomic64_read(&mdsc->metric.write_latency_max);
-	max = jiffies_to_usecs(max);
-	seq_printf(s, "%-14s%-12lld%-16lld%-16lld%-16lld%lld\n", "write",
-		   total, sum, avg, min, max);
-
-	total = percpu_counter_sum(&mdsc->metric.total_metadatas);
-	sum = percpu_counter_sum(&mdsc->metric.metadata_latency_sum);
-	sum = jiffies_to_usecs(sum);
-	avg = total ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
+	sq = percpu_counter_sum(&mdsc->metric.write_latency_sq_sum);
+	METRIC("write", total, avg, min, max, sq);
+
+	avg = get_avg(&mdsc->metric.total_metadatas,
+		      &mdsc->metric.metadata_latency_sum,
+		      &mdsc->metric.metadata_latency_lock,
+		      &total);
 	min = atomic64_read(&mdsc->metric.metadata_latency_min);
-	min = jiffies_to_usecs(min == S64_MAX ? 0 : min);
 	max = atomic64_read(&mdsc->metric.metadata_latency_max);
-	max = jiffies_to_usecs(max);
-	seq_printf(s, "%-14s%-12lld%-16lld%-16lld%-16lld%lld\n", "metadata",
-		   total, sum, avg, min, max);
+	sq = percpu_counter_sum(&mdsc->metric.metadata_latency_sq_sum);
+	METRIC("metadata", total, avg, min, max, sq);
 
 	seq_printf(s, "\n");
 	seq_printf(s, "item          total           miss            hit\n");
diff --git a/fs/ceph/metric.c b/fs/ceph/metric.c
index 1b764df..23bd80f 100644
--- a/fs/ceph/metric.c
+++ b/fs/ceph/metric.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#include <linux/kernel.h>
 #include <linux/atomic.h>
 #include <linux/percpu_counter.h>
 
@@ -29,52 +30,43 @@ int ceph_mdsc_metric_init(struct ceph_client_metric *m)
 	if (ret)
 		goto err_i_caps_mis;
 
-	ret = percpu_counter_init(&m->total_reads, 0, GFP_KERNEL);
+	ret = percpu_counter_init(&m->read_latency_sq_sum, 0, GFP_KERNEL);
 	if (ret)
-		goto err_total_reads;
-
-	ret = percpu_counter_init(&m->read_latency_sum, 0, GFP_KERNEL);
-	if (ret)
-		goto err_read_latency_sum;
+		goto err_read_latency_sq_sum;
 
+	spin_lock_init(&m->read_latency_lock);
+	atomic64_set(&m->total_reads, 0);
+	atomic64_set(&m->read_latency_sum, 0);
 	atomic64_set(&m->read_latency_min, S64_MAX);
 	atomic64_set(&m->read_latency_max, 0);
 
-	ret = percpu_counter_init(&m->total_writes, 0, GFP_KERNEL);
-	if (ret)
-		goto err_total_writes;
-
-	ret = percpu_counter_init(&m->write_latency_sum, 0, GFP_KERNEL);
+	ret = percpu_counter_init(&m->write_latency_sq_sum, 0, GFP_KERNEL);
 	if (ret)
-		goto err_write_latency_sum;
+		goto err_write_latency_sq_sum;
 
+	spin_lock_init(&m->write_latency_lock);
+	atomic64_set(&m->total_writes, 0);
+	atomic64_set(&m->write_latency_sum, 0);
 	atomic64_set(&m->write_latency_min, S64_MAX);
 	atomic64_set(&m->write_latency_max, 0);
 
-	ret = percpu_counter_init(&m->total_metadatas, 0, GFP_KERNEL);
+	ret = percpu_counter_init(&m->metadata_latency_sq_sum, 0, GFP_KERNEL);
 	if (ret)
-		goto err_total_metadatas;
-
-	ret = percpu_counter_init(&m->metadata_latency_sum, 0, GFP_KERNEL);
-	if (ret)
-		goto err_metadata_latency_sum;
+		goto err_metadata_latency_sq_sum;
 
+	spin_lock_init(&m->metadata_latency_lock);
+	atomic64_set(&m->total_metadatas, 0);
+	atomic64_set(&m->metadata_latency_sum, 0);
 	atomic64_set(&m->metadata_latency_min, S64_MAX);
 	atomic64_set(&m->metadata_latency_max, 0);
 
 	return 0;
 
-err_metadata_latency_sum:
-	percpu_counter_destroy(&m->total_metadatas);
-err_total_metadatas:
-	percpu_counter_destroy(&m->write_latency_sum);
-err_write_latency_sum:
-	percpu_counter_destroy(&m->total_writes);
-err_total_writes:
-	percpu_counter_destroy(&m->read_latency_sum);
-err_read_latency_sum:
-	percpu_counter_destroy(&m->total_reads);
-err_total_reads:
+err_metadata_latency_sq_sum:
+	percpu_counter_destroy(&m->write_latency_sq_sum);
+err_write_latency_sq_sum:
+	percpu_counter_destroy(&m->read_latency_sq_sum);
+err_read_latency_sq_sum:
 	percpu_counter_destroy(&m->i_caps_mis);
 err_i_caps_mis:
 	percpu_counter_destroy(&m->i_caps_hit);
@@ -88,12 +80,9 @@ int ceph_mdsc_metric_init(struct ceph_client_metric *m)
 
 void ceph_mdsc_metric_destroy(struct ceph_client_metric *m)
 {
-	percpu_counter_destroy(&m->metadata_latency_sum);
-	percpu_counter_destroy(&m->total_metadatas);
-	percpu_counter_destroy(&m->write_latency_sum);
-	percpu_counter_destroy(&m->total_writes);
-	percpu_counter_destroy(&m->read_latency_sum);
-	percpu_counter_destroy(&m->total_reads);
+	percpu_counter_destroy(&m->metadata_latency_sq_sum);
+	percpu_counter_destroy(&m->write_latency_sq_sum);
+	percpu_counter_destroy(&m->read_latency_sq_sum);
 	percpu_counter_destroy(&m->i_caps_mis);
 	percpu_counter_destroy(&m->i_caps_hit);
 	percpu_counter_destroy(&m->d_lease_mis);
@@ -124,6 +113,28 @@ static inline void __update_max_latency(atomic64_t *max, unsigned long lat)
 	} while (unlikely((cur = atomic64_cmpxchg(max, old, lat)) != old));
 }
 
+static inline void __update_avg_and_sq(atomic64_t *totalp, atomic64_t *lat_sump,
+				       struct percpu_counter *sq_sump,
+				       spinlock_t *lockp, unsigned long lat)
+{
+	s64 total, avg, sq, lsum;
+
+	spin_lock(lockp);
+	total = atomic64_inc_return(totalp);
+	lsum = atomic64_add_return(lat, lat_sump);
+	spin_unlock(lockp);
+
+	if (unlikely(total == 1))
+		return;
+
+	/* the sq is (lat - old_avg) * (lat - new_avg) */
+	avg = DIV64_U64_ROUND_CLOSEST((lsum - lat), (total - 1));
+	sq = lat - avg;
+	avg = DIV64_U64_ROUND_CLOSEST(lsum, total);
+	sq = sq * (lat - avg);
+	percpu_counter_add(sq_sump, sq);
+}
+
 void ceph_update_read_latency(struct ceph_client_metric *m,
 			      unsigned long r_start,
 			      unsigned long r_end,
@@ -134,10 +145,12 @@ void ceph_update_read_latency(struct ceph_client_metric *m,
 	if (rc < 0 && rc != -ENOENT && rc != -ETIMEDOUT)
 		return;
 
-	percpu_counter_inc(&m->total_reads);
-	percpu_counter_add(&m->read_latency_sum, lat);
 	__update_min_latency(&m->read_latency_min, lat);
 	__update_max_latency(&m->read_latency_max, lat);
+	__update_avg_and_sq(&m->total_reads, &m->read_latency_sum,
+			    &m->read_latency_sq_sum,
+			    &m->read_latency_lock,
+			    lat);
 }
 
 void ceph_update_write_latency(struct ceph_client_metric *m,
@@ -150,10 +163,12 @@ void ceph_update_write_latency(struct ceph_client_metric *m,
 	if (rc && rc != -ETIMEDOUT)
 		return;
 
-	percpu_counter_inc(&m->total_writes);
-	percpu_counter_add(&m->write_latency_sum, lat);
 	__update_min_latency(&m->write_latency_min, lat);
 	__update_max_latency(&m->write_latency_max, lat);
+	__update_avg_and_sq(&m->total_writes, &m->write_latency_sum,
+			    &m->write_latency_sq_sum,
+			    &m->write_latency_lock,
+			    lat);
 }
 
 void ceph_update_metadata_latency(struct ceph_client_metric *m,
@@ -166,8 +181,10 @@ void ceph_update_metadata_latency(struct ceph_client_metric *m,
 	if (rc && rc != -ENOENT)
 		return;
 
-	percpu_counter_inc(&m->total_metadatas);
-	percpu_counter_add(&m->metadata_latency_sum, lat);
 	__update_min_latency(&m->metadata_latency_min, lat);
 	__update_max_latency(&m->metadata_latency_max, lat);
+	__update_avg_and_sq(&m->total_metadatas, &m->metadata_latency_sum,
+			    &m->metadata_latency_sq_sum,
+			    &m->metadata_latency_lock,
+			    lat);
 }
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index f139aff..d63b95e 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -14,18 +14,24 @@ 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;
+	struct percpu_counter read_latency_sq_sum;
+	spinlock_t read_latency_lock;
+	atomic64_t total_reads;
+	atomic64_t read_latency_sum;
 	atomic64_t read_latency_min;
 	atomic64_t read_latency_max;
 
-	struct percpu_counter total_writes;
-	struct percpu_counter write_latency_sum;
+	struct percpu_counter write_latency_sq_sum;
+	spinlock_t write_latency_lock;
+	atomic64_t total_writes;
+	atomic64_t write_latency_sum;
 	atomic64_t write_latency_min;
 	atomic64_t write_latency_max;
 
-	struct percpu_counter total_metadatas;
-	struct percpu_counter metadata_latency_sum;
+	struct percpu_counter metadata_latency_sq_sum;
+	spinlock_t metadata_latency_lock;
+	atomic64_t total_metadatas;
+	atomic64_t metadata_latency_sum;
 	atomic64_t metadata_latency_min;
 	atomic64_t metadata_latency_max;
 };
-- 
1.8.3.1

Re: [PATCH v4 0/4] ceph: add min/max/stdev latency support

[Ilya Dryomov]

On Wed, Mar 18, 2020 at 6:46 AM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> Changed in V4:
> - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4]
> - rebase and combine the stdev patch series [3/4][4/4]
> - remove the sum latency showing, which makes no sense for debugging, if it
>   is really needed in some case then just do (avg * total) in userland. [4/4]
> - switch {read/write/metadata}_latency_sum to atomic type since it will be
>   readed very time when updating the latencies to calculate the stdev. [4/4]
>
> Changed in V2:
> - switch spin lock to cmpxchg [1/4]
>
> Changed in V3:
> - add the __update_min/max_latency helpers [1/4]
>
>
>
> # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics
> item          total       avg_lat(us)     min_lat(us)     max_lat(us)     stdev(us)
> -----------------------------------------------------------------------------------
> read          2312        9000            1000            100000          607.4
> write         21777       925000          2000            44551000        29700.3
> metadata      6           4179000         1000            21414000        19590.8
>
> item          total           miss            hit
> -------------------------------------------------
> d_lease       2               0               11
> caps          2               14              398418
>
>
>
> Xiubo Li (4):
>   ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches
>   ceph: add min/max latency support for read/write/metadata metrics
>   ceph: move the metric helpers into one separate file
>   ceph: add standard deviation support for read/write/metadata perf
>     metric
>
>  fs/ceph/Makefile     |   2 +-
>  fs/ceph/debugfs.c    |  89 ++++++++++++++++++------
>  fs/ceph/mds_client.c |  83 +---------------------
>  fs/ceph/metric.c     | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/ceph/metric.h     |  79 +++++++++++----------
>  5 files changed, 297 insertions(+), 146 deletions(-)
>  create mode 100644 fs/ceph/metric.c

Hi Xiubo,

I think these additions need to be merged with your previous series,
so that the history is clean.  Ideally the whole thing would start with
a single patch adding all of the metrics infrastructure to metric.[ch],
followed by patches introducing new metrics and ceph_update_*() calls.

Related metrics and ceph_update_*() calls should be added together.
No point in splitting read and write OSD latency in two patches as they
touch the same functions in addr.c and file.c.

Thanks,

                Ilya

Re: [PATCH v4 0/4] ceph: add min/max/stdev latency support

[Xiubo Li]

On 2020/3/18 17:11, Ilya Dryomov wrote:
> On Wed, Mar 18, 2020 at 6:46 AM <xiubli@redhat.com> wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> Changed in V4:
>> - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4]
>> - rebase and combine the stdev patch series [3/4][4/4]
>> - remove the sum latency showing, which makes no sense for debugging, if it
>>    is really needed in some case then just do (avg * total) in userland. [4/4]
>> - switch {read/write/metadata}_latency_sum to atomic type since it will be
>>    readed very time when updating the latencies to calculate the stdev. [4/4]
>>
>> Changed in V2:
>> - switch spin lock to cmpxchg [1/4]
>>
>> Changed in V3:
>> - add the __update_min/max_latency helpers [1/4]
>>
>>
>>
>> # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics
>> item          total       avg_lat(us)     min_lat(us)     max_lat(us)     stdev(us)
>> -----------------------------------------------------------------------------------
>> read          2312        9000            1000            100000          607.4
>> write         21777       925000          2000            44551000        29700.3
>> metadata      6           4179000         1000            21414000        19590.8
>>
>> item          total           miss            hit
>> -------------------------------------------------
>> d_lease       2               0               11
>> caps          2               14              398418
>>
>>
>>
>> Xiubo Li (4):
>>    ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches
>>    ceph: add min/max latency support for read/write/metadata metrics
>>    ceph: move the metric helpers into one separate file
>>    ceph: add standard deviation support for read/write/metadata perf
>>      metric
>>
>>   fs/ceph/Makefile     |   2 +-
>>   fs/ceph/debugfs.c    |  89 ++++++++++++++++++------
>>   fs/ceph/mds_client.c |  83 +---------------------
>>   fs/ceph/metric.c     | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   fs/ceph/metric.h     |  79 +++++++++++----------
>>   5 files changed, 297 insertions(+), 146 deletions(-)
>>   create mode 100644 fs/ceph/metric.c
> Hi Xiubo,
>
> I think these additions need to be merged with your previous series,
> so that the history is clean.  Ideally the whole thing would start with
> a single patch adding all of the metrics infrastructure to metric.[ch],
> followed by patches introducing new metrics and ceph_update_*() calls.
>
> Related metrics and ceph_update_*() calls should be added together.
> No point in splitting read and write OSD latency in two patches as they
> touch the same functions in addr.c and file.c.

Hi Ilya,

Yeah, it makes sense and I will merge all the related patch series about 
the metric and post it again.

Thanks

BRs

> Thanks,
>
>                  Ilya
>

Re: [PATCH v4 0/4] ceph: add min/max/stdev latency support

[Jeff Layton]

On Wed, 2020-03-18 at 18:36 +0800, Xiubo Li wrote:
> On 2020/3/18 17:11, Ilya Dryomov wrote:
> > On Wed, Mar 18, 2020 at 6:46 AM <xiubli@redhat.com> wrote:
> > > From: Xiubo Li <xiubli@redhat.com>
> > > 
> > > Changed in V4:
> > > - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4]
> > > - rebase and combine the stdev patch series [3/4][4/4]
> > > - remove the sum latency showing, which makes no sense for debugging, if it
> > >    is really needed in some case then just do (avg * total) in userland. [4/4]
> > > - switch {read/write/metadata}_latency_sum to atomic type since it will be
> > >    readed very time when updating the latencies to calculate the stdev. [4/4]
> > > 
> > > Changed in V2:
> > > - switch spin lock to cmpxchg [1/4]
> > > 
> > > Changed in V3:
> > > - add the __update_min/max_latency helpers [1/4]
> > > 
> > > 
> > > 
> > > # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics
> > > item          total       avg_lat(us)     min_lat(us)     max_lat(us)     stdev(us)
> > > -----------------------------------------------------------------------------------
> > > read          2312        9000            1000            100000          607.4
> > > write         21777       925000          2000            44551000        29700.3
> > > metadata      6           4179000         1000            21414000        19590.8
> > > 
> > > item          total           miss            hit
> > > -------------------------------------------------
> > > d_lease       2               0               11
> > > caps          2               14              398418
> > > 
> > > 
> > > 
> > > Xiubo Li (4):
> > >    ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches
> > >    ceph: add min/max latency support for read/write/metadata metrics
> > >    ceph: move the metric helpers into one separate file
> > >    ceph: add standard deviation support for read/write/metadata perf
> > >      metric
> > > 
> > >   fs/ceph/Makefile     |   2 +-
> > >   fs/ceph/debugfs.c    |  89 ++++++++++++++++++------
> > >   fs/ceph/mds_client.c |  83 +---------------------
> > >   fs/ceph/metric.c     | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >   fs/ceph/metric.h     |  79 +++++++++++----------
> > >   5 files changed, 297 insertions(+), 146 deletions(-)
> > >   create mode 100644 fs/ceph/metric.c
> > Hi Xiubo,
> > 
> > I think these additions need to be merged with your previous series,
> > so that the history is clean.  Ideally the whole thing would start with
> > a single patch adding all of the metrics infrastructure to metric.[ch],
> > followed by patches introducing new metrics and ceph_update_*() calls.
> > 
> > Related metrics and ceph_update_*() calls should be added together.
> > No point in splitting read and write OSD latency in two patches as they
> > touch the same functions in addr.c and file.c.
> 
> Hi Ilya,
> 
> Yeah, it makes sense and I will merge all the related patch series about 
> the metric and post it again.
> 

Sounds good. I've gone ahead and dropped all of the metrics patches from
the "testing" branch for now. Please resend the whole series and I'll
re-merge them.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v4 0/4] ceph: add min/max/stdev latency support

[Xiubo Li]

On 2020/3/18 18:43, Jeff Layton wrote:
> On Wed, 2020-03-18 at 18:36 +0800, Xiubo Li wrote:
>> On 2020/3/18 17:11, Ilya Dryomov wrote:
>>> On Wed, Mar 18, 2020 at 6:46 AM <xiubli@redhat.com> wrote:
>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>
>>>> Changed in V4:
>>>> - fix the 32-bit arches div errors by using DIV64_U64_ROUND_CLOSEST instead. [1/4]
>>>> - rebase and combine the stdev patch series [3/4][4/4]
>>>> - remove the sum latency showing, which makes no sense for debugging, if it
>>>>     is really needed in some case then just do (avg * total) in userland. [4/4]
>>>> - switch {read/write/metadata}_latency_sum to atomic type since it will be
>>>>     readed very time when updating the latencies to calculate the stdev. [4/4]
>>>>
>>>> Changed in V2:
>>>> - switch spin lock to cmpxchg [1/4]
>>>>
>>>> Changed in V3:
>>>> - add the __update_min/max_latency helpers [1/4]
>>>>
>>>>
>>>>
>>>> # cat /sys/kernel/debug/ceph/0f923fe5-00e6-4866-bf01-2027cb75e94b.client4150/metrics
>>>> item          total       avg_lat(us)     min_lat(us)     max_lat(us)     stdev(us)
>>>> -----------------------------------------------------------------------------------
>>>> read          2312        9000            1000            100000          607.4
>>>> write         21777       925000          2000            44551000        29700.3
>>>> metadata      6           4179000         1000            21414000        19590.8
>>>>
>>>> item          total           miss            hit
>>>> -------------------------------------------------
>>>> d_lease       2               0               11
>>>> caps          2               14              398418
>>>>
>>>>
>>>>
>>>> Xiubo Li (4):
>>>>     ceph: switch to DIV64_U64_ROUND_CLOSEST to support 32-bit arches
>>>>     ceph: add min/max latency support for read/write/metadata metrics
>>>>     ceph: move the metric helpers into one separate file
>>>>     ceph: add standard deviation support for read/write/metadata perf
>>>>       metric
>>>>
>>>>    fs/ceph/Makefile     |   2 +-
>>>>    fs/ceph/debugfs.c    |  89 ++++++++++++++++++------
>>>>    fs/ceph/mds_client.c |  83 +---------------------
>>>>    fs/ceph/metric.c     | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    fs/ceph/metric.h     |  79 +++++++++++----------
>>>>    5 files changed, 297 insertions(+), 146 deletions(-)
>>>>    create mode 100644 fs/ceph/metric.c
>>> Hi Xiubo,
>>>
>>> I think these additions need to be merged with your previous series,
>>> so that the history is clean.  Ideally the whole thing would start with
>>> a single patch adding all of the metrics infrastructure to metric.[ch],
>>> followed by patches introducing new metrics and ceph_update_*() calls.
>>>
>>> Related metrics and ceph_update_*() calls should be added together.
>>> No point in splitting read and write OSD latency in two patches as they
>>> touch the same functions in addr.c and file.c.
>> Hi Ilya,
>>
>> Yeah, it makes sense and I will merge all the related patch series about
>> the metric and post it again.
>>
> Sounds good. I've gone ahead and dropped all of the metrics patches from
> the "testing" branch for now. Please resend the whole series and I'll
> re-merge them.

Sure, thanks Jeff.

BRs


> Thanks,