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
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.
Also, why one update per second? Should that interval be tunable?
> 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?
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 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.
--
Jeff Layton <jlayton@xxxxxxxxxx>
