Re: [RFC PATCH 1/7] core/metricfs: Create metricfs, standardized files under debugfs.
debugfs interaction nits:
On Fri, Aug 07, 2020 at 02:29:10PM -0700, Jonathan Adams wrote:
> +static struct dentry *metricfs_init_dentry(void)
> +{
> + static int once;
> +
> + if (d_metricfs)
> + return d_metricfs;
> +
> + if (!debugfs_initialized())
> + return NULL;
> +
> + d_metricfs = debugfs_create_dir("metricfs", NULL);
> +
> + if (!d_metricfs && !once) {
As it is impossible for d_metricfs to ever be NULL, why are you checking
it?
> + once = 1;
> + pr_warn("Could not create debugfs directory 'metricfs'\n");
There is a pr_warn_once I think, but again, how can this ever trigger?
> + return NULL;
> + }
> +
> + return d_metricfs;
> +}
> +
> +/* We always cast in and out to struct dentry. */
> +struct metricfs_subsys {
> + struct dentry dentry;
> +};
> +
> +static struct dentry *metricfs_create_file(const char *name,
> +mode_t mode,
> +struct dentry *parent,
> +void *data,
> +const struct file_operations *fops)
> +{
> + struct dentry *ret;
> +
> + ret = debugfs_create_file(name, mode, parent, data, fops);
> + if (!ret)
> + pr_warn("Could not create debugfs '%s' entry\n", name);
As ret can never be NULL, why check?
There is no need to ever check debugfs return values, just keep on
going, that's the design of it.
thanks,
greg k-h
[RFC PATCH 1/7] core/metricfs: Create metricfs, standardized files under debugfs.
From: Justin TerAvest
Metricfs is a standardized set of files and directories under debugfs,
with a kernel API designed to be simpler than exporting new files under
sysfs. Type and field information is reported so that a userspace daemon
can easily process the information.
The statistics live under debugfs, in a tree rooted at:
/sys/kernel/debug/metricfs
Each metric is a directory, with four files in it. This patch includes
a single "metricfs_presence" metric, whose files look like:
/sys/kernel/debug/metricfs:
metricfs_presence/annotations
DESCRIPTION A\ basic\ presence\ metric.
metricfs_presence/fields
value
int
metricfs_presence/values
1
metricfs_presence/version
1
Statistics can have zero, one, or two 'fields', which are keys for the
table of metric values. With no fields, you have a simple statistic as
above, with one field you have a 1-dimensional table of string -> value,
and with two fields you have a 2-dimensional table of
{string, string} -> value.
When a statistic's 'values' file is opened, we pre-allocate a 64k buffer
and call the statistic's callback to fill it with data, truncating if
the buffer overflows.
Statistic creators can create a hierarchy for their statistics using
metricfs_create_subsys().
Signed-off-by: Justin TerAvest
[jwad...@google.com: Forward ported to v5.8, cleaned up and modernized
code significantly]
Signed-off-by: Jonathan Adams
---
notes:
* To go upstream, this will need documentation and a MAINTAINERS update.
* It's not clear what the "version" file is for; it's vestigial and
should probably be removed.
jwad...@google.com: Forward ported to v5.8, removed some google-isms and
cleaned up some anachronisms (atomic->refcount, moving to
kvmalloc(), using POISON_POINTER_DELTA, made more functions static,
made 'emitter_fn' into an explicit union instead of a void *),
renamed 'struct emitter -> metric_emitter' and renamed
some funcs for consistency.
---
include/linux/metricfs.h | 103 ++
kernel/Makefile| 2 +
kernel/metricfs.c | 727 +
kernel/metricfs_examples.c | 151
lib/Kconfig.debug | 18 +
5 files changed, 1001 insertions(+)
create mode 100644 include/linux/metricfs.h
create mode 100644 kernel/metricfs.c
create mode 100644 kernel/metricfs_examples.c
diff --git a/include/linux/metricfs.h b/include/linux/metricfs.h
new file mode 100644
index ..65a1baa8e8c1
--- /dev/null
+++ b/include/linux/metricfs.h
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _METRICFS_H_
+#define _METRICFS_H_
+
+#include
+#include
+#include
+
+struct metric;
+struct metricfs_subsys;
+
+#define METRIC_EXPORT_GENERIC(name, desc, fname0, fname1, fn, is_str,
cumulative) \
+static struct metric *metric_##name; \
+void metric_init_##name(struct metricfs_subsys *parent) \
+{ \
+ metric_##name = metric_register(__stringify(name), (parent), (desc), \
+ (fname0), (fname1), (fn), (is_str), \
+ (cumulative), THIS_MODULE); \
+} \
+void metric_exit_##name(void) \
+{ \
+ metric_unregister(metric_##name); \
+}
+
+/*
+ * Metricfs only deals with two types: int64_t and const char*.
+ *
+ * If a metric has fewer than two fields, pass NULL for the field name
+ * arguments.
+ *
+ * The metric does not take ownership of any of the strings passed in.
+ *
+ * See kernel/metricfs_examples.c for a set of example metrics, with
+ * corresponding output.
+ *
+ * METRIC_EXPORT_INT - An integer-valued metric.
+ * METRIC_EXPORT_COUNTER - An integer-valued cumulative metric.
+ * METRIC_EXPORT_STR - A string-valued metric.
+ */
+#define METRIC_EXPORT_INT(name, desc, fname0, fname1, fn) \
+ METRIC_EXPORT_GENERIC(name, (desc), (fname0), (fname1), (fn), \
+ false, false)
+#define METRIC_EXPORT_COUNTER(name, desc, fname0, fname1, fn) \
+ METRIC_EXPORT_GENERIC(name, (desc), (fname0), (fname1), (fn), \
+ false, true)
+#define METRIC_EXPORT_STR(name, desc, fname0, fname1, fn) \
+ METRIC_EXPORT_GENERIC(name, (desc), (fname0), (fname1), (fn), \
+ true, false)
+
+/* Subsystem support. */
+/* Pass NULL as 'parent' to create a new top-level subsystem. */
+struct metricfs_subsys *metricfs_create_subsys(const char *name,
+ struct metricfs_subsys *parent);
+void metricfs_destroy_subsys(struct metricfs_subsys *d);
+
+/*
+ * An opaque struct that metric emit functions use to keep our internal
+ * state.
+ */
+struct metric_emitter;
+
+/* The number of non-NULL arguments passed to EMIT macros must match the number
+ * of arguments passed to the EXPORT macro for a given metric.
+ *
+ * Failure to do so will cause data to be mangled (or dropped) by userspace or
+ * Monarch.
+ */
+#define METRIC_EMIT_INT(e, v, f0, f1) \
+ metric
[RFC PATCH 1/7] core/metricfs: Create metricfs, standardized files under debugfs.
From: Justin TerAvest
Metricfs is a standardized set of files and directories under debugfs,
with a kernel API designed to be simpler than exporting new files under
sysfs. Type and field information is reported so that a userspace daemon
can easily process the information.
The statistics live under debugfs, in a tree rooted at:
/sys/kernel/debug/metricfs
Each metric is a directory, with four files in it. This patch includes
a single "metricfs_presence" metric, whose files look like:
/sys/kernel/debug/metricfs:
metricfs_presence/annotations
DESCRIPTION A\ basic\ presence\ metric.
metricfs_presence/fields
value
int
metricfs_presence/values
1
metricfs_presence/version
1
Statistics can have zero, one, or two 'fields', which are keys for the
table of metric values. With no fields, you have a simple statistic as
above, with one field you have a 1-dimensional table of string -> value,
and with two fields you have a 2-dimensional table of
{string, string} -> value.
When a statistic's 'values' file is opened, we pre-allocate a 64k buffer
and call the statistic's callback to fill it with data, truncating if
the buffer overflows.
Statistic creators can create a hierarchy for their statistics using
metricfs_create_subsys().
Signed-off-by: Justin TerAvest
[jwad...@google.com: Forward ported to v5.8, cleaned up and modernized
code significantly]
Signed-off-by: Jonathan Adams
---
notes:
* To go upstream, this will need documentation and a MAINTAINERS update.
* It's not clear what the "version" file is for; it's vestigial and
should probably be removed.
jwad...@google.com: Forward ported to v5.8, removed some google-isms and
cleaned up some anachronisms (atomic->refcount, moving to
kvmalloc(), using POISON_POINTER_DELTA, made more functions static,
made 'emitter_fn' into an explicit union instead of a void *),
renamed 'struct emitter -> metric_emitter' and renamed
some funcs for consistency.
---
include/linux/metricfs.h | 103 ++
kernel/Makefile| 2 +
kernel/metricfs.c | 727 +
kernel/metricfs_examples.c | 151
lib/Kconfig.debug | 18 +
5 files changed, 1001 insertions(+)
create mode 100644 include/linux/metricfs.h
create mode 100644 kernel/metricfs.c
create mode 100644 kernel/metricfs_examples.c
diff --git a/include/linux/metricfs.h b/include/linux/metricfs.h
new file mode 100644
index ..65a1baa8e8c1
--- /dev/null
+++ b/include/linux/metricfs.h
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _METRICFS_H_
+#define _METRICFS_H_
+
+#include
+#include
+#include
+
+struct metric;
+struct metricfs_subsys;
+
+#define METRIC_EXPORT_GENERIC(name, desc, fname0, fname1, fn, is_str,
cumulative) \
+static struct metric *metric_##name; \
+void metric_init_##name(struct metricfs_subsys *parent) \
+{ \
+ metric_##name = metric_register(__stringify(name), (parent), (desc), \
+ (fname0), (fname1), (fn), (is_str), \
+ (cumulative), THIS_MODULE); \
+} \
+void metric_exit_##name(void) \
+{ \
+ metric_unregister(metric_##name); \
+}
+
+/*
+ * Metricfs only deals with two types: int64_t and const char*.
+ *
+ * If a metric has fewer than two fields, pass NULL for the field name
+ * arguments.
+ *
+ * The metric does not take ownership of any of the strings passed in.
+ *
+ * See kernel/metricfs_examples.c for a set of example metrics, with
+ * corresponding output.
+ *
+ * METRIC_EXPORT_INT - An integer-valued metric.
+ * METRIC_EXPORT_COUNTER - An integer-valued cumulative metric.
+ * METRIC_EXPORT_STR - A string-valued metric.
+ */
+#define METRIC_EXPORT_INT(name, desc, fname0, fname1, fn) \
+ METRIC_EXPORT_GENERIC(name, (desc), (fname0), (fname1), (fn), \
+ false, false)
+#define METRIC_EXPORT_COUNTER(name, desc, fname0, fname1, fn) \
+ METRIC_EXPORT_GENERIC(name, (desc), (fname0), (fname1), (fn), \
+ false, true)
+#define METRIC_EXPORT_STR(name, desc, fname0, fname1, fn) \
+ METRIC_EXPORT_GENERIC(name, (desc), (fname0), (fname1), (fn), \
+ true, false)
+
+/* Subsystem support. */
+/* Pass NULL as 'parent' to create a new top-level subsystem. */
+struct metricfs_subsys *metricfs_create_subsys(const char *name,
+ struct metricfs_subsys *parent);
+void metricfs_destroy_subsys(struct metricfs_subsys *d);
+
+/*
+ * An opaque struct that metric emit functions use to keep our internal
+ * state.
+ */
+struct metric_emitter;
+
+/* The number of non-NULL arguments passed to EMIT macros must match the number
+ * of arguments passed to the EXPORT macro for a given metric.
+ *
+ * Failure to do so will cause data to be mangled (or dropped) by userspace or
+ * Monarch.
+ */
+#define METRIC_EMIT_INT(e, v, f0, f1) \
+ metric
