On Mon, Feb 10, 2020 at 6:34 AM <xiubli@xxxxxxxxxx> wrote: > > From: Xiubo Li <xiubli@xxxxxxxxxx> > > Sometimes we need to discard the old perf metrics and start to get > new ones. And this will reset the most metric counters, except the > total numbers for caps and dentries. > > URL: https://tracker.ceph.com/issues/43215 > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > --- > fs/ceph/debugfs.c | 38 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c > index 60f3e307fca1..6e595a37af5d 100644 > --- a/fs/ceph/debugfs.c > +++ b/fs/ceph/debugfs.c > @@ -179,6 +179,43 @@ static int metric_show(struct seq_file *s, void *p) > return 0; > } > > +static ssize_t metric_store(struct file *file, const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct seq_file *s = file->private_data; > + struct ceph_fs_client *fsc = s->private; > + struct ceph_mds_client *mdsc = fsc->mdsc; > + struct ceph_client_metric *metric = &mdsc->metric; > + char buf[8]; > + > + if (copy_from_user(buf, user_buf, 8)) > + return -EFAULT; > + > + if (strncmp(buf, "reset", strlen("reset"))) { > + pr_err("Invalid set value '%s', only 'reset' is valid\n", buf); > + return -EINVAL; > + } Hi Xiubo, Why strncmp? How does this handle inputs like "resetfoobar"? > + > + percpu_counter_set(&metric->d_lease_hit, 0); > + percpu_counter_set(&metric->d_lease_mis, 0); > + > + percpu_counter_set(&metric->i_caps_hit, 0); > + percpu_counter_set(&metric->i_caps_mis, 0); > + > + percpu_counter_set(&metric->read_latency_sum, 0); > + percpu_counter_set(&metric->total_reads, 0); > + > + percpu_counter_set(&metric->write_latency_sum, 0); > + percpu_counter_set(&metric->total_writes, 0); > + > + percpu_counter_set(&metric->metadata_latency_sum, 0); > + percpu_counter_set(&metric->total_metadatas, 0); > + > + return count; > +} > + > +CEPH_DEFINE_RW_FUNC(metric); More broadly, how are these metrics going to be used? I suspect the MDSes will gradually start relying on the them in the future and probably make decisions based off of them? If that is the case, did you think about clients being able to mess with that by zeroing these counters on a regular basis? It looks like all of this is still in flight on the userspace side, but I don't see anything similar in https://github.com/ceph/ceph/pull/32120. Is there a different PR or is this kernel-only for some reason? Thanks, Ilya
