From: "Liang, Kan" <kan.liang@linux.intel.com>
To: Ian Rogers <irogers@google.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Howard Chu <howardchu95@gmail.com>,
Weilin Wang <weilin.wang@intel.com>,
Levi Yun <yeoreum.yun@arm.com>,
"Dr. David Alan Gilbert" <linux@treblig.org>,
Andi Kleen <ak@linux.intel.com>,
James Clark <james.clark@linaro.org>,
Dominique Martinet <asmadeus@codewreck.org>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/5] perf stat: Better hybrid support for the NMI watchdog warning
Date: Wed, 2 Apr 2025 11:23:12 -0400 [thread overview]
Message-ID: <28016335-385c-48b9-8767-9b087a0ec4cc@linux.intel.com> (raw)
In-Reply-To: <20250318041442.321230-2-irogers@google.com>
On 2025-03-18 12:14 a.m., Ian Rogers wrote:
> Prior to this patch evlist__has_hybrid would return false if the
> processor wasn't hybrid or the evlist didn't contain any core
> events. If the only PMU used by events was cpu_core then it would
> true even though there are no cpu_atom events. For example:
>
> ```
> $ perf stat --cputype=cpu_core -e '{cycles,cycles,cycles,cycles,cycles,cycles,cycles,cycles,cycles}' true
>
> Performance counter stats for 'true':
>
> <not counted> cpu_core/cycles/ (0.00%)
> <not counted> cpu_core/cycles/ (0.00%)
> <not counted> cpu_core/cycles/ (0.00%)
> <not counted> cpu_core/cycles/ (0.00%)
> <not counted> cpu_core/cycles/ (0.00%)
> <not counted> cpu_core/cycles/ (0.00%)
> <not counted> cpu_core/cycles/ (0.00%)
> <not counted> cpu_core/cycles/ (0.00%)
> <not counted> cpu_core/cycles/ (0.00%)
>
> 0.001981900 seconds time elapsed
>
> 0.002311000 seconds user
> 0.000000000 seconds sys
> ```
>
> This patch changes evlist__has_hybrid to return true only if the
> evlist contains events from >1 core PMU. This means the NMI watchdog
> warning is shown for the case above.
Nit:
The function name may still bring confusions.
It may be better to change the function name as well, e.g.,
evlist__has_hybrid_pmus()? It implies more than one PMU.
Thanks,
Kan>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/stat-display.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index e852ac0d9847..f311f1960e29 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -825,13 +825,25 @@ static bool is_mixed_hw_group(struct evsel *counter)
> static bool evlist__has_hybrid(struct evlist *evlist)
> {
> struct evsel *evsel;
> + struct perf_pmu *last_core_pmu = NULL;
>
> if (perf_pmus__num_core_pmus() == 1)
> return false;
>
> evlist__for_each_entry(evlist, evsel) {
> - if (evsel->core.is_pmu_core)
> + if (evsel->core.is_pmu_core) {
> + struct perf_pmu *pmu = evsel__find_pmu(evsel);
> +
> + if (pmu == last_core_pmu)
> + continue;
> +
> + if (last_core_pmu == NULL) {
> + last_core_pmu = pmu;
> + continue;
> + }
> + /* A distinct core PMU. */
> return true;
> + }
> }
>
> return false;
next prev parent reply other threads:[~2025-04-02 15:23 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-18 4:14 [PATCH v1 0/5] NMI warning and debug improvements Ian Rogers
2025-03-18 4:14 ` [PATCH v1 1/5] perf stat: Better hybrid support for the NMI watchdog warning Ian Rogers
2025-04-02 15:23 ` Liang, Kan [this message]
2025-04-02 15:49 ` Ian Rogers
2025-03-18 4:14 ` [PATCH v1 2/5] perf stat: Remove print_mixed_hw_group_error Ian Rogers
2025-03-18 4:14 ` [PATCH v1 3/5] perf evlist: Refactor evlist__scnprintf_evsels Ian Rogers
2025-04-02 15:25 ` Liang, Kan
2025-04-02 15:55 ` Ian Rogers
2025-03-18 4:14 ` [PATCH v1 4/5] perf evlist: Add groups to evlist__format_evsels Ian Rogers
2025-03-18 4:14 ` [PATCH v1 5/5] perf parse-events: Add debug dump of evlist if reordered Ian Rogers
2025-04-02 15:34 ` Liang, Kan
2025-04-01 22:05 ` [PATCH v1 0/5] NMI warning and debug improvements Ian Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=28016335-385c-48b9-8767-9b087a0ec4cc@linux.intel.com \
--to=kan.liang@linux.intel.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=asmadeus@codewreck.org \
--cc=howardchu95@gmail.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linux@treblig.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=weilin.wang@intel.com \
--cc=yeoreum.yun@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox