lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAP-5=fUpwgtLcSs37ab9Hpauf6EwoBVOePFgQoa0HZzd6LRVxQ@mail.gmail.com>
Date: Fri, 8 Nov 2024 16:30:39 -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 4/7] perf hwmon_pmu: Add a tool PMU exposing events
 from hwmon in sysfs

On Fri, Nov 8, 2024 at 11:39 AM Namhyung Kim <namhyung@...nel.org> 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@...gle.com>
> > ---
> >  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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ