On 2020/2/6 5:43, Jeff Layton wrote:
On Wed, 2020-01-29 at 03:27 -0500, xiubli@xxxxxxxxxx wrote:
[...]
+/*
+ * 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;
Shouldn't that be a bool cast? Do we even need a cast there?
Will switch sending_metrics to bool type instead.
+ 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_SIMPLE_ATTRIBUTE(sending_metrics_fops, sending_metrics_get,
+ sending_metrics_set, "%llu\n");
+
I'd like to hear more about how we expect users to use this facility.
This debugfs file doesn't seem consistent with the rest of the UI, and I
imagine if the box reboots you'd have to (manually) re-enable it after
mount, right? Maybe this should be a mount option instead?
A mount option means we must do the unmounting to disable it.
I was thinking with the debugfs file we can do the debug or tuning even
in the product setups at any time, usually this should be disabled since
it will send it per second.
Or we could merge the "sending_metric" to "metrics" UI, just writing
"enable"/"disable" to enable/disable sending the metrics to ceph, and
just like the "reset" does to clean the metrics.
Then the "/sys/kernel/debug/ceph/XXX.clientYYY/metrics" could be
writable with:
"reset" --> to clean and reset the metrics counters
"enable" --> enable sending metrics to ceph cluster
"disable" --> disable sending metrics to ceph cluster
Will this be better ?
[...]
/*
* delayed work -- periodically trim expired leases, renew caps with mds
*/
+#define CEPH_WORK_DELAY_DEF 5
static void schedule_delayed(struct ceph_mds_client *mdsc)
{
- int delay = 5;
- unsigned hz = round_jiffies_relative(HZ * delay);
+ unsigned int hz;
+ int delay = CEPH_WORK_DELAY_DEF;
+
+ mutex_lock(&mdsc->mutex);
+ if (mdsc->sending_metrics)
+ delay = 1;
+ mutex_unlock(&mdsc->mutex);
+
The mdsc->mutex is dropped in the callers a little before this is
called, so this is a little too mutex-thrashy. I think you'd be better
off changing this function to be called with the mutex still held.
Will fix it.
[...]
+/* metric caps header */
+struct ceph_metric_cap {
+ __le32 type; /* ceph metric type */
+
+ __u8 ver;
+ __u8 campat;
I think you meant "compat" here.
Will fix it.
[...]
+/* metric metadata latency header */
+struct ceph_metric_metadata_latency {
+ __le32 type; /* ceph metric type */
+
+ __u8 ver;
+ __u8 campat;
+
+ __le32 data_len; /* length of sizeof(sec + nsec) */
+ __le32 sec;
+ __le32 nsec;
+} __attribute__ ((packed));
+
+struct ceph_metric_head {
+ __le32 num; /* the number of metrics will be sent */
"the number of metrics that will be sent"
Will fix it.
Thanks,
+} __attribute__ ((packed));
+
/* This is the global metrics */
struct ceph_client_metric {
atomic64_t total_dentries;
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 3f4829222528..a91431e9bdf7 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -128,6 +128,7 @@ struct ceph_fs_client {
struct dentry *debugfs_congestion_kb;
struct dentry *debugfs_bdi;
struct dentry *debugfs_mdsc, *debugfs_mdsmap;
+ struct dentry *debugfs_sending_metrics;
struct dentry *debugfs_metric;
struct dentry *debugfs_mds_sessions;
#endif
diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
index a099f60feb7b..6028d3e865e4 100644
--- a/include/linux/ceph/ceph_fs.h
+++ b/include/linux/ceph/ceph_fs.h
@@ -130,6 +130,7 @@ struct ceph_dir_layout {
#define CEPH_MSG_CLIENT_REQUEST 24
#define CEPH_MSG_CLIENT_REQUEST_FORWARD 25
#define CEPH_MSG_CLIENT_REPLY 26
+#define CEPH_MSG_CLIENT_METRICS 29
#define CEPH_MSG_CLIENT_CAPS 0x310
#define CEPH_MSG_CLIENT_LEASE 0x311
#define CEPH_MSG_CLIENT_SNAP 0x312
