[PATCH v1 0/4] ceph: forward average read/write/metadata latency

[PATCH v1 0/4] ceph: forward average read/write/metadata latency

[Venky Shankar]

Right now, cumulative read/write/metadata latencies are tracked
and are periodically forwarded to the MDS. These meterics are not
particularly useful. A much more useful metric is the average latency
and standard deviation (stdev) which is what this series of patches
aims to do.

The userspace (libcephfs+tool) changes are here::

          https://github.com/ceph/ceph/pull/41397

The math involved in keeping track of the average latency and stdev
seems incorrect, so, this series fixes that up too (closely mimics
how its done in userspace with some restrictions obviously) as per::

          NEW_AVG = OLD_AVG + ((latency - OLD_AVG) / total_ops)
          NEW_STDEV = SQRT(((OLD_STDEV + (latency - OLD_AVG)*(latency - NEW_AVG)) / (total_ops - 1)))

Note that the cumulative latencies are still forwarded to the MDS but
the tool (cephfs-top) ignores it altogether.

Venky Shankar (4):
  ceph: use "struct ceph_timespec" for r/w/m latencies
  ceph: track average/stdev r/w/m latency
  ceph: include average/stddev r/w/m latency in mds metrics
  ceph: use tracked average r/w/m latencies to display metrics in
    debugfs

 fs/ceph/debugfs.c | 12 +++----
 fs/ceph/metric.c  | 81 +++++++++++++++++++++++++----------------------
 fs/ceph/metric.h  | 64 +++++++++++++++++++++++--------------
 3 files changed, 90 insertions(+), 67 deletions(-)

-- 
2.27.0

[PATCH v1 1/4] ceph: use "struct ceph_timespec" for r/w/m latencies

[Venky Shankar]

Signed-off-by: Venky Shankar <vshankar@redhat.com>
---
 fs/ceph/metric.c | 12 ++++++------
 fs/ceph/metric.h | 17 +++++++----------
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/fs/ceph/metric.c b/fs/ceph/metric.c
index 5ec94bd4c1de..19bca20cf7e1 100644
--- a/fs/ceph/metric.c
+++ b/fs/ceph/metric.c
@@ -56,8 +56,8 @@ static bool ceph_mdsc_send_metrics(struct ceph_mds_client *mdsc,
 	read->data_len = cpu_to_le32(sizeof(*read) - 10);
 	sum = m->read_latency_sum;
 	jiffies_to_timespec64(sum, &ts);
-	read->sec = cpu_to_le32(ts.tv_sec);
-	read->nsec = cpu_to_le32(ts.tv_nsec);
+	read->lat.tv_sec = cpu_to_le32(ts.tv_sec);
+	read->lat.tv_nsec = cpu_to_le32(ts.tv_nsec);
 	items++;
 
 	/* encode the write latency metric */
@@ -68,8 +68,8 @@ static bool ceph_mdsc_send_metrics(struct ceph_mds_client *mdsc,
 	write->data_len = cpu_to_le32(sizeof(*write) - 10);
 	sum = m->write_latency_sum;
 	jiffies_to_timespec64(sum, &ts);
-	write->sec = cpu_to_le32(ts.tv_sec);
-	write->nsec = cpu_to_le32(ts.tv_nsec);
+	write->lat.tv_sec = cpu_to_le32(ts.tv_sec);
+	write->lat.tv_nsec = cpu_to_le32(ts.tv_nsec);
 	items++;
 
 	/* encode the metadata latency metric */
@@ -80,8 +80,8 @@ static bool ceph_mdsc_send_metrics(struct ceph_mds_client *mdsc,
 	meta->data_len = cpu_to_le32(sizeof(*meta) - 10);
 	sum = m->metadata_latency_sum;
 	jiffies_to_timespec64(sum, &ts);
-	meta->sec = cpu_to_le32(ts.tv_sec);
-	meta->nsec = cpu_to_le32(ts.tv_nsec);
+	meta->lat.tv_sec = cpu_to_le32(ts.tv_sec);
+	meta->lat.tv_nsec = cpu_to_le32(ts.tv_nsec);
 	items++;
 
 	/* encode the dentry lease metric */
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index af6038ff39d4..1151d64dbcbc 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -2,7 +2,7 @@
 #ifndef _FS_CEPH_MDS_METRIC_H
 #define _FS_CEPH_MDS_METRIC_H
 
-#include <linux/types.h>
+#include <linux/ceph/types.h>
 #include <linux/percpu_counter.h>
 #include <linux/ktime.h>
 
@@ -52,9 +52,8 @@ struct ceph_metric_read_latency {
 	__u8  ver;
 	__u8  compat;
 
-	__le32 data_len; /* length of sizeof(sec + nsec) */
-	__le32 sec;
-	__le32 nsec;
+	__le32 data_len; /* length of sizeof(lat) */
+	struct ceph_timespec lat;
 } __packed;
 
 /* metric write latency header */
@@ -64,9 +63,8 @@ struct ceph_metric_write_latency {
 	__u8  ver;
 	__u8  compat;
 
-	__le32 data_len; /* length of sizeof(sec + nsec) */
-	__le32 sec;
-	__le32 nsec;
+	__le32 data_len; /* length of sizeof(lat) */
+	struct ceph_timespec lat;
 } __packed;
 
 /* metric metadata latency header */
@@ -76,9 +74,8 @@ struct ceph_metric_metadata_latency {
 	__u8  ver;
 	__u8  compat;
 
-	__le32 data_len; /* length of sizeof(sec + nsec) */
-	__le32 sec;
-	__le32 nsec;
+	__le32 data_len; /* length of sizeof(lat) */
+	struct ceph_timespec lat;
 } __packed;
 
 /* metric dentry lease header */
-- 
2.27.0

[PATCH v1 2/4] ceph: track average/stdev r/w/m latency

[Venky Shankar]

The math involved in tracking average and standard deviation
for r/w/m latencies looks incorrect. Fix that up. Also, change
the variable name that tracks standard deviation (*_sq_sum) to
*_stdev.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
---
 fs/ceph/debugfs.c |  6 +++---
 fs/ceph/metric.c  | 45 ++++++++++++++++++++++++---------------------
 fs/ceph/metric.h  |  9 ++++++---
 3 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index f9ff70704423..9153bc233e08 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -176,7 +176,7 @@ static int metric_show(struct seq_file *s, void *p)
 	avg = total > 0 ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
 	min = m->read_latency_min;
 	max = m->read_latency_max;
-	sq = m->read_latency_sq_sum;
+	sq = m->read_latency_stdev;
 	spin_unlock(&m->read_latency_lock);
 	CEPH_METRIC_SHOW("read", total, avg, min, max, sq);
 
@@ -186,7 +186,7 @@ static int metric_show(struct seq_file *s, void *p)
 	avg = total > 0 ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
 	min = m->write_latency_min;
 	max = m->write_latency_max;
-	sq = m->write_latency_sq_sum;
+	sq = m->write_latency_stdev;
 	spin_unlock(&m->write_latency_lock);
 	CEPH_METRIC_SHOW("write", total, avg, min, max, sq);
 
@@ -196,7 +196,7 @@ static int metric_show(struct seq_file *s, void *p)
 	avg = total > 0 ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
 	min = m->metadata_latency_min;
 	max = m->metadata_latency_max;
-	sq = m->metadata_latency_sq_sum;
+	sq = m->metadata_latency_stdev;
 	spin_unlock(&m->metadata_latency_lock);
 	CEPH_METRIC_SHOW("metadata", total, avg, min, max, sq);
 
diff --git a/fs/ceph/metric.c b/fs/ceph/metric.c
index 19bca20cf7e1..e00b1c2ce25b 100644
--- a/fs/ceph/metric.c
+++ b/fs/ceph/metric.c
@@ -184,21 +184,24 @@ int ceph_metric_init(struct ceph_client_metric *m)
 		goto err_i_caps_mis;
 
 	spin_lock_init(&m->read_latency_lock);
-	m->read_latency_sq_sum = 0;
+	m->read_latency_stdev = 0;
+	m->avg_read_latency = 0;
 	m->read_latency_min = KTIME_MAX;
 	m->read_latency_max = 0;
 	m->total_reads = 0;
 	m->read_latency_sum = 0;
 
 	spin_lock_init(&m->write_latency_lock);
-	m->write_latency_sq_sum = 0;
+	m->write_latency_stdev = 0;
+	m->avg_write_latency = 0;
 	m->write_latency_min = KTIME_MAX;
 	m->write_latency_max = 0;
 	m->total_writes = 0;
 	m->write_latency_sum = 0;
 
 	spin_lock_init(&m->metadata_latency_lock);
-	m->metadata_latency_sq_sum = 0;
+	m->metadata_latency_stdev = 0;
+	m->avg_metadata_latency = 0;
 	m->metadata_latency_min = KTIME_MAX;
 	m->metadata_latency_max = 0;
 	m->total_metadatas = 0;
@@ -250,10 +253,10 @@ void ceph_metric_destroy(struct ceph_client_metric *m)
 }
 
 static inline void __update_latency(ktime_t *totalp, ktime_t *lsump,
-				    ktime_t *min, ktime_t *max,
-				    ktime_t *sq_sump, ktime_t lat)
+				    ktime_t *lavgp, ktime_t *min, ktime_t *max,
+				    ktime_t *lstdev, ktime_t lat)
 {
-	ktime_t total, avg, sq, lsum;
+	ktime_t total, avg, stdev, lsum;
 
 	total = ++(*totalp);
 	lsum = (*lsump += lat);
@@ -263,15 +266,15 @@ static inline void __update_latency(ktime_t *totalp, ktime_t *lsump,
 	if (unlikely(lat > *max))
 		*max = lat;
 
-	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);
-	*sq_sump += sq;
+	if (unlikely(total == 1)) {
+		*lavgp = lat;
+		*lstdev = 0;
+	} else {
+		avg = *lavgp + div64_s64(lat - *lavgp, total);
+		stdev = *lstdev + (lat - *lavgp)*(lat - avg);
+		*lstdev = int_sqrt(div64_u64(stdev, total - 1));
+		*lavgp = avg;
+	}
 }
 
 void ceph_update_read_latency(struct ceph_client_metric *m,
@@ -285,8 +288,8 @@ void ceph_update_read_latency(struct ceph_client_metric *m,
 
 	spin_lock(&m->read_latency_lock);
 	__update_latency(&m->total_reads, &m->read_latency_sum,
-			 &m->read_latency_min, &m->read_latency_max,
-			 &m->read_latency_sq_sum, lat);
+			 &m->avg_read_latency, &m->read_latency_min,
+			 &m->read_latency_max, &m->read_latency_stdev, lat);
 	spin_unlock(&m->read_latency_lock);
 }
 
@@ -301,8 +304,8 @@ void ceph_update_write_latency(struct ceph_client_metric *m,
 
 	spin_lock(&m->write_latency_lock);
 	__update_latency(&m->total_writes, &m->write_latency_sum,
-			 &m->write_latency_min, &m->write_latency_max,
-			 &m->write_latency_sq_sum, lat);
+			 &m->avg_write_latency, &m->write_latency_min,
+			 &m->write_latency_max, &m->write_latency_stdev, lat);
 	spin_unlock(&m->write_latency_lock);
 }
 
@@ -317,7 +320,7 @@ void ceph_update_metadata_latency(struct ceph_client_metric *m,
 
 	spin_lock(&m->metadata_latency_lock);
 	__update_latency(&m->total_metadatas, &m->metadata_latency_sum,
-			 &m->metadata_latency_min, &m->metadata_latency_max,
-			 &m->metadata_latency_sq_sum, lat);
+			 &m->avg_metadata_latency, &m->metadata_latency_min,
+			 &m->metadata_latency_max, &m->metadata_latency_stdev, lat);
 	spin_unlock(&m->metadata_latency_lock);
 }
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index 1151d64dbcbc..136689fa0aec 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -108,21 +108,24 @@ struct ceph_client_metric {
 	spinlock_t read_latency_lock;
 	u64 total_reads;
 	ktime_t read_latency_sum;
-	ktime_t read_latency_sq_sum;
+	ktime_t avg_read_latency;
+	ktime_t read_latency_stdev;
 	ktime_t read_latency_min;
 	ktime_t read_latency_max;
 
 	spinlock_t write_latency_lock;
 	u64 total_writes;
 	ktime_t write_latency_sum;
-	ktime_t write_latency_sq_sum;
+	ktime_t avg_write_latency;
+	ktime_t write_latency_stdev;
 	ktime_t write_latency_min;
 	ktime_t write_latency_max;
 
 	spinlock_t metadata_latency_lock;
 	u64 total_metadatas;
 	ktime_t metadata_latency_sum;
-	ktime_t metadata_latency_sq_sum;
+	ktime_t avg_metadata_latency;
+	ktime_t metadata_latency_stdev;
 	ktime_t metadata_latency_min;
 	ktime_t metadata_latency_max;
 
-- 
2.27.0

[PATCH v1 3/4] ceph: include average/stddev r/w/m latency in mds metrics

[Venky Shankar]

The use of `jiffies_to_timespec64()` seems incorrect too, switch
that to `ktime_to_timespec64()`.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
---
 fs/ceph/metric.c | 36 +++++++++++++++++++-----------------
 fs/ceph/metric.h | 44 +++++++++++++++++++++++++++++++-------------
 2 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/fs/ceph/metric.c b/fs/ceph/metric.c
index e00b1c2ce25b..cb332ae2411f 100644
--- a/fs/ceph/metric.c
+++ b/fs/ceph/metric.c
@@ -8,6 +8,13 @@
 #include "metric.h"
 #include "mds_client.h"
 
+static void to_ceph_timespec(struct ceph_timespec *ts, ktime_t val)
+{
+	struct timespec64 t = ktime_to_timespec64(val);
+	ts->tv_sec = cpu_to_le32(t.tv_sec);
+	ts->tv_nsec = cpu_to_le32(t.tv_nsec);
+}
+
 static bool ceph_mdsc_send_metrics(struct ceph_mds_client *mdsc,
 				   struct ceph_mds_session *s)
 {
@@ -20,8 +27,6 @@ static bool ceph_mdsc_send_metrics(struct ceph_mds_client *mdsc,
 	struct ceph_client_metric *m = &mdsc->metric;
 	u64 nr_caps = atomic64_read(&m->total_caps);
 	struct ceph_msg *msg;
-	struct timespec64 ts;
-	s64 sum;
 	s32 items = 0;
 	s32 len;
 
@@ -51,37 +56,34 @@ static bool ceph_mdsc_send_metrics(struct ceph_mds_client *mdsc,
 	/* encode the read latency metric */
 	read = (struct ceph_metric_read_latency *)(cap + 1);
 	read->type = cpu_to_le32(CLIENT_METRIC_TYPE_READ_LATENCY);
-	read->ver = 1;
+	read->ver = 2;
 	read->compat = 1;
 	read->data_len = cpu_to_le32(sizeof(*read) - 10);
-	sum = m->read_latency_sum;
-	jiffies_to_timespec64(sum, &ts);
-	read->lat.tv_sec = cpu_to_le32(ts.tv_sec);
-	read->lat.tv_nsec = cpu_to_le32(ts.tv_nsec);
+	to_ceph_timespec(&read->lat, m->read_latency_sum);
+	to_ceph_timespec(&read->avg, m->avg_read_latency);
+	to_ceph_timespec(&read->stdev, m->read_latency_stdev);
 	items++;
 
 	/* encode the write latency metric */
 	write = (struct ceph_metric_write_latency *)(read + 1);
 	write->type = cpu_to_le32(CLIENT_METRIC_TYPE_WRITE_LATENCY);
-	write->ver = 1;
+	write->ver = 2;
 	write->compat = 1;
 	write->data_len = cpu_to_le32(sizeof(*write) - 10);
-	sum = m->write_latency_sum;
-	jiffies_to_timespec64(sum, &ts);
-	write->lat.tv_sec = cpu_to_le32(ts.tv_sec);
-	write->lat.tv_nsec = cpu_to_le32(ts.tv_nsec);
+	to_ceph_timespec(&write->lat, m->write_latency_sum);
+	to_ceph_timespec(&write->avg, m->avg_write_latency);
+	to_ceph_timespec(&write->stdev, m->write_latency_stdev);
 	items++;
 
 	/* encode the metadata latency metric */
 	meta = (struct ceph_metric_metadata_latency *)(write + 1);
 	meta->type = cpu_to_le32(CLIENT_METRIC_TYPE_METADATA_LATENCY);
-	meta->ver = 1;
+	meta->ver = 2;
 	meta->compat = 1;
 	meta->data_len = cpu_to_le32(sizeof(*meta) - 10);
-	sum = m->metadata_latency_sum;
-	jiffies_to_timespec64(sum, &ts);
-	meta->lat.tv_sec = cpu_to_le32(ts.tv_sec);
-	meta->lat.tv_nsec = cpu_to_le32(ts.tv_nsec);
+	to_ceph_timespec(&meta->lat, m->metadata_latency_sum);
+	to_ceph_timespec(&meta->avg, m->avg_metadata_latency);
+	to_ceph_timespec(&meta->stdev, m->metadata_latency_stdev);
 	items++;
 
 	/* encode the dentry lease metric */
diff --git a/fs/ceph/metric.h b/fs/ceph/metric.h
index 136689fa0aec..b68927378e59 100644
--- a/fs/ceph/metric.h
+++ b/fs/ceph/metric.h
@@ -14,22 +14,34 @@ enum ceph_metric_type {
 	CLIENT_METRIC_TYPE_WRITE_LATENCY,
 	CLIENT_METRIC_TYPE_METADATA_LATENCY,
 	CLIENT_METRIC_TYPE_DENTRY_LEASE,
-
-	CLIENT_METRIC_TYPE_MAX = CLIENT_METRIC_TYPE_DENTRY_LEASE,
+	CLIENT_METRIC_TYPE_AVG_READ_LATENCY,
+	CLIENT_METRIC_TYPE_STDEV_READ_LATENCY,
+	CLIENT_METRIC_TYPE_AVG_WRITE_LATENCY,
+	CLIENT_METRIC_TYPE_STDEV_WRITE_LATENCY,
+	CLIENT_METRIC_TYPE_AVG_METADATA_LATENCY,
+	CLIENT_METRIC_TYPE_STDEV_METADATA_LATENCY,
+
+	CLIENT_METRIC_TYPE_MAX = CLIENT_METRIC_TYPE_STDEV_METADATA_LATENCY,
 };
 
 /*
  * This will always have the highest metric bit value
  * as the last element of the array.
  */
-#define CEPHFS_METRIC_SPEC_CLIENT_SUPPORTED {	\
-	CLIENT_METRIC_TYPE_CAP_INFO,		\
-	CLIENT_METRIC_TYPE_READ_LATENCY,	\
-	CLIENT_METRIC_TYPE_WRITE_LATENCY,	\
-	CLIENT_METRIC_TYPE_METADATA_LATENCY,	\
-	CLIENT_METRIC_TYPE_DENTRY_LEASE,	\
-						\
-	CLIENT_METRIC_TYPE_MAX,			\
+#define CEPHFS_METRIC_SPEC_CLIENT_SUPPORTED {	   \
+	CLIENT_METRIC_TYPE_CAP_INFO,		   \
+	CLIENT_METRIC_TYPE_READ_LATENCY,	   \
+	CLIENT_METRIC_TYPE_WRITE_LATENCY,	   \
+	CLIENT_METRIC_TYPE_METADATA_LATENCY,	   \
+	CLIENT_METRIC_TYPE_DENTRY_LEASE,	   \
+	CLIENT_METRIC_TYPE_AVG_READ_LATENCY,	   \
+	CLIENT_METRIC_TYPE_STDEV_READ_LATENCY,	   \
+	CLIENT_METRIC_TYPE_AVG_WRITE_LATENCY,	   \
+	CLIENT_METRIC_TYPE_STDEV_WRITE_LATENCY,	   \
+	CLIENT_METRIC_TYPE_AVG_METADATA_LATENCY,   \
+	CLIENT_METRIC_TYPE_STDEV_METADATA_LATENCY, \
+						   \
+	CLIENT_METRIC_TYPE_MAX,			   \
 }
 
 /* metric caps header */
@@ -52,8 +64,10 @@ struct ceph_metric_read_latency {
 	__u8  ver;
 	__u8  compat;
 
-	__le32 data_len; /* length of sizeof(lat) */
+	__le32 data_len; /* length of sizeof(lat+avg+stdev) */
 	struct ceph_timespec lat;
+	struct ceph_timespec avg;
+	struct ceph_timespec stdev;
 } __packed;
 
 /* metric write latency header */
@@ -63,8 +77,10 @@ struct ceph_metric_write_latency {
 	__u8  ver;
 	__u8  compat;
 
-	__le32 data_len; /* length of sizeof(lat) */
+	__le32 data_len; /* length of sizeof(lat_avg_stdev) */
 	struct ceph_timespec lat;
+	struct ceph_timespec avg;
+	struct ceph_timespec stdev;
 } __packed;
 
 /* metric metadata latency header */
@@ -74,8 +90,10 @@ struct ceph_metric_metadata_latency {
 	__u8  ver;
 	__u8  compat;
 
-	__le32 data_len; /* length of sizeof(lat) */
+	__le32 data_len; /* length of sizeof(lat+avg+stdev) */
 	struct ceph_timespec lat;
+	struct ceph_timespec avg;
+	struct ceph_timespec stdev;
 } __packed;
 
 /* metric dentry lease header */
-- 
2.27.0

[PATCH v1 4/4] ceph: use tracked average r/w/m latencies to display metrics in debugfs

[Venky Shankar]

Signed-off-by: Venky Shankar <vshankar@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 9153bc233e08..e3fccf7c6fb5 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -173,7 +173,7 @@ static int metric_show(struct seq_file *s, void *p)
 	spin_lock(&m->read_latency_lock);
 	total = m->total_reads;
 	sum = m->read_latency_sum;
-	avg = total > 0 ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
+	avg = m->avg_read_latency;
 	min = m->read_latency_min;
 	max = m->read_latency_max;
 	sq = m->read_latency_stdev;
@@ -183,7 +183,7 @@ static int metric_show(struct seq_file *s, void *p)
 	spin_lock(&m->write_latency_lock);
 	total = m->total_writes;
 	sum = m->write_latency_sum;
-	avg = total > 0 ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
+	avg = m->avg_write_latency;
 	min = m->write_latency_min;
 	max = m->write_latency_max;
 	sq = m->write_latency_stdev;
@@ -193,7 +193,7 @@ static int metric_show(struct seq_file *s, void *p)
 	spin_lock(&m->metadata_latency_lock);
 	total = m->total_metadatas;
 	sum = m->metadata_latency_sum;
-	avg = total > 0 ? DIV64_U64_ROUND_CLOSEST(sum, total) : 0;
+	avg = m->avg_metadata_latency;
 	min = m->metadata_latency_min;
 	max = m->metadata_latency_max;
 	sq = m->metadata_latency_stdev;
-- 
2.27.0

Re: [PATCH v1 0/4] ceph: forward average read/write/metadata latency

[Jeff Layton]

On Mon, 2021-09-13 at 18:43 +0530, Venky Shankar wrote:
> Right now, cumulative read/write/metadata latencies are tracked
> and are periodically forwarded to the MDS. These meterics are not
> particularly useful. A much more useful metric is the average latency
> and standard deviation (stdev) which is what this series of patches
> aims to do.
> 
> The userspace (libcephfs+tool) changes are here::
> 
>           https://github.com/ceph/ceph/pull/41397
> 
> The math involved in keeping track of the average latency and stdev
> seems incorrect, so, this series fixes that up too (closely mimics
> how its done in userspace with some restrictions obviously) as per::
> 
>           NEW_AVG = OLD_AVG + ((latency - OLD_AVG) / total_ops)
>           NEW_STDEV = SQRT(((OLD_STDEV + (latency - OLD_AVG)*(latency - NEW_AVG)) / (total_ops - 1)))
> 
> Note that the cumulative latencies are still forwarded to the MDS but
> the tool (cephfs-top) ignores it altogether.
> 
> Venky Shankar (4):
>   ceph: use "struct ceph_timespec" for r/w/m latencies
>   ceph: track average/stdev r/w/m latency
>   ceph: include average/stddev r/w/m latency in mds metrics
>   ceph: use tracked average r/w/m latencies to display metrics in
>     debugfs
> 
>  fs/ceph/debugfs.c | 12 +++----
>  fs/ceph/metric.c  | 81 +++++++++++++++++++++++++----------------------
>  fs/ceph/metric.h  | 64 +++++++++++++++++++++++--------------
>  3 files changed, 90 insertions(+), 67 deletions(-)
> 

This looks reasonably sane. I'll plan to go ahead and pull this into the
testing kernels and do some testing with them. If anyone has objections
(Xiubo?) let me know and I can take them out.

Thanks,
-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH v1 0/4] ceph: forward average read/write/metadata latency

[Jeff Layton]

On Mon, 2021-09-13 at 11:13 -0400, Jeff Layton wrote:
> On Mon, 2021-09-13 at 18:43 +0530, Venky Shankar wrote:
> > Right now, cumulative read/write/metadata latencies are tracked
> > and are periodically forwarded to the MDS. These meterics are not
> > particularly useful. A much more useful metric is the average latency
> > and standard deviation (stdev) which is what this series of patches
> > aims to do.
> > 
> > The userspace (libcephfs+tool) changes are here::
> > 
> >           https://github.com/ceph/ceph/pull/41397
> > 
> > The math involved in keeping track of the average latency and stdev
> > seems incorrect, so, this series fixes that up too (closely mimics
> > how its done in userspace with some restrictions obviously) as per::
> > 
> >           NEW_AVG = OLD_AVG + ((latency - OLD_AVG) / total_ops)
> >           NEW_STDEV = SQRT(((OLD_STDEV + (latency - OLD_AVG)*(latency - NEW_AVG)) / (total_ops - 1)))
> > 
> > Note that the cumulative latencies are still forwarded to the MDS but
> > the tool (cephfs-top) ignores it altogether.
> > 
> > Venky Shankar (4):
> >   ceph: use "struct ceph_timespec" for r/w/m latencies
> >   ceph: track average/stdev r/w/m latency
> >   ceph: include average/stddev r/w/m latency in mds metrics
> >   ceph: use tracked average r/w/m latencies to display metrics in
> >     debugfs
> > 
> >  fs/ceph/debugfs.c | 12 +++----
> >  fs/ceph/metric.c  | 81 +++++++++++++++++++++++++----------------------
> >  fs/ceph/metric.h  | 64 +++++++++++++++++++++++--------------
> >  3 files changed, 90 insertions(+), 67 deletions(-)
> > 
> 
> This looks reasonably sane. I'll plan to go ahead and pull this into the
> testing kernels and do some testing with them. If anyone has objections
> (Xiubo?) let me know and I can take them out.
> 
> Thanks,

Hmm...I take it back. There are some non-trivial merge conflicts in this
series vs. the current testing branch. Venky can you rebase this onto
the ceph-client/testing branch and resubmit?

Thanks,
-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH v1 0/4] ceph: forward average read/write/metadata latency

[Venky Shankar]

On Mon, Sep 13, 2021 at 8:51 PM Jeff Layton <jlayton@redhat.com> wrote:
>
> On Mon, 2021-09-13 at 11:13 -0400, Jeff Layton wrote:
> > On Mon, 2021-09-13 at 18:43 +0530, Venky Shankar wrote:
> > > Right now, cumulative read/write/metadata latencies are tracked
> > > and are periodically forwarded to the MDS. These meterics are not
> > > particularly useful. A much more useful metric is the average latency
> > > and standard deviation (stdev) which is what this series of patches
> > > aims to do.
> > >
> > > The userspace (libcephfs+tool) changes are here::
> > >
> > >           https://github.com/ceph/ceph/pull/41397
> > >
> > > The math involved in keeping track of the average latency and stdev
> > > seems incorrect, so, this series fixes that up too (closely mimics
> > > how its done in userspace with some restrictions obviously) as per::
> > >
> > >           NEW_AVG = OLD_AVG + ((latency - OLD_AVG) / total_ops)
> > >           NEW_STDEV = SQRT(((OLD_STDEV + (latency - OLD_AVG)*(latency - NEW_AVG)) / (total_ops - 1)))
> > >
> > > Note that the cumulative latencies are still forwarded to the MDS but
> > > the tool (cephfs-top) ignores it altogether.
> > >
> > > Venky Shankar (4):
> > >   ceph: use "struct ceph_timespec" for r/w/m latencies
> > >   ceph: track average/stdev r/w/m latency
> > >   ceph: include average/stddev r/w/m latency in mds metrics
> > >   ceph: use tracked average r/w/m latencies to display metrics in
> > >     debugfs
> > >
> > >  fs/ceph/debugfs.c | 12 +++----
> > >  fs/ceph/metric.c  | 81 +++++++++++++++++++++++++----------------------
> > >  fs/ceph/metric.h  | 64 +++++++++++++++++++++++--------------
> > >  3 files changed, 90 insertions(+), 67 deletions(-)
> > >
> >
> > This looks reasonably sane. I'll plan to go ahead and pull this into the
> > testing kernels and do some testing with them. If anyone has objections
> > (Xiubo?) let me know and I can take them out.
> >
> > Thanks,
>
> Hmm...I take it back. There are some non-trivial merge conflicts in this
> series vs. the current testing branch. Venky can you rebase this onto
> the ceph-client/testing branch and resubmit?

Sure, will do.

>
> Thanks,
> --
> Jeff Layton <jlayton@redhat.com>
>


-- 
Cheers,
Venky

Re: [PATCH v1 0/4] ceph: forward average read/write/metadata latency

[Xiubo Li]

On 9/13/21 11:13 PM, Jeff Layton wrote:
> On Mon, 2021-09-13 at 18:43 +0530, Venky Shankar wrote:
>> Right now, cumulative read/write/metadata latencies are tracked
>> and are periodically forwarded to the MDS. These meterics are not
>> particularly useful. A much more useful metric is the average latency
>> and standard deviation (stdev) which is what this series of patches
>> aims to do.
>>
>> The userspace (libcephfs+tool) changes are here::
>>
>>            https://github.com/ceph/ceph/pull/41397
>>
>> The math involved in keeping track of the average latency and stdev
>> seems incorrect, so, this series fixes that up too (closely mimics
>> how its done in userspace with some restrictions obviously) as per::
>>
>>            NEW_AVG = OLD_AVG + ((latency - OLD_AVG) / total_ops)
>>            NEW_STDEV = SQRT(((OLD_STDEV + (latency - OLD_AVG)*(latency - NEW_AVG)) / (total_ops - 1)))
>>
>> Note that the cumulative latencies are still forwarded to the MDS but
>> the tool (cephfs-top) ignores it altogether.
>>
>> Venky Shankar (4):
>>    ceph: use "struct ceph_timespec" for r/w/m latencies
>>    ceph: track average/stdev r/w/m latency
>>    ceph: include average/stddev r/w/m latency in mds metrics
>>    ceph: use tracked average r/w/m latencies to display metrics in
>>      debugfs
>>
>>   fs/ceph/debugfs.c | 12 +++----
>>   fs/ceph/metric.c  | 81 +++++++++++++++++++++++++----------------------
>>   fs/ceph/metric.h  | 64 +++++++++++++++++++++++--------------
>>   3 files changed, 90 insertions(+), 67 deletions(-)
>>
> This looks reasonably sane. I'll plan to go ahead and pull this into the
> testing kernels and do some testing with them. If anyone has objections
> (Xiubo?) let me know and I can take them out.

Sorry I think I missed this patch set.

Comment it in the V2.

Thanks.


>
> Thanks,