* [PATCH] perf pmu: Fix perf stat output with correct scale and unit
@ 2023-09-20 12:23 Wyes Karny
2023-09-20 16:02 ` Ian Rogers
0 siblings, 1 reply; 6+ messages in thread
From: Wyes Karny @ 2023-09-20 12:23 UTC (permalink / raw)
To: irogers, peterz, mingo, acme
Cc: mark.rutland, alexander.shishkin, jolsa, namhyung, adrian.hunter,
kan.liang, james.clark, linux-perf-users, linux-kernel,
sandipan.das, ravi.bangoria, Wyes Karny
The perf_pmu__parse_* functions for the sysfs files of pmu event’s
scale, unit, per-pkg and snapshot were updated in commit 7b723dbb96e8
("perf pmu: Be lazy about loading event info files from sysfs").
However, the paths for these sysfs files were incorrect. This resulted
in perf stat reporting values with wrong scaling and missing units. This
is fixed by correcting the paths for these sysfs files.
Before this fix:
$sudo perf stat -e power/energy-pkg/ -- sleep 2
Performance counter stats for 'system wide':
351,217,188,864 power/energy-pkg/
2.004127961 seconds time elapsed
After this fix:
$sudo perf stat -e power/energy-pkg/ -- sleep 2
Performance counter stats for 'system wide':
80.58 Joules power/energy-pkg/
2.004009749 seconds time elapsed
Fixes: 7b723dbb96e8 ("perf pmu: Be lazy about loading event info files from sysfs")
Signed-off-by: Wyes Karny <wyes.karny@amd.com>
---
tools/perf/util/pmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 0d81c059c91c..93feee29d054 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -295,7 +295,7 @@ static int perf_pmu__parse_scale(struct perf_pmu *pmu, struct perf_pmu_alias *al
len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
if (!len)
return 0;
- scnprintf(path + len, sizeof(path) - len, "%s/%s.scale", pmu->name, alias->name);
+ scnprintf(path + len, sizeof(path) - len, "%s/events/%s.scale", pmu->name, alias->name);
fd = open(path, O_RDONLY);
if (fd == -1)
@@ -330,7 +330,7 @@ static int perf_pmu__parse_unit(struct perf_pmu *pmu, struct perf_pmu_alias *ali
len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
if (!len)
return 0;
- scnprintf(path + len, sizeof(path) - len, "%s/%s.unit", pmu->name, alias->name);
+ scnprintf(path + len, sizeof(path) - len, "%s/events/%s.unit", pmu->name, alias->name);
fd = open(path, O_RDONLY);
if (fd == -1)
@@ -364,7 +364,7 @@ perf_pmu__parse_per_pkg(struct perf_pmu *pmu, struct perf_pmu_alias *alias)
len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
if (!len)
return 0;
- scnprintf(path + len, sizeof(path) - len, "%s/%s.per-pkg", pmu->name, alias->name);
+ scnprintf(path + len, sizeof(path) - len, "%s/events/%s.per-pkg", pmu->name, alias->name);
fd = open(path, O_RDONLY);
if (fd == -1)
@@ -385,7 +385,7 @@ static int perf_pmu__parse_snapshot(struct perf_pmu *pmu, struct perf_pmu_alias
len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
if (!len)
return 0;
- scnprintf(path + len, sizeof(path) - len, "%s/%s.snapshot", pmu->name, alias->name);
+ scnprintf(path + len, sizeof(path) - len, "%s/events/%s.snapshot", pmu->name, alias->name);
fd = open(path, O_RDONLY);
if (fd == -1)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf pmu: Fix perf stat output with correct scale and unit
2023-09-20 12:23 [PATCH] perf pmu: Fix perf stat output with correct scale and unit Wyes Karny
@ 2023-09-20 16:02 ` Ian Rogers
2023-09-26 22:50 ` Namhyung Kim
0 siblings, 1 reply; 6+ messages in thread
From: Ian Rogers @ 2023-09-20 16:02 UTC (permalink / raw)
To: Wyes Karny
Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, adrian.hunter, kan.liang, james.clark,
linux-perf-users, linux-kernel, sandipan.das, ravi.bangoria
On Wed, Sep 20, 2023 at 5:24 AM Wyes Karny <wyes.karny@amd.com> wrote:
>
> The perf_pmu__parse_* functions for the sysfs files of pmu event’s
> scale, unit, per-pkg and snapshot were updated in commit 7b723dbb96e8
> ("perf pmu: Be lazy about loading event info files from sysfs").
> However, the paths for these sysfs files were incorrect. This resulted
> in perf stat reporting values with wrong scaling and missing units. This
> is fixed by correcting the paths for these sysfs files.
>
> Before this fix:
>
> $sudo perf stat -e power/energy-pkg/ -- sleep 2
>
> Performance counter stats for 'system wide':
>
> 351,217,188,864 power/energy-pkg/
>
> 2.004127961 seconds time elapsed
>
> After this fix:
>
> $sudo perf stat -e power/energy-pkg/ -- sleep 2
>
> Performance counter stats for 'system wide':
>
> 80.58 Joules power/energy-pkg/
>
> 2.004009749 seconds time elapsed
>
> Fixes: 7b723dbb96e8 ("perf pmu: Be lazy about loading event info files from sysfs")
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>
Reviewed-by: Ian Rogers <irogers@google.com>
Thanks!
Ian
> ---
> tools/perf/util/pmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 0d81c059c91c..93feee29d054 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -295,7 +295,7 @@ static int perf_pmu__parse_scale(struct perf_pmu *pmu, struct perf_pmu_alias *al
> len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
> if (!len)
> return 0;
> - scnprintf(path + len, sizeof(path) - len, "%s/%s.scale", pmu->name, alias->name);
> + scnprintf(path + len, sizeof(path) - len, "%s/events/%s.scale", pmu->name, alias->name);
>
> fd = open(path, O_RDONLY);
> if (fd == -1)
> @@ -330,7 +330,7 @@ static int perf_pmu__parse_unit(struct perf_pmu *pmu, struct perf_pmu_alias *ali
> len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
> if (!len)
> return 0;
> - scnprintf(path + len, sizeof(path) - len, "%s/%s.unit", pmu->name, alias->name);
> + scnprintf(path + len, sizeof(path) - len, "%s/events/%s.unit", pmu->name, alias->name);
>
> fd = open(path, O_RDONLY);
> if (fd == -1)
> @@ -364,7 +364,7 @@ perf_pmu__parse_per_pkg(struct perf_pmu *pmu, struct perf_pmu_alias *alias)
> len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
> if (!len)
> return 0;
> - scnprintf(path + len, sizeof(path) - len, "%s/%s.per-pkg", pmu->name, alias->name);
> + scnprintf(path + len, sizeof(path) - len, "%s/events/%s.per-pkg", pmu->name, alias->name);
>
> fd = open(path, O_RDONLY);
> if (fd == -1)
> @@ -385,7 +385,7 @@ static int perf_pmu__parse_snapshot(struct perf_pmu *pmu, struct perf_pmu_alias
> len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
> if (!len)
> return 0;
> - scnprintf(path + len, sizeof(path) - len, "%s/%s.snapshot", pmu->name, alias->name);
> + scnprintf(path + len, sizeof(path) - len, "%s/events/%s.snapshot", pmu->name, alias->name);
>
> fd = open(path, O_RDONLY);
> if (fd == -1)
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf pmu: Fix perf stat output with correct scale and unit
2023-09-20 16:02 ` Ian Rogers
@ 2023-09-26 22:50 ` Namhyung Kim
2023-09-27 4:06 ` Ian Rogers
0 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2023-09-26 22:50 UTC (permalink / raw)
To: Ian Rogers
Cc: Wyes Karny, peterz, mingo, acme, mark.rutland,
alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark,
linux-perf-users, linux-kernel, sandipan.das, ravi.bangoria
On Wed, Sep 20, 2023 at 9:02 AM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Sep 20, 2023 at 5:24 AM Wyes Karny <wyes.karny@amd.com> wrote:
> >
> > The perf_pmu__parse_* functions for the sysfs files of pmu event’s
> > scale, unit, per-pkg and snapshot were updated in commit 7b723dbb96e8
> > ("perf pmu: Be lazy about loading event info files from sysfs").
> > However, the paths for these sysfs files were incorrect. This resulted
> > in perf stat reporting values with wrong scaling and missing units. This
> > is fixed by correcting the paths for these sysfs files.
> >
> > Before this fix:
> >
> > $sudo perf stat -e power/energy-pkg/ -- sleep 2
> >
> > Performance counter stats for 'system wide':
> >
> > 351,217,188,864 power/energy-pkg/
> >
> > 2.004127961 seconds time elapsed
> >
> > After this fix:
> >
> > $sudo perf stat -e power/energy-pkg/ -- sleep 2
> >
> > Performance counter stats for 'system wide':
> >
> > 80.58 Joules power/energy-pkg/
> >
> > 2.004009749 seconds time elapsed
> >
> > Fixes: 7b723dbb96e8 ("perf pmu: Be lazy about loading event info files from sysfs")
> > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
>
> Reviewed-by: Ian Rogers <irogers@google.com>
Applied to perf-tools-next, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf pmu: Fix perf stat output with correct scale and unit
2023-09-26 22:50 ` Namhyung Kim
@ 2023-09-27 4:06 ` Ian Rogers
2023-09-27 4:39 ` Namhyung Kim
0 siblings, 1 reply; 6+ messages in thread
From: Ian Rogers @ 2023-09-27 4:06 UTC (permalink / raw)
To: Namhyung Kim
Cc: Wyes Karny, peterz, mingo, acme, mark.rutland,
alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark,
linux-perf-users, linux-kernel, sandipan.das, ravi.bangoria
On Tue, Sep 26, 2023 at 3:51 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Sep 20, 2023 at 9:02 AM Ian Rogers <irogers@google.com> wrote:
> >
> > On Wed, Sep 20, 2023 at 5:24 AM Wyes Karny <wyes.karny@amd.com> wrote:
> > >
> > > The perf_pmu__parse_* functions for the sysfs files of pmu event’s
> > > scale, unit, per-pkg and snapshot were updated in commit 7b723dbb96e8
> > > ("perf pmu: Be lazy about loading event info files from sysfs").
> > > However, the paths for these sysfs files were incorrect. This resulted
> > > in perf stat reporting values with wrong scaling and missing units. This
> > > is fixed by correcting the paths for these sysfs files.
> > >
> > > Before this fix:
> > >
> > > $sudo perf stat -e power/energy-pkg/ -- sleep 2
> > >
> > > Performance counter stats for 'system wide':
> > >
> > > 351,217,188,864 power/energy-pkg/
> > >
> > > 2.004127961 seconds time elapsed
> > >
> > > After this fix:
> > >
> > > $sudo perf stat -e power/energy-pkg/ -- sleep 2
> > >
> > > Performance counter stats for 'system wide':
> > >
> > > 80.58 Joules power/energy-pkg/
> > >
> > > 2.004009749 seconds time elapsed
> > >
> > > Fixes: 7b723dbb96e8 ("perf pmu: Be lazy about loading event info files from sysfs")
> > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> >
> > Reviewed-by: Ian Rogers <irogers@google.com>
>
> Applied to perf-tools-next, thanks!
Thanks Namhyung, I think this should be a candidate for perf-tools so
that 6.6 doesn't regress for sysfs events.
Ian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf pmu: Fix perf stat output with correct scale and unit
2023-09-27 4:06 ` Ian Rogers
@ 2023-09-27 4:39 ` Namhyung Kim
2023-09-30 6:56 ` Namhyung Kim
0 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2023-09-27 4:39 UTC (permalink / raw)
To: Ian Rogers
Cc: Wyes Karny, peterz, mingo, acme, mark.rutland,
alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark,
linux-perf-users, linux-kernel, sandipan.das, ravi.bangoria
Hi Ian,
On Tue, Sep 26, 2023 at 9:07 PM Ian Rogers <irogers@google.com> wrote:
>
> On Tue, Sep 26, 2023 at 3:51 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Wed, Sep 20, 2023 at 9:02 AM Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Wed, Sep 20, 2023 at 5:24 AM Wyes Karny <wyes.karny@amd.com> wrote:
> > > >
> > > > The perf_pmu__parse_* functions for the sysfs files of pmu event’s
> > > > scale, unit, per-pkg and snapshot were updated in commit 7b723dbb96e8
> > > > ("perf pmu: Be lazy about loading event info files from sysfs").
> > > > However, the paths for these sysfs files were incorrect. This resulted
> > > > in perf stat reporting values with wrong scaling and missing units. This
> > > > is fixed by correcting the paths for these sysfs files.
> > > >
> > > > Before this fix:
> > > >
> > > > $sudo perf stat -e power/energy-pkg/ -- sleep 2
> > > >
> > > > Performance counter stats for 'system wide':
> > > >
> > > > 351,217,188,864 power/energy-pkg/
> > > >
> > > > 2.004127961 seconds time elapsed
> > > >
> > > > After this fix:
> > > >
> > > > $sudo perf stat -e power/energy-pkg/ -- sleep 2
> > > >
> > > > Performance counter stats for 'system wide':
> > > >
> > > > 80.58 Joules power/energy-pkg/
> > > >
> > > > 2.004009749 seconds time elapsed
> > > >
> > > > Fixes: 7b723dbb96e8 ("perf pmu: Be lazy about loading event info files from sysfs")
> > > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > >
> > > Reviewed-by: Ian Rogers <irogers@google.com>
> >
> > Applied to perf-tools-next, thanks!
>
> Thanks Namhyung, I think this should be a candidate for perf-tools so
> that 6.6 doesn't regress for sysfs events.
I see, I overlooked that it's for the current cycle.
I'll move it to perf-tools.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf pmu: Fix perf stat output with correct scale and unit
2023-09-27 4:39 ` Namhyung Kim
@ 2023-09-30 6:56 ` Namhyung Kim
0 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2023-09-30 6:56 UTC (permalink / raw)
To: Ian Rogers
Cc: Wyes Karny, peterz, mingo, acme, mark.rutland,
alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark,
linux-perf-users, linux-kernel, sandipan.das, ravi.bangoria
On Tue, Sep 26, 2023 at 9:39 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Tue, Sep 26, 2023 at 9:07 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Tue, Sep 26, 2023 at 3:51 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > On Wed, Sep 20, 2023 at 9:02 AM Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > On Wed, Sep 20, 2023 at 5:24 AM Wyes Karny <wyes.karny@amd.com> wrote:
> > > > >
> > > > > The perf_pmu__parse_* functions for the sysfs files of pmu event’s
> > > > > scale, unit, per-pkg and snapshot were updated in commit 7b723dbb96e8
> > > > > ("perf pmu: Be lazy about loading event info files from sysfs").
> > > > > However, the paths for these sysfs files were incorrect. This resulted
> > > > > in perf stat reporting values with wrong scaling and missing units. This
> > > > > is fixed by correcting the paths for these sysfs files.
> > > > >
> > > > > Before this fix:
> > > > >
> > > > > $sudo perf stat -e power/energy-pkg/ -- sleep 2
> > > > >
> > > > > Performance counter stats for 'system wide':
> > > > >
> > > > > 351,217,188,864 power/energy-pkg/
> > > > >
> > > > > 2.004127961 seconds time elapsed
> > > > >
> > > > > After this fix:
> > > > >
> > > > > $sudo perf stat -e power/energy-pkg/ -- sleep 2
> > > > >
> > > > > Performance counter stats for 'system wide':
> > > > >
> > > > > 80.58 Joules power/energy-pkg/
> > > > >
> > > > > 2.004009749 seconds time elapsed
> > > > >
> > > > > Fixes: 7b723dbb96e8 ("perf pmu: Be lazy about loading event info files from sysfs")
> > > > > Signed-off-by: Wyes Karny <wyes.karny@amd.com>
> > > >
> > > > Reviewed-by: Ian Rogers <irogers@google.com>
> > >
> > > Applied to perf-tools-next, thanks!
> >
> > Thanks Namhyung, I think this should be a candidate for perf-tools so
> > that 6.6 doesn't regress for sysfs events.
>
> I see, I overlooked that it's for the current cycle.
> I'll move it to perf-tools.
Applied to perf-tools, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-30 6:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-20 12:23 [PATCH] perf pmu: Fix perf stat output with correct scale and unit Wyes Karny
2023-09-20 16:02 ` Ian Rogers
2023-09-26 22:50 ` Namhyung Kim
2023-09-27 4:06 ` Ian Rogers
2023-09-27 4:39 ` Namhyung Kim
2023-09-30 6:56 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®