| 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
| ||
|
Message-ID: <CAP-5=fW20XxHwQnjeJZjLvCF0nVJ8tJ6tDeCm4-eDHKDdg0m4Q@mail.gmail.com>
Date: Tue, 4 Nov 2025 10:47:03 -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>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
"Dr. David Alan Gilbert" <linux@...blig.org>, Yang Li <yang.lee@...ux.alibaba.com>,
James Clark <james.clark@...aro.org>, Thomas Falcon <thomas.falcon@...el.com>,
Thomas Richter <tmricht@...ux.ibm.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/3] perf stat-shadow: Read tool events directly
On Tue, Nov 4, 2025 at 10:10 AM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Hi Ian,
>
> On Mon, Nov 03, 2025 at 09:34:48PM -0800, Ian Rogers wrote:
> > When reading time values for metrics don't use the globals updated in
> > builtin-stat, just read the events as regular events. The only
> > exception is for time events where nanoseconds need converting to
> > seconds as metrics assume time metrics are in seconds.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/perf/util/stat-shadow.c | 123 +++++++++++++---------------------
> > 1 file changed, 45 insertions(+), 78 deletions(-)
> >
> > diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
> > index abaf6b579bfc..9fae3d32a519 100644
> > --- a/tools/perf/util/stat-shadow.c
> > +++ b/tools/perf/util/stat-shadow.c
> > @@ -371,6 +371,18 @@ static void print_nsecs(struct perf_stat_config *config,
> > }
> > }
> >
> > +static double tool_pmu__scale_for_metric(const struct evsel *evsel)
> > +{
> > + enum tool_pmu_event event = evsel__tool_event(evsel);
> > +
> > + if (event == TOOL_PMU__EVENT_DURATION_TIME ||
> > + event == TOOL_PMU__EVENT_USER_TIME ||
> > + event == TOOL_PMU__EVENT_SYSTEM_TIME)
> > + return 1e-9; /* Scale nanoseconds to seconds. */
> > +
> > + return 1.0;
> > +}
> > +
> > static int prepare_metric(const struct metric_expr *mexp,
> > const struct evsel *evsel,
> > struct expr_parse_ctx *pctx,
> > @@ -382,90 +394,45 @@ static int prepare_metric(const struct metric_expr *mexp,
> >
> > for (i = 0; metric_events[i]; i++) {
> > char *n;
> > - double val;
> > + double val, scale = tool_pmu__scale_for_metric(metric_events[i]);
> > int source_count = 0;
> > + struct perf_stat_evsel *ps = metric_events[i]->stats;
> > + struct perf_stat_aggr *aggr;
> >
> > - if (evsel__is_tool(metric_events[i])) {
> > - struct stats *stats;
> > - double scale;
> > -
> > - switch (evsel__tool_event(metric_events[i])) {
> > - case TOOL_PMU__EVENT_DURATION_TIME:
> > - stats = &walltime_nsecs_stats;
> > - scale = 1e-9;
> > - break;
> > - case TOOL_PMU__EVENT_USER_TIME:
> > - stats = &ru_stats.ru_utime_usec_stat;
> > - scale = 1e-6;
> > - break;
> > - case TOOL_PMU__EVENT_SYSTEM_TIME:
> > - stats = &ru_stats.ru_stime_usec_stat;
> > - scale = 1e-6;
>
> Do {USER,SYSTEM}_TIME become nanosecond now?
The behavior is the same. In the event system/user time are read as
microseconds while duration time is nanoseconds, the system/user times
are scaled to be nanoseconds for some kind of consistency. In the
metrics we were doing a similar scaling but scaling nanoseconds and
microseconds to be seconds. Using the events directly we know the
counts are always in nanoseconds so we just need to do 1 scaling
factor.
Thanks,
Ian
> > + /*
> > + * If there are multiple uncore PMUs and we're not reading the
> > + * leader's stats, determine the stats for the appropriate
> > + * uncore PMU.
> > + */
> > + if (evsel && evsel->metric_leader &&
> > + evsel->pmu != evsel->metric_leader->pmu &&
> > + mexp->metric_events[i]->pmu == evsel->metric_leader->pmu) {
> > + struct evsel *pos;
> > +
> > + evlist__for_each_entry(evsel->evlist, pos) {
> > + if (pos->pmu != evsel->pmu)
> > + continue;
> > + if (pos->metric_leader != mexp->metric_events[i])
> > + continue;
> > + ps = pos->stats;
> > + source_count = 1;
> > break;
> > - case TOOL_PMU__EVENT_NONE:
> > - pr_err("Invalid tool event 'none'");
> > - abort();
> > - case TOOL_PMU__EVENT_MAX:
> > - pr_err("Invalid tool event 'max'");
> > - abort();
> > - case TOOL_PMU__EVENT_HAS_PMEM:
> > - case TOOL_PMU__EVENT_NUM_CORES:
> > - case TOOL_PMU__EVENT_NUM_CPUS:
> > - case TOOL_PMU__EVENT_NUM_CPUS_ONLINE:
> > - case TOOL_PMU__EVENT_NUM_DIES:
> > - case TOOL_PMU__EVENT_NUM_PACKAGES:
> > - case TOOL_PMU__EVENT_SLOTS:
> > - case TOOL_PMU__EVENT_SMT_ON:
> > - case TOOL_PMU__EVENT_SYSTEM_TSC_FREQ:
> > - default:
> > - pr_err("Unexpected tool event '%s'", evsel__name(metric_events[i]));
> > - abort();
> > }
> > - val = avg_stats(stats) * scale;
> > - source_count = 1;
> > - } else {
> > - struct perf_stat_evsel *ps = metric_events[i]->stats;
> > - struct perf_stat_aggr *aggr;
> > -
> > + }
> > + aggr = &ps->aggr[aggr_idx];
> > + if (!aggr || !metric_events[i]->supported) {
> > /*
> > - * If there are multiple uncore PMUs and we're not
> > - * reading the leader's stats, determine the stats for
> > - * the appropriate uncore PMU.
> > + * Not supported events will have a count of 0, which
> > + * can be confusing in a metric. Explicitly set the
> > + * value to NAN. Not counted events (enable time of 0)
> > + * are read as 0.
> > */
> > - if (evsel && evsel->metric_leader &&
> > - evsel->pmu != evsel->metric_leader->pmu &&
> > - mexp->metric_events[i]->pmu == evsel->metric_leader->pmu) {
> > - struct evsel *pos;
> > -
> > - evlist__for_each_entry(evsel->evlist, pos) {
> > - if (pos->pmu != evsel->pmu)
> > - continue;
> > - if (pos->metric_leader != mexp->metric_events[i])
> > - continue;
> > - ps = pos->stats;
> > - source_count = 1;
> > - break;
> > - }
> > - }
> > - aggr = &ps->aggr[aggr_idx];
> > - if (!aggr)
> > - break;
> > -
> > - if (!metric_events[i]->supported) {
> > - /*
> > - * Not supported events will have a count of 0,
> > - * which can be confusing in a
> > - * metric. Explicitly set the value to NAN. Not
> > - * counted events (enable time of 0) are read as
> > - * 0.
> > - */
> > - val = NAN;
> > - source_count = 0;
> > - } else {
> > - val = aggr->counts.val;
> > - if (!source_count)
> > - source_count = evsel__source_count(metric_events[i]);
> > - }
> > + val = NAN;
> > + source_count = 0;
> > + } else {
> > + val = aggr->counts.val * scale;
> > + if (!source_count)
> > + source_count = evsel__source_count(metric_events[i]);
> > }
> > n = strdup(evsel__metric_id(metric_events[i]));
> > if (!n)
> > --
> > 2.51.2.1006.ga50a493c49-goog
> >
Powered by blists - more mailing lists