Re: [PATCH v7 4/7] perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs
From: Ian Rogers
Date: Fri Nov 08 2024 - 19:31:00 EST
On Fri, Nov 8, 2024 at 11:39 AM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> On Fri, Nov 08, 2024 at 09:49:33AM -0800, Ian Rogers wrote:
> > Add a tool PMU for hwmon events but don't enable.
> >
> > The hwmon sysfs ABI is defined in
> > Documentation/hwmon/sysfs-interface.rst. Create a PMU that reads the
> > hwmon input and can be used in `perf stat` and metrics much as an
> > uncore PMU can.
> >
> > For example, when enabled by a later patch, the following shows
> > reading the CPU temperature and 2 fan speeds alongside the uncore
> > frequency:
> > ```
> > $ perf stat -e temp_cpu,fan1,hwmon_thinkpad/fan2/,tool/num_cpus_online/ -M UNCORE_FREQ -I 1000
> > 1.001153138 52.00 'C temp_cpu
> > 1.001153138 2,588 rpm fan1
> > 1.001153138 2,482 rpm hwmon_thinkpad/fan2/
> > 1.001153138 8 tool/num_cpus_online/
> > 1.001153138 1,077,101,397 UNC_CLOCK.SOCKET # 1.08 UNCORE_FREQ
> > 1.001153138 1,012,773,595 duration_time
> > ...
> > ```
> >
> > The PMUs are named from /sys/class/hwmon/hwmon<num>/name and have an
> > alias of hwmon<num>.
> >
> > Hwmon data is presented in multiple <type><number>_<item> files. The
> > <type><number> is used to identify the event as is the <type> followed
> > by the contents of the <type>_label file if it exists. The
> > <type><number>_input file gives the data read by perf.
> >
> > When enabled by a later patch, in `perf list` the other hwmon <item>
> > files are used to give a richer description, for example:
> > ```
> > hwmon:
> > temp1
> > [Temperature in unit acpitz named temp1. Unit: hwmon_acpitz]
> > in0
> > [Voltage in unit bat0 named in0. Unit: hwmon_bat0]
> > temp_core_0 OR temp2
> > [Temperature in unit coretemp named Core 0. crit=100'C,max=100'C crit_alarm=0'C. Unit:
> > hwmon_coretemp]
> > temp_core_1 OR temp3
> > [Temperature in unit coretemp named Core 1. crit=100'C,max=100'C crit_alarm=0'C. Unit:
> > hwmon_coretemp]
> > ...
> > temp_package_id_0 OR temp1
> > [Temperature in unit coretemp named Package id 0. crit=100'C,max=100'C crit_alarm=0'C.
> > Unit: hwmon_coretemp]
> > temp1
> > [Temperature in unit iwlwifi_1 named temp1. Unit: hwmon_iwlwifi_1]
> > temp_composite OR temp1
> > [Temperature in unit nvme named Composite. alarm=0'C,crit=86.85'C,max=75.85'C,
> > min=-273.15'C. Unit: hwmon_nvme]
> > temp_sensor_1 OR temp2
> > [Temperature in unit nvme named Sensor 1. max=65261.8'C,min=-273.15'C. Unit: hwmon_nvme]
> > temp_sensor_2 OR temp3
> > [Temperature in unit nvme named Sensor 2. max=65261.8'C,min=-273.15'C. Unit: hwmon_nvme]
> > fan1
> > [Fan in unit thinkpad named fan1. Unit: hwmon_thinkpad]
> > fan2
> > [Fan in unit thinkpad named fan2. Unit: hwmon_thinkpad]
> > ...
> > temp_cpu OR temp1
> > [Temperature in unit thinkpad named CPU. Unit: hwmon_thinkpad]
> > temp_gpu OR temp2
> > [Temperature in unit thinkpad named GPU. Unit: hwmon_thinkpad]
> > curr1
> > [Current in unit ucsi_source_psy_usbc000_0 named curr1. max=1.5A. Unit:
> > hwmon_ucsi_source_psy_usbc000_0]
> > in0
> > [Voltage in unit ucsi_source_psy_usbc000_0 named in0. max=5V,min=5V. Unit:
> > hwmon_ucsi_source_psy_usbc000_0]
> > ```
> >
> > As there may be multiple hwmon devices a range of PMU types are
> > reserved for their use and to identify the PMU as belonging to the
> > hwmon types.
> >
> > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> > ---
> > tools/perf/util/hwmon_pmu.c | 683 ++++++++++++++++++++++++++++++++++++
> > tools/perf/util/hwmon_pmu.h | 45 +++
> > tools/perf/util/pmu.h | 2 +
> > 3 files changed, 730 insertions(+)
> >
> > diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
> > index f4b7b3b6a052..3f1bf9a9cfdf 100644
> > --- a/tools/perf/util/hwmon_pmu.c
> > +++ b/tools/perf/util/hwmon_pmu.c
> [SNIP]
> > +struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, int hwmon_dir, const char *sysfs_name, const char *name)
> > +{
> > + char buf[32];
> > + struct hwmon_pmu *hwm;
> > +
> > + hwm = zalloc(sizeof(*hwm));
> > + if (!hwm)
> > + return NULL;
> > +
> > +
>
> Two blank lines.
Fixed.
> > + hwm->hwmon_dir_fd = hwmon_dir;
> > + hwm->pmu.type = PERF_PMU_TYPE_HWMON_START + strtoul(sysfs_name + 5, NULL, 10);
> > + if (hwm->pmu.type > PERF_PMU_TYPE_HWMON_END) {
> > + pr_err("Unable to encode hwmon type from %s in valid PMU type\n", sysfs_name);
> > + goto err_out;
> > + }
> > + snprintf(buf, sizeof(buf), "hwmon_%s", name);
> > + fix_name(buf + 6);
> > + hwm->pmu.name = strdup(buf);
> > + if (!hwm->pmu.name)
> > + goto err_out;
> > + hwm->pmu.alias_name = strdup(sysfs_name);
> > + if (!hwm->pmu.alias_name)
> > + goto err_out;
> > + hwm->pmu.cpus = perf_cpu_map__new("0");
> > + if (!hwm->pmu.cpus)
> > + goto err_out;
> > + INIT_LIST_HEAD(&hwm->pmu.format);
> > + INIT_LIST_HEAD(&hwm->pmu.aliases);
> > + INIT_LIST_HEAD(&hwm->pmu.caps);
> > + hashmap__init(&hwm->events, hwmon_pmu__event_hashmap_hash,
> > + hwmon_pmu__event_hashmap_equal, /*ctx=*/NULL);
> > +
> > + list_add_tail(&hwm->pmu.list, pmus);
> > + return &hwm->pmu;
> > +err_out:
> > + free((char *)hwm->pmu.name);
> > + free(hwm->pmu.alias_name);
> > + free(hwm);
> > + close(hwmon_dir);
> > + return NULL;
> > +}
>
> [...]
> > diff --git a/tools/perf/util/hwmon_pmu.h b/tools/perf/util/hwmon_pmu.h
> > index df0ab5ff7534..ebfdfe3b295a 100644
> > --- a/tools/perf/util/hwmon_pmu.h
> > +++ b/tools/perf/util/hwmon_pmu.h
> > @@ -2,8 +2,12 @@
> > #ifndef __HWMON_PMU_H
> > #define __HWMON_PMU_H
> >
> > +#include "pmu.h"
> > #include <stdbool.h>
> >
> > +struct list_head;
> > +struct perf_thread_map;
> > +
> > /**
> > * enum hwmon_type:
> > *
> > @@ -87,6 +91,14 @@ enum hwmon_item {
> > HWMON_ITEM__MAX,
> > };
> >
> > +/** Strings that correspond to enum hwmon_type. */
> > +extern const char * const hwmon_type_strs[HWMON_TYPE_MAX];
> > +/** Strings that correspond to enum hwmon_item. */
> > +extern const char * const hwmon_item_strs[HWMON_ITEM__MAX];
>
> Belongs to the patch2? But it'd be nice if we can remove them.
Made static in v8.
Thanks,
Ian