Re: [PATCH v7 2/7] perf hwmon_pmu: Add hwmon filename parser

[Ian Rogers <irogers@google.com>]

On Fri, Nov 8, 2024 at 11:33 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Fri, Nov 08, 2024 at 09:49:31AM -0800, Ian Rogers wrote:
> > hwmon filenames have a specific encoding that will be used to give a
> > config value. The encoding is described in:
> > Documentation/hwmon/sysfs-interface.rst
> > Add a function to parse the filename into consituent enums/ints that
> > will then be amenable to config encoding.
> >
> > Note, things are done this way to allow mapping names to config and
> > back without the use of hash/dynamic lookup tables.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/Build       |   1 +
> >  tools/perf/util/hwmon_pmu.c | 142 ++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/hwmon_pmu.h | 111 ++++++++++++++++++++++++++++
> >  3 files changed, 254 insertions(+)
> >  create mode 100644 tools/perf/util/hwmon_pmu.c
> >  create mode 100644 tools/perf/util/hwmon_pmu.h
> >
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index 1eedead5f2f2..78b990c04f71 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -83,6 +83,7 @@ perf-util-y += pmu.o
> >  perf-util-y += pmus.o
> >  perf-util-y += pmu-flex.o
> >  perf-util-y += pmu-bison.o
> > +perf-util-y += hwmon_pmu.o
> >  perf-util-y += tool_pmu.o
> >  perf-util-y += svghelper.o
> >  perf-util-$(CONFIG_LIBTRACEEVENT) += trace-event-info.o
> > diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
> > new file mode 100644
> > index 000000000000..ee5fb1c41da3
> > --- /dev/null
> > +++ b/tools/perf/util/hwmon_pmu.c
> > @@ -0,0 +1,142 @@
> > +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> > +#include "debug.h"
> > +#include "hwmon_pmu.h"
> > +#include <assert.h>
> > +#include <stddef.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <linux/kernel.h>
> > +
> > +const char * const hwmon_type_strs[HWMON_TYPE_MAX] = {
>
> Can we make this array static?  I don't think it's used elsewhere.
>
> > +     NULL,
> > +     "cpu",
> > +     "curr",
> > +     "energy",
> > +     "fan",
> > +     "humidity",
> > +     "in",
> > +     "intrusion",
> > +     "power",
> > +     "pwm",
> > +     "temp",
> > +};
> > +#define LONGEST_HWMON_TYPE_STR "intrusion"
> > +
> > +const char * const hwmon_item_strs[HWMON_ITEM__MAX] = {
>
> Ditto.

Done in v8. The testing no longer uses the values.

Thanks,
Ian

> > +     NULL,
> > +     "accuracy",
> > +     "alarm",
> > +     "auto_channels_temp",
> > +     "average",
> > +     "average_highest",
> > +     "average_interval",
> > +     "average_interval_max",
> > +     "average_interval_min",
> > +     "average_lowest",
> > +     "average_max",
> > +     "average_min",
> > +     "beep",
> > +     "cap",
> > +     "cap_hyst",
> > +     "cap_max",
> > +     "cap_min",
> > +     "crit",
> > +     "crit_hyst",
> > +     "div",
> > +     "emergency",
> > +     "emergency_hist",
> > +     "enable",
> > +     "fault",
> > +     "freq",
> > +     "highest",
> > +     "input",
> > +     "label",
> > +     "lcrit",
> > +     "lcrit_hyst",
> > +     "lowest",
> > +     "max",
> > +     "max_hyst",
> > +     "min",
> > +     "min_hyst",
> > +     "mod",
> > +     "offset",
> > +     "pulses",
> > +     "rated_max",
> > +     "rated_min",
> > +     "reset_history",
> > +     "target",
> > +     "type",
> > +     "vid",
> > +};
> > +#define LONGEST_HWMON_ITEM_STR "average_interval_max"
> > +
> > +static int hwmon_strcmp(const void *a, const void *b)
> > +{
> > +     const char *sa = a;
> > +     const char * const *sb = b;
> > +
> > +     return strcmp(sa, *sb);
> > +}
> > +
> > +bool parse_hwmon_filename(const char *filename,
> > +                       enum hwmon_type *type,
> > +                       int *number,
> > +                       enum hwmon_item *item,
> > +                       bool *alarm)
> > +{
> > +     char fn_type[24];
> > +     const char **elem;
> > +     const char *fn_item = NULL;
> > +     size_t fn_item_len;
> > +
> > +     assert(strlen(LONGEST_HWMON_TYPE_STR) < sizeof(fn_type));
> > +     strlcpy(fn_type, filename, sizeof(fn_type));
> > +     for (size_t i = 0; fn_type[i] != '\0'; i++) {
> > +             if (fn_type[i] >= '0' && fn_type[i] <= '9') {
> > +                     fn_type[i] = '\0';
> > +                     *number = strtoul(&filename[i], (char **)&fn_item, 10);
> > +                     if (*fn_item == '_')
> > +                             fn_item++;
> > +                     break;
> > +             }
> > +             if (fn_type[i] == '_') {
> > +                     fn_type[i] = '\0';
> > +                     *number = -1;
> > +                     fn_item = &filename[i + 1];
> > +                     break;
> > +             }
> > +     }
> > +     if (fn_item == NULL || fn_type[0] == '\0' || (item != NULL && fn_item[0] == '\0')) {
> > +             pr_debug("hwmon_pmu: not a hwmon file '%s'\n", filename);
> > +             return false;
> > +     }
> > +     elem = bsearch(&fn_type, hwmon_type_strs + 1, ARRAY_SIZE(hwmon_type_strs) - 1,
> > +                    sizeof(hwmon_type_strs[0]), hwmon_strcmp);
> > +     if (!elem) {
> > +             pr_debug("hwmon_pmu: not a hwmon type '%s' in file name '%s'\n",
> > +                      fn_type, filename);
> > +             return false;
> > +     }
> > +
> > +     *type = elem - &hwmon_type_strs[0];
> > +     if (!item)
> > +             return true;
> > +
> > +     *alarm = false;
> > +     fn_item_len = strlen(fn_item);
> > +     if (fn_item_len > 6 && !strcmp(&fn_item[fn_item_len - 6], "_alarm")) {
> > +             assert(strlen(LONGEST_HWMON_ITEM_STR) < sizeof(fn_type));
> > +             strlcpy(fn_type, fn_item, fn_item_len - 6);
> > +             fn_item = fn_type;
> > +             *alarm = true;
> > +     }
> > +     elem = bsearch(fn_item, hwmon_item_strs + 1, ARRAY_SIZE(hwmon_item_strs) - 1,
> > +                    sizeof(hwmon_item_strs[0]), hwmon_strcmp);
> > +     if (!elem) {
> > +             pr_debug("hwmon_pmu: not a hwmon item '%s' in file name '%s'\n",
> > +                      fn_item, filename);
> > +             return false;
> > +     }
> > +     *item = elem - &hwmon_item_strs[0];
> > +     return true;
> > +}
> > diff --git a/tools/perf/util/hwmon_pmu.h b/tools/perf/util/hwmon_pmu.h
> > new file mode 100644
> > index 000000000000..df0ab5ff7534
> > --- /dev/null
> > +++ b/tools/perf/util/hwmon_pmu.h
> > @@ -0,0 +1,111 @@
> > +/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> > +#ifndef __HWMON_PMU_H
> > +#define __HWMON_PMU_H
> > +
> > +#include <stdbool.h>
> > +
> > +/**
> > + * enum hwmon_type:
> > + *
> > + * As described in Documentation/hwmon/sysfs-interface.rst hwmon events are
> > + * defined over multiple files of the form <type><num>_<item>. This enum
> > + * captures potential <type> values.
> > + *
> > + * This enum is exposed for testing.
> > + */
> > +enum hwmon_type {
> > +     HWMON_TYPE_NONE,
> > +
> > +     HWMON_TYPE_CPU,
> > +     HWMON_TYPE_CURR,
> > +     HWMON_TYPE_ENERGY,
> > +     HWMON_TYPE_FAN,
> > +     HWMON_TYPE_HUMIDITY,
> > +     HWMON_TYPE_IN,
> > +     HWMON_TYPE_INTRUSION,
> > +     HWMON_TYPE_POWER,
> > +     HWMON_TYPE_PWM,
> > +     HWMON_TYPE_TEMP,
> > +
> > +     HWMON_TYPE_MAX
> > +};
> > +
> > +/**
> > + * enum hwmon_item:
> > + *
> > + * Similar to enum hwmon_type but describes the item part of a a sysfs filename.
> > + *
> > + * This enum is exposed for testing.
> > + */
> > +enum hwmon_item {
> > +     HWMON_ITEM_NONE,
> > +
> > +     HWMON_ITEM_ACCURACY,
> > +     HWMON_ITEM_ALARM,
> > +     HWMON_ITEM_AUTO_CHANNELS_TEMP,
> > +     HWMON_ITEM_AVERAGE,
> > +     HWMON_ITEM_AVERAGE_HIGHEST,
> > +     HWMON_ITEM_AVERAGE_INTERVAL,
> > +     HWMON_ITEM_AVERAGE_INTERVAL_MAX,
> > +     HWMON_ITEM_AVERAGE_INTERVAL_MIN,
> > +     HWMON_ITEM_AVERAGE_LOWEST,
> > +     HWMON_ITEM_AVERAGE_MAX,
> > +     HWMON_ITEM_AVERAGE_MIN,
> > +     HWMON_ITEM_BEEP,
> > +     HWMON_ITEM_CAP,
> > +     HWMON_ITEM_CAP_HYST,
> > +     HWMON_ITEM_CAP_MAX,
> > +     HWMON_ITEM_CAP_MIN,
> > +     HWMON_ITEM_CRIT,
> > +     HWMON_ITEM_CRIT_HYST,
> > +     HWMON_ITEM_DIV,
> > +     HWMON_ITEM_EMERGENCY,
> > +     HWMON_ITEM_EMERGENCY_HIST,
> > +     HWMON_ITEM_ENABLE,
> > +     HWMON_ITEM_FAULT,
> > +     HWMON_ITEM_FREQ,
> > +     HWMON_ITEM_HIGHEST,
> > +     HWMON_ITEM_INPUT,
> > +     HWMON_ITEM_LABEL,
> > +     HWMON_ITEM_LCRIT,
> > +     HWMON_ITEM_LCRIT_HYST,
> > +     HWMON_ITEM_LOWEST,
> > +     HWMON_ITEM_MAX,
> > +     HWMON_ITEM_MAX_HYST,
> > +     HWMON_ITEM_MIN,
> > +     HWMON_ITEM_MIN_HYST,
> > +     HWMON_ITEM_MOD,
> > +     HWMON_ITEM_OFFSET,
> > +     HWMON_ITEM_PULSES,
> > +     HWMON_ITEM_RATED_MAX,
> > +     HWMON_ITEM_RATED_MIN,
> > +     HWMON_ITEM_RESET_HISTORY,
> > +     HWMON_ITEM_TARGET,
> > +     HWMON_ITEM_TYPE,
> > +     HWMON_ITEM_VID,
> > +
> > +     HWMON_ITEM__MAX,
> > +};
> > +
> > +/**
> > + * parse_hwmon_filename() - Parse filename into constituent parts.
> > + *
> > + * @filename: To be parsed, of the form <type><number>_<item>.
> > + * @type: The type defined from the parsed file name.
> > + * @number: The number of the type, for example there may be more than 1 fan.
> > + * @item: A hwmon <type><number> may have multiple associated items.
> > + * @alarm: Is the filename for an alarm value?
> > + *
> > + * An example of a hwmon filename is "temp1_input". The type is temp for a
> > + * temperature value. The number is 1. The item within the file is an input
> > + * value - the temperature itself. This file doesn't contain an alarm value.
> > + *
> > + * Exposed for testing.
> > + */
> > +bool parse_hwmon_filename(const char *filename,
> > +                       enum hwmon_type *type,
> > +                       int *number,
> > +                       enum hwmon_item *item,
> > +                       bool *alarm);
> > +
> > +#endif /* __HWMON_PMU_H */
> > --
> > 2.47.0.277.g8800431eea-goog
> >