[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fUaSCc6z6C36ce4X02tGF4pthULQ9k6+RFzZi1Z2bRDyg@mail.gmail.com>
Date: Fri, 8 Nov 2024 16:29:44 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>,
Ravi Bangoria <ravi.bangoria@....com>, Weilin Wang <weilin.wang@...el.com>,
Yoshihiro Furudera <fj5100bi@...itsu.com>, James Clark <james.clark@...aro.org>,
Athira Jajeev <atrajeev@...ux.vnet.ibm.com>, Howard Chu <howardchu95@...il.com>,
Oliver Upton <oliver.upton@...ux.dev>, Changbin Du <changbin.du@...wei.com>,
Ze Gao <zegao2021@...il.com>, Junhao He <hejunhao3@...wei.com>, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v7 2/7] perf hwmon_pmu: Add hwmon filename parser
On Fri, Nov 8, 2024 at 11:33 AM Namhyung Kim <namhyung@...nel.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@...gle.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
> >
Powered by blists - more mailing lists