On Tue, Dec 24, 2019 at 5:05 AM <xiubli@xxxxxxxxxx> wrote: > > From: Xiubo Li <xiubli@xxxxxxxxxx> > > Disabled as default, if it's enabled the kclient will send metrics > every second. > > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > --- > fs/ceph/debugfs.c | 44 ++++++++++++++++++++++++++++++-- > fs/ceph/mds_client.c | 60 +++++++++++++++++++++++++++++++------------- > fs/ceph/mds_client.h | 3 +++ > fs/ceph/super.h | 1 + > 4 files changed, 89 insertions(+), 19 deletions(-) > > diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c > index c132fdb40d53..a26e559473fd 100644 > --- a/fs/ceph/debugfs.c > +++ b/fs/ceph/debugfs.c > @@ -124,6 +124,40 @@ static int mdsc_show(struct seq_file *s, void *p) > return 0; > } > > +/* > + * metrics debugfs > + */ > +static int sending_metrics_set(void *data, u64 val) > +{ > + struct ceph_fs_client *fsc = (struct ceph_fs_client *)data; > + struct ceph_mds_client *mdsc = fsc->mdsc; > + > + if (val > 1) { > + pr_err("Invalid sending metrics set value %llu\n", val); > + return -EINVAL; > + } > + > + mutex_lock(&mdsc->mutex); > + mdsc->sending_metrics = (unsigned int)val; > + mutex_unlock(&mdsc->mutex); > + > + return 0; > +} > + > +static int sending_metrics_get(void *data, u64 *val) > +{ > + struct ceph_fs_client *fsc = (struct ceph_fs_client *)data; > + struct ceph_mds_client *mdsc = fsc->mdsc; > + > + mutex_lock(&mdsc->mutex); > + *val = (u64)mdsc->sending_metrics; > + mutex_unlock(&mdsc->mutex); > + > + return 0; > +} > +DEFINE_DEBUGFS_ATTRIBUTE(sending_metrics_fops, sending_metrics_get, > + sending_metrics_set, "%llu\n"); > + > static int metric_show(struct seq_file *s, void *p) > { > struct ceph_fs_client *fsc = s->private; > @@ -279,11 +313,9 @@ static int congestion_kb_get(void *data, u64 *val) > *val = (u64)fsc->mount_options->congestion_kb; > return 0; > } > - > DEFINE_SIMPLE_ATTRIBUTE(congestion_kb_fops, congestion_kb_get, > congestion_kb_set, "%llu\n"); > > - > void ceph_fs_debugfs_cleanup(struct ceph_fs_client *fsc) > { > dout("ceph_fs_debugfs_cleanup\n"); > @@ -293,6 +325,7 @@ void ceph_fs_debugfs_cleanup(struct ceph_fs_client *fsc) > debugfs_remove(fsc->debugfs_mds_sessions); > debugfs_remove(fsc->debugfs_caps); > debugfs_remove(fsc->debugfs_metric); > + debugfs_remove(fsc->debugfs_sending_metrics); > debugfs_remove(fsc->debugfs_mdsc); > } > > @@ -333,6 +366,13 @@ void ceph_fs_debugfs_init(struct ceph_fs_client *fsc) > fsc, > &mdsc_show_fops); > > + fsc->debugfs_sending_metrics = > + debugfs_create_file_unsafe("sending_metrics", > + 0600, > + fsc->client->debugfs_dir, > + fsc, > + &sending_metrics_fops); Hi Xiubo, Same question as to Chen. Why are you using the unsafe variant with DEFINE_DEBUGFS_ATTRIBUTE instead of just mirroring the existing writeback_congestion_kb? Have you verified that it is safe? I was a little confused by this series as a whole too. Patch 3 says that these metrics will be sent every 5 seconds, which matches the caps tick interval. This patch changes that to 1 second and makes sending metrics optional. Perhaps merge patches 3 and 4 into a single patch with a better changelog? Do we really need to send metrics more often than we potentially renew caps? Thanks, Ilya