On 2020/1/7 19:13, Ilya Dryomov wrote:
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?
Just copied the code from some other place.
Any struct file_operations defined by means of the
DEFINE_DEBUGFS_ATTRIBUTE macro is protected against file removals, so it
should be okay.
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?
Just to make each patch as small as possible, which will be easier to
review.
I am okay to merge them with a better changelog.
Do we really need to send metrics more often than we potentially renew
caps?
I had the same question, while the user client is tied to Client::tick()
and sending the metrics every second.
Here will just keep it the same with the user client logic, but per
second is not a must.
Thanks.
Thanks,
Ilya
