From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 83E92ECE582 for ; Tue, 10 Sep 2024 06:58:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=r9de0d8L8/KvIYx1QHw4BEMifcqFubTSWsRHK7Qewlw=; b=pQBoZocLAOC6cwoFlN52Tk+PhN mBQAGz1oap6eyw02jmDkDTmpcWAqsfuzipI0u4yQUj38UXUtt2/nk9dLVT9RC/q0Ck3UyW8ViymUE QydORjL3jlimnpzYcEqYhHd0FoQ8rf+ho8vi2VqrSv2c1B/uxYuxT86nix+8OvoGtMMoP/yFmN83A JF34cE1bg7YWU3xjKmZX1oiBiaxchozEc4fKf6fPsdxUS66jEX9iAmbGlZGGNMEyeK2pk6spBI22J kRdX5VrZrk30FTo5JJL7NK4Rpq+EQPYQ2IxqhfC/v1VxKhfLAuF0vuyD6rq9ECzZgMhD8Q+RBz21Y PxbWrLhw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1snup6-00000004W3r-0Aqt; Tue, 10 Sep 2024 06:58:00 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1snuo1-00000004VrQ-1Qm9 for linux-arm-kernel@lists.infradead.org; Tue, 10 Sep 2024 06:56:56 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 6167A5C04E4; Tue, 10 Sep 2024 06:56:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 12B41C4CEC3; Tue, 10 Sep 2024 06:56:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1725951411; bh=JRiZ0xj6jU/m2XriEJTGefpRMtVhRkzsDQX6ITf+Bwc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Yz2iTe1kuePXBsZ7y9rcvK/ycR27hFMNXP2BI18hfPMBJwwWNC/PWdI/JX3FjSnl/ G0i+GQp4CuU+ZCJRysZvwx62e5cyGzYdjUdTbc2ooJCm7APQdOUIVjNrUHvSEsc5y0 JAPFPMFxHzgF6RUEvSsREfH1bKPFXtoB0hp1K/P4pXX4AjzKIP5lLK02bSZ1ZZ3fen z0BWHcrHhE6eTuo9WZgnOoYlDl41AQglaz0TWBInyY6Lo1XzKDCUFtmP+QVLK0otv3 v/wNuVzobwolSfG7BUUAjxTABlcmZy2OKjQK3vrdtXjFP6gij/IyPWuYmokSXsGvj3 So3FaLjijAZ7A== Date: Mon, 9 Sep 2024 23:56:48 -0700 From: Namhyung Kim To: Ian Rogers Cc: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Kan Liang , John Garry , Will Deacon , James Clark , Mike Leach , Leo Yan , Ravi Bangoria , Weilin Wang , Jing Zhang , Xu Yang , Sandipan Das , Benjamin Gray , Athira Jajeev , Howard Chu , Dominique Martinet , Yang Jihong , Colin Ian King , Veronika Molnarova , "Dr. David Alan Gilbert" , Oliver Upton , Changbin Du , Ze Gao , Andi Kleen , =?utf-8?Q?Cl=C3=A9ment?= Le Goffic , Sun Haiyong , Junhao He , Tiezhu Yang , Yicong Yang , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v1 14/15] perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs Message-ID: References: <20240907050830.6752-1-irogers@google.com> <20240907050830.6752-15-irogers@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20240907050830.6752-15-irogers@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240909_235653_552226_20BBA8FC X-CRM114-Status: GOOD ( 39.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Sep 06, 2024 at 10:08:29PM -0700, Ian Rogers wrote: > 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, 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 > ... > ``` Interesting! While they don't seem to be counters, it'd be useful to see the values from various sources/PMUs especially for interval mode. > > The PMUs are named from /sys/class/hwmon/hwmon/name and have an > alias of hwmon. The events are naned using the _label files as > well as the prefix, the latter guaranteed to be unique. > > In `perf list` the other hwmon 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 > --- > tools/perf/util/Build | 1 + > tools/perf/util/evsel.c | 9 + > tools/perf/util/hwmon_pmu.c | 879 ++++++++++++++++++++++++++++++++++++ > tools/perf/util/hwmon_pmu.h | 30 ++ > tools/perf/util/pmu.c | 17 + > tools/perf/util/pmu.h | 2 + > tools/perf/util/pmus.c | 2 + > 7 files changed, 940 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 80187e3a52be..b1dd5d176d1c 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/evsel.c b/tools/perf/util/evsel.c > index 9e748ed20988..64883d2aa1bb 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -50,6 +50,7 @@ > #include "off_cpu.h" > #include "pmu.h" > #include "pmus.h" > +#include "hwmon_pmu.h" > #include "tool_pmu.h" > #include "rlimit.h" > #include "../perf-sys.h" > @@ -1657,6 +1658,9 @@ int evsel__read_counter(struct evsel *evsel, int cpu_map_idx, int thread) > if (evsel__is_tool(evsel)) > return evsel__tool_pmu_read(evsel, cpu_map_idx, thread); > > + if (evsel__is_hwmon(evsel)) > + return evsel__hwmon_pmu_read(evsel, cpu_map_idx, thread); > + > if (evsel__is_retire_lat(evsel)) > return evsel__read_retire_lat(evsel, cpu_map_idx, thread); > > @@ -2094,6 +2098,11 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus, > start_cpu_map_idx, > end_cpu_map_idx); > } > + if (evsel__is_hwmon(evsel)) { > + return evsel__hwmon_pmu_open(evsel, threads, > + start_cpu_map_idx, > + end_cpu_map_idx); > + } > > for (idx = start_cpu_map_idx; idx < end_cpu_map_idx; idx++) { > > diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c > new file mode 100644 > index 000000000000..cc8816b787ca > --- /dev/null > +++ b/tools/perf/util/hwmon_pmu.c > @@ -0,0 +1,879 @@ > +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) > +#include "counts.h" > +#include "debug.h" > +#include "evsel.h" > +#include "hashmap.h" > +#include "hwmon_pmu.h" > +#include "pmu.h" > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +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, Where does this list come from? Is it exhaustive? > + > + HWMON_TYPE_MAX > +}; > + > +static const char * const hwmon_type_strs[HWMON_TYPE_MAX] = { > + NULL, > + "cpu", > + "curr", > + "energy", > + "fan", > + "humidity", > + "in", > + "intrusion", > + "power", > + "pwm", > + "temp", > +}; > + > +static const char *const hwmon_units[HWMON_TYPE_MAX] = { > + NULL, > + "V", /* cpu */ > + "A", /* curr */ > + "J", /* energy */ > + "rpm", /* fan */ > + "%", /* humidity */ > + "V", /* in */ > + "", /* intrusion */ > + "W", /* power */ > + "Hz", /* pwm */ > + "'C", /* temp */ > +}; > + > +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, > +}; > + > +static const char * const hwmon_item_strs[HWMON_ITEM__MAX] = { > + 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", > +}; > + > +struct hwmon_pmu { > + struct perf_pmu pmu; > + struct hashmap events; > + int pmu_dir_fd; > +}; > + > +/** > + * union hwmon_pmu_event_key: Key for hwmon_pmu->events as such each key > + * represents an event. > + * > + * Related hwmon files start that this key represents. > + */ > +union hwmon_pmu_event_key { > + long type_and_num; > + struct { > + int num :16; > + enum hwmon_type type :8; Why not plain int types? I'm not sure how much we care about 32 bits but you could use short int then. > + }; > +}; > + > +/** > + * struct hwmon_pmu_event_value: Value in hwmon_pmu->events. > + * > + * Hwmon files are of the form _ and may have a suffix > + * _alarm. > + */ > +struct hwmon_pmu_event_value { > + /** @items: which item files are present. */ > + DECLARE_BITMAP(items, HWMON_ITEM__MAX); > + /** @alarm_items: which item files are present. */ > + DECLARE_BITMAP(alarm_items, HWMON_ITEM__MAX); > + /** @label: contents of _label if present. */ > + char *label; > + /** @name: name computed from label of the form _