| Message ID | 20210914084902.1618064-1-vshankar@redhat.com |
|---|---|
| Headers | show |
| Series | ceph: forward average read/write/metadata latency | expand |
On 9/14/21 4:49 PM, Venky Shankar wrote: > The use of `jiffies_to_timespec64()` seems incorrect too, switch > that to `ktime_to_timespec64()`. I think this has been missed after I switched the jeffies to ktime for the r_start and r_end in my previous patch set. This LGTM :-) > Signed-off-by: Venky Shankar <vshankar@redhat.com> > --- > fs/ceph/metric.c | 35 +++++++++++++++++++---------------- > fs/ceph/metric.h | 48 +++++++++++++++++++++++++++++++++--------------- > 2 files changed, 52 insertions(+), 31 deletions(-) > > diff --git a/fs/ceph/metric.c b/fs/ceph/metric.c > index 6b774b1a88ce..78a50bb7bd0f 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) > { > @@ -26,7 +33,6 @@ static bool ceph_mdsc_send_metrics(struct ceph_mds_client *mdsc, > u64 nr_caps = atomic64_read(&m->total_caps); > u32 header_len = sizeof(struct ceph_metric_header); > struct ceph_msg *msg; > - struct timespec64 ts; > s64 sum; > s32 items = 0; > s32 len; > @@ -59,37 +65,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->header.type = cpu_to_le32(CLIENT_METRIC_TYPE_READ_LATENCY); > - read->header.ver = 1; > + read->header.ver = 2; > read->header.compat = 1; > read->header.data_len = cpu_to_le32(sizeof(*read) - header_len); > - 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->header.type = cpu_to_le32(CLIENT_METRIC_TYPE_WRITE_LATENCY); > - write->header.ver = 1; > + write->header.ver = 2; > write->header.compat = 1; > write->header.data_len = cpu_to_le32(sizeof(*write) - header_len); > - 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->header.type = cpu_to_le32(CLIENT_METRIC_TYPE_METADATA_LATENCY); > - meta->header.ver = 1; > + meta->header.ver = 2; > meta->header.compat = 1; > meta->header.data_len = cpu_to_le32(sizeof(*meta) - header_len); > - 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 a5da21b8f8ed..2dd506dedebf 100644 > --- a/fs/ceph/metric.h > +++ b/fs/ceph/metric.h > @@ -19,27 +19,39 @@ enum ceph_metric_type { > CLIENT_METRIC_TYPE_OPENED_INODES, > CLIENT_METRIC_TYPE_READ_IO_SIZES, > CLIENT_METRIC_TYPE_WRITE_IO_SIZES, > - > - CLIENT_METRIC_TYPE_MAX = CLIENT_METRIC_TYPE_WRITE_IO_SIZES, > + 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_OPENED_FILES, \ > - CLIENT_METRIC_TYPE_PINNED_ICAPS, \ > - CLIENT_METRIC_TYPE_OPENED_INODES, \ > - CLIENT_METRIC_TYPE_READ_IO_SIZES, \ > - CLIENT_METRIC_TYPE_WRITE_IO_SIZES, \ > - \ > - 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_OPENED_FILES, \ > + CLIENT_METRIC_TYPE_PINNED_ICAPS, \ > + CLIENT_METRIC_TYPE_OPENED_INODES, \ > + CLIENT_METRIC_TYPE_READ_IO_SIZES, \ > + CLIENT_METRIC_TYPE_WRITE_IO_SIZES, \ > + 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, \ > } > > struct ceph_metric_header { > @@ -61,18 +73,24 @@ struct ceph_metric_cap { > struct ceph_metric_read_latency { > struct ceph_metric_header header; > struct ceph_timespec lat; > + struct ceph_timespec avg; > + struct ceph_timespec stdev; > } __packed; > > /* metric write latency header */ > struct ceph_metric_write_latency { > struct ceph_metric_header header; > struct ceph_timespec lat; > + struct ceph_timespec avg; > + struct ceph_timespec stdev; > } __packed; > > /* metric metadata latency header */ > struct ceph_metric_metadata_latency { > struct ceph_metric_header header; > struct ceph_timespec lat; > + struct ceph_timespec avg; > + struct ceph_timespec stdev; > } __packed; > > /* metric dentry lease header */
v2: - based on top of ceph-client/testing branch 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 | 20 ++++----- fs/ceph/metric.c | 105 ++++++++++++++++++++++------------------------ fs/ceph/metric.h | 68 +++++++++++++++++++----------- 3 files changed, 104 insertions(+), 89 deletions(-)