On 2020/2/6 19:31, Jeff Layton wrote:
On Thu, 2020-02-06 at 10:36 +0800, Xiubo Li wrote:
On 2020/2/6 5:43, Jeff Layton wrote:
On Wed, 2020-01-29 at 03:27 -0500, xiubli@xxxxxxxxxx wrote:
[...]
+
+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.
Technically, no. You could wire it up so that you could enable and
disable it via -o remount. For example:
# mount -o remount,metrics=disabled
Yeah, this is cool.
Another option might be a module parameter if this is something that you
really want to be global (and not per-mount or per-session).
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.
Meh, one frame per second doesn't seem like it'll add much overhead.
Okay.
Also, why one update per second? Should that interval be tunable?
Per second just keep it the same with the fuse client.
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 ?
I guess it's not clear to me how you intend for this to be used.
A debugfs switch means that this is being enabled and disabled on a per-
session basis. Is the user supposed to turn this on for all, or just one
session? How do they know?
Not for all, just per-superblock.
Is this something we expect people to just turn on briefly when they are
experiencing a problem, or is this something that we expect to be turned
on and left on for long periods of time?
If this won't add much overhead even per second, let's keep sending the
metrics to ceph always and the mount option for this switch is not
needed any more.
And there is already a switch to enable/disable showing the metrics in
the ceph side, if here add another switch per client, it will be also
yucky for admins .
Let's make the update interval tunable and per second as default. Maybe
we should make this as a global UI for all clients ?
Is this okay ?
Thanks.
If it's the latter then setting up a mount in /etc/fstab is not going to
be sufficient for an admin. She'll have to write a script or something
that goes in after the mount and enables this by writing to debugfs
after rebooting. Yuck.
