[PATCH 1/3] perf stat: Remove dead code

[PATCH 1/3] perf stat: Remove dead code

[Ian Rogers]

No need to set os.evsel twice.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/stat-display.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 493ec372fdec..4b57c0c07632 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -946,7 +946,6 @@ static void print_metric_headers(struct perf_stat_config *config,
 		out.print_metric = print_metric_header;
 		out.new_line = new_line_metric;
 		out.force_header = true;
-		os.evsel = counter;
 		perf_stat__print_shadow_stats(config, counter, 0,
 					      0,
 					      &out,
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 2/3] perf metricgroup: Fix uncore metric expressions

[Ian Rogers]

A metric like DRAM_BW_Use has on SkylakeX events uncore_imc/cas_count_read/
and uncore_imc/case_count_write/. These events open 6 events per socket
with pmu names of uncore_imc_[0-5]. The current metric setup code in
find_evsel_group assumes one ID will map to 1 event to be recorded in
metric_events. For events with multiple matches, the first event is
recorded in metric_events (avoiding matching >1 event with the same
name) and the evlist_used updated so that duplicate events aren't
removed when the evlist has unused events removed.

Before this change:
$ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1

 Performance counter stats for 'system wide':

             41.14 MiB  uncore_imc/cas_count_read/
     1,002,614,251 ns   duration_time

       1.002614251 seconds time elapsed

After this change:
$ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1

 Performance counter stats for 'system wide':

            157.47 MiB  uncore_imc/cas_count_read/ #     0.00 DRAM_BW_Use
            126.97 MiB  uncore_imc/cas_count_write/
     1,003,019,728 ns   duration_time

Erroneous duplication introduced in:
commit 2440689d62e9 ("perf metricgroup: Remove duped metric group events").

Fixes: ded80bda8bc9 ("perf expr: Migrate expr ids table to a hashmap").
Reported-by: Jin, Yao <yao.jin@linux.intel.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/metricgroup.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 8831b964288f..d216a161f41c 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -206,6 +206,18 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 				sizeof(struct evsel *) * idnum);
 			current_leader = ev->leader;
 		}
+		/*
+		 * Check for duplicate events with the same name. For example,
+		 * uncore_imc/cas_count_read/ will turn into 6 events per socket
+		 * on skylakex. Only the first such event is placed in
+		 * metric_events.
+		 */
+		for (i = 0; i < matched_events; i++) {
+			if (!strcmp(metric_events[i]->name, ev->name))
+				break;
+		}
+		if (i != matched_events)
+			continue;
 		if (hashmap__find(&pctx->ids, ev->name, (void **)&val_ptr)) {
 			if (has_constraint) {
 				/*
@@ -248,6 +260,16 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 		ev = metric_events[i];
 		ev->metric_leader = ev;
 		set_bit(ev->idx, evlist_used);
+		/*
+		 * Mark two events with identical names in the same group as
+		 * being in use as uncore events may be duplicated for each pmu.
+		 */
+		evlist__for_each_entry(perf_evlist, ev) {
+			if (metric_events[i]->leader == ev->leader &&
+			    !strcmp(metric_events[i]->name, ev->name)) {
+				set_bit(ev->idx, evlist_used);
+			}
+		}
 	}
 
 	return metric_events[0];
-- 
2.28.0.526.ge36021eeef-goog

Re: [PATCH 2/3] perf metricgroup: Fix uncore metric expressions

[Namhyung Kim]

Hi Ian,

On Wed, Sep 9, 2020 at 5:02 PM Ian Rogers <irogers@google.com> wrote:
>
> A metric like DRAM_BW_Use has on SkylakeX events uncore_imc/cas_count_read/
> and uncore_imc/case_count_write/. These events open 6 events per socket
> with pmu names of uncore_imc_[0-5]. The current metric setup code in
> find_evsel_group assumes one ID will map to 1 event to be recorded in
> metric_events. For events with multiple matches, the first event is
> recorded in metric_events (avoiding matching >1 event with the same
> name) and the evlist_used updated so that duplicate events aren't
> removed when the evlist has unused events removed.
>
> Before this change:
> $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
>
>  Performance counter stats for 'system wide':
>
>              41.14 MiB  uncore_imc/cas_count_read/
>      1,002,614,251 ns   duration_time
>
>        1.002614251 seconds time elapsed
>
> After this change:
> $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
>
>  Performance counter stats for 'system wide':
>
>             157.47 MiB  uncore_imc/cas_count_read/ #     0.00 DRAM_BW_Use

Hmm.. I guess the 0.00 result is incorrect, no?


>             126.97 MiB  uncore_imc/cas_count_write/
>      1,003,019,728 ns   duration_time
>
> Erroneous duplication introduced in:
> commit 2440689d62e9 ("perf metricgroup: Remove duped metric group events").
>
> Fixes: ded80bda8bc9 ("perf expr: Migrate expr ids table to a hashmap").
> Reported-by: Jin, Yao <yao.jin@linux.intel.com>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
[SNIP]
> @@ -248,6 +260,16 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
>                 ev = metric_events[i];
>                 ev->metric_leader = ev;
>                 set_bit(ev->idx, evlist_used);
> +               /*
> +                * Mark two events with identical names in the same group as
> +                * being in use as uncore events may be duplicated for each pmu.
> +                */
> +               evlist__for_each_entry(perf_evlist, ev) {
> +                       if (metric_events[i]->leader == ev->leader &&
> +                           !strcmp(metric_events[i]->name, ev->name)) {
> +                               set_bit(ev->idx, evlist_used);

I'm not sure whether they are grouped together.
But if so, you can use for_each_group_member(ev, leader).

Thanks
Namhyung


> +                       }
> +               }
>         }
>
>         return metric_events[0];
> --
> 2.28.0.526.ge36021eeef-goog
>

Re: [PATCH 2/3] perf metricgroup: Fix uncore metric expressions

[Ian Rogers]

On Wed, Sep 9, 2020 at 2:53 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Wed, Sep 9, 2020 at 5:02 PM Ian Rogers <irogers@google.com> wrote:
> >
> > A metric like DRAM_BW_Use has on SkylakeX events uncore_imc/cas_count_read/
> > and uncore_imc/case_count_write/. These events open 6 events per socket
> > with pmu names of uncore_imc_[0-5]. The current metric setup code in
> > find_evsel_group assumes one ID will map to 1 event to be recorded in
> > metric_events. For events with multiple matches, the first event is
> > recorded in metric_events (avoiding matching >1 event with the same
> > name) and the evlist_used updated so that duplicate events aren't
> > removed when the evlist has unused events removed.
> >
> > Before this change:
> > $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
> >
> >  Performance counter stats for 'system wide':
> >
> >              41.14 MiB  uncore_imc/cas_count_read/
> >      1,002,614,251 ns   duration_time
> >
> >        1.002614251 seconds time elapsed
> >
> > After this change:
> > $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
> >
> >  Performance counter stats for 'system wide':
> >
> >             157.47 MiB  uncore_imc/cas_count_read/ #     0.00 DRAM_BW_Use
>
> Hmm.. I guess the 0.00 result is incorrect, no?

Agreed. There are a number of pre-existing bugs in this code. I'll try
to look into this one.

> >             126.97 MiB  uncore_imc/cas_count_write/
> >      1,003,019,728 ns   duration_time
> >
> > Erroneous duplication introduced in:
> > commit 2440689d62e9 ("perf metricgroup: Remove duped metric group events").
> >
> > Fixes: ded80bda8bc9 ("perf expr: Migrate expr ids table to a hashmap").
> > Reported-by: Jin, Yao <yao.jin@linux.intel.com>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> [SNIP]
> > @@ -248,6 +260,16 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> >                 ev = metric_events[i];
> >                 ev->metric_leader = ev;
> >                 set_bit(ev->idx, evlist_used);
> > +               /*
> > +                * Mark two events with identical names in the same group as
> > +                * being in use as uncore events may be duplicated for each pmu.
> > +                */
> > +               evlist__for_each_entry(perf_evlist, ev) {
> > +                       if (metric_events[i]->leader == ev->leader &&
> > +                           !strcmp(metric_events[i]->name, ev->name)) {
> > +                               set_bit(ev->idx, evlist_used);
>
> I'm not sure whether they are grouped together.
> But if so, you can use for_each_group_member(ev, leader).

Good suggestion, unfortunately the groups may be removed for things
like NMI watchdog or --metric-no-group and so there wouldn't be a
leader to follow. We could do something like this:

if (metric_events[i]->leader) {
  for_each_group_member(ev, leader) {
    if (!strcmp(metric_events[i]->name, ev->name))
      set_bit(ev->idx, evlist_used);
  }
} else {
  evlist__for_each_entry(perf_evlist, ev) {
    if (!ev->leader && !strcmp(metric_events[i]->name, ev->name))
      set_bit(ev->idx, evlist_used);
  }
 }
}

What do you think?

Thanks,
Ian

> Thanks
> Namhyung
>
>
> > +                       }
> > +               }
> >         }
> >
> >         return metric_events[0];
> > --
> > 2.28.0.526.ge36021eeef-goog
> >

Re: [PATCH 2/3] perf metricgroup: Fix uncore metric expressions

[Ian Rogers]

On Wed, Sep 9, 2020 at 10:52 AM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Sep 9, 2020 at 2:53 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hi Ian,
> >
> > On Wed, Sep 9, 2020 at 5:02 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > A metric like DRAM_BW_Use has on SkylakeX events uncore_imc/cas_count_read/
> > > and uncore_imc/case_count_write/. These events open 6 events per socket
> > > with pmu names of uncore_imc_[0-5]. The current metric setup code in
> > > find_evsel_group assumes one ID will map to 1 event to be recorded in
> > > metric_events. For events with multiple matches, the first event is
> > > recorded in metric_events (avoiding matching >1 event with the same
> > > name) and the evlist_used updated so that duplicate events aren't
> > > removed when the evlist has unused events removed.
> > >
> > > Before this change:
> > > $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
> > >
> > >  Performance counter stats for 'system wide':
> > >
> > >              41.14 MiB  uncore_imc/cas_count_read/
> > >      1,002,614,251 ns   duration_time
> > >
> > >        1.002614251 seconds time elapsed
> > >
> > > After this change:
> > > $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
> > >
> > >  Performance counter stats for 'system wide':
> > >
> > >             157.47 MiB  uncore_imc/cas_count_read/ #     0.00 DRAM_BW_Use
> >
> > Hmm.. I guess the 0.00 result is incorrect, no?
>
> Agreed. There are a number of pre-existing bugs in this code. I'll try
> to look into this one.

There was a bug where the metric_leader wasn't being set correctly and
so the accumulation of values wasn't working as expected. There's a
small fix in:
https://lore.kernel.org/lkml/20200910032632.511566-3-irogers@google.com/T/#u
that also does the change I mentioned below.

The metric still remains at 0.0 following this change as I believe
there is a bug in the metric. The metric expression is:
"( 64 * ( uncore_imc@cas_count_read@ + uncore_imc@cas_count_write@ ) /
1000000000 ) / duration_time"
ie the counts of uncore_imc/cas_count_read/ and
uncore_imc/cas_count_write/ are being first being scaled up by 64,
that is to turn a count of cache lines into bytes, the count is then
divided by 1000000000 or a GB to supposedly give GB/s. However, the
counts are read and scaled:

...
void perf_stat__update_shadow_stats(struct evsel *counter, u64 count,
...
  count *= counter->scale;
...

The scale value from
/sys/devices/uncore_imc_0/events/cas_count_read.scale is
6.103515625e-5 or 64/(1024*1024). This means the result of the
expression is 64 times too large but in PB/s and so too small to
display. I don't rule out there being other issues but this appears to
be a significant one. Printing using intervals also looks suspicious
as the count appears to just increase rather than vary up and down.
Jin Yao, I don't know if you can take a look?

Thanks,
Ian

> > >             126.97 MiB  uncore_imc/cas_count_write/
> > >      1,003,019,728 ns   duration_time
> > >
> > > Erroneous duplication introduced in:
> > > commit 2440689d62e9 ("perf metricgroup: Remove duped metric group events").
> > >
> > > Fixes: ded80bda8bc9 ("perf expr: Migrate expr ids table to a hashmap").
> > > Reported-by: Jin, Yao <yao.jin@linux.intel.com>
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > [SNIP]
> > > @@ -248,6 +260,16 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> > >                 ev = metric_events[i];
> > >                 ev->metric_leader = ev;
> > >                 set_bit(ev->idx, evlist_used);
> > > +               /*
> > > +                * Mark two events with identical names in the same group as
> > > +                * being in use as uncore events may be duplicated for each pmu.
> > > +                */
> > > +               evlist__for_each_entry(perf_evlist, ev) {
> > > +                       if (metric_events[i]->leader == ev->leader &&
> > > +                           !strcmp(metric_events[i]->name, ev->name)) {
> > > +                               set_bit(ev->idx, evlist_used);
> >
> > I'm not sure whether they are grouped together.
> > But if so, you can use for_each_group_member(ev, leader).
>
> Good suggestion, unfortunately the groups may be removed for things
> like NMI watchdog or --metric-no-group and so there wouldn't be a
> leader to follow. We could do something like this:
>
> if (metric_events[i]->leader) {
>   for_each_group_member(ev, leader) {
>     if (!strcmp(metric_events[i]->name, ev->name))
>       set_bit(ev->idx, evlist_used);
>   }
> } else {
>   evlist__for_each_entry(perf_evlist, ev) {
>     if (!ev->leader && !strcmp(metric_events[i]->name, ev->name))
>       set_bit(ev->idx, evlist_used);
>   }
>  }
> }
>
> What do you think?
>
> Thanks,
> Ian
>
> > Thanks
> > Namhyung
> >
> >
> > > +                       }
> > > +               }
> > >         }
> > >
> > >         return metric_events[0];
> > > --
> > > 2.28.0.526.ge36021eeef-goog
> > >

Re: [PATCH 2/3] perf metricgroup: Fix uncore metric expressions

[Namhyung Kim]

On Thu, Sep 10, 2020 at 12:41 PM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Sep 9, 2020 at 10:52 AM Ian Rogers <irogers@google.com> wrote:
> >
> > On Wed, Sep 9, 2020 at 2:53 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > Hi Ian,
> > >
> > > On Wed, Sep 9, 2020 at 5:02 PM Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > A metric like DRAM_BW_Use has on SkylakeX events uncore_imc/cas_count_read/
> > > > and uncore_imc/case_count_write/. These events open 6 events per socket
> > > > with pmu names of uncore_imc_[0-5]. The current metric setup code in
> > > > find_evsel_group assumes one ID will map to 1 event to be recorded in
> > > > metric_events. For events with multiple matches, the first event is
> > > > recorded in metric_events (avoiding matching >1 event with the same
> > > > name) and the evlist_used updated so that duplicate events aren't
> > > > removed when the evlist has unused events removed.
> > > >
> > > > Before this change:
> > > > $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
> > > >
> > > >  Performance counter stats for 'system wide':
> > > >
> > > >              41.14 MiB  uncore_imc/cas_count_read/
> > > >      1,002,614,251 ns   duration_time
> > > >
> > > >        1.002614251 seconds time elapsed
> > > >
> > > > After this change:
> > > > $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
> > > >
> > > >  Performance counter stats for 'system wide':
> > > >
> > > >             157.47 MiB  uncore_imc/cas_count_read/ #     0.00 DRAM_BW_Use
> > >
> > > Hmm.. I guess the 0.00 result is incorrect, no?
> >
> > Agreed. There are a number of pre-existing bugs in this code. I'll try
> > to look into this one.
>
> There was a bug where the metric_leader wasn't being set correctly and
> so the accumulation of values wasn't working as expected. There's a
> small fix in:
> https://lore.kernel.org/lkml/20200910032632.511566-3-irogers@google.com/T/#u
> that also does the change I mentioned below.
>
> The metric still remains at 0.0 following this change as I believe
> there is a bug in the metric. The metric expression is:
> "( 64 * ( uncore_imc@cas_count_read@ + uncore_imc@cas_count_write@ ) /
> 1000000000 ) / duration_time"
> ie the counts of uncore_imc/cas_count_read/ and
> uncore_imc/cas_count_write/ are being first being scaled up by 64,
> that is to turn a count of cache lines into bytes, the count is then
> divided by 1000000000 or a GB to supposedly give GB/s. However, the
> counts are read and scaled:
>
> ...
> void perf_stat__update_shadow_stats(struct evsel *counter, u64 count,
> ...
>   count *= counter->scale;
> ...
>
> The scale value from
> /sys/devices/uncore_imc_0/events/cas_count_read.scale is
> 6.103515625e-5 or 64/(1024*1024). This means the result of the
> expression is 64 times too large but in PB/s and so too small to
> display. I don't rule out there being other issues but this appears to
> be a significant one. Printing using intervals also looks suspicious
> as the count appears to just increase rather than vary up and down.

You're right!

Maybe we can change it to simply like below and change the unit to MiB/s?

( uncore_imc@cas_count_read@ + uncore_imc@cas_count_write@ ) / duration_time

Thanks
Namhyung

Re: [PATCH 2/3] perf metricgroup: Fix uncore metric expressions

[Ian Rogers]

On Wed, Sep 9, 2020 at 1:33 AM Jin, Yao <yao.jin@linux.intel.com> wrote:
>
>
>
> On 9/9/2020 4:02 PM, Ian Rogers wrote:
> > A metric like DRAM_BW_Use has on SkylakeX events uncore_imc/cas_count_read/
> > and uncore_imc/case_count_write/. These events open 6 events per socket
> > with pmu names of uncore_imc_[0-5]. The current metric setup code in
> > find_evsel_group assumes one ID will map to 1 event to be recorded in
> > metric_events. For events with multiple matches, the first event is
> > recorded in metric_events (avoiding matching >1 event with the same
> > name) and the evlist_used updated so that duplicate events aren't
> > removed when the evlist has unused events removed.
> >
> > Before this change:
> > $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
> >
> >   Performance counter stats for 'system wide':
> >
> >               41.14 MiB  uncore_imc/cas_count_read/
> >       1,002,614,251 ns   duration_time
> >
> >         1.002614251 seconds time elapsed
> >
> > After this change:
> > $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
> >
> >   Performance counter stats for 'system wide':
> >
> >              157.47 MiB  uncore_imc/cas_count_read/ #     0.00 DRAM_BW_Use
> >              126.97 MiB  uncore_imc/cas_count_write/
> >       1,003,019,728 ns   duration_time
> >
> > Erroneous duplication introduced in:
> > commit 2440689d62e9 ("perf metricgroup: Remove duped metric group events").
> >
> > Fixes: ded80bda8bc9 ("perf expr: Migrate expr ids table to a hashmap").
> > Reported-by: Jin, Yao <yao.jin@linux.intel.com>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >   tools/perf/util/metricgroup.c | 22 ++++++++++++++++++++++
> >   1 file changed, 22 insertions(+)
> >
> > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > index 8831b964288f..d216a161f41c 100644
> > --- a/tools/perf/util/metricgroup.c
> > +++ b/tools/perf/util/metricgroup.c
> > @@ -206,6 +206,18 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> >                               sizeof(struct evsel *) * idnum);
> >                       current_leader = ev->leader;
> >               }
> > +             /*
> > +              * Check for duplicate events with the same name. For example,
> > +              * uncore_imc/cas_count_read/ will turn into 6 events per socket
> > +              * on skylakex. Only the first such event is placed in
> > +              * metric_events.
> > +              */
> > +             for (i = 0; i < matched_events; i++) {
> > +                     if (!strcmp(metric_events[i]->name, ev->name))
> > +                             break;
> > +             }
> > +             if (i != matched_events)
> > +                     continue;
> >               if (hashmap__find(&pctx->ids, ev->name, (void **)&val_ptr)) {
> >                       if (has_constraint) {
> >                               /*
> > @@ -248,6 +260,16 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> >               ev = metric_events[i];
> >               ev->metric_leader = ev;
> >               set_bit(ev->idx, evlist_used);
> > +             /*
> > +              * Mark two events with identical names in the same group as
> > +              * being in use as uncore events may be duplicated for each pmu.
> > +              */
> > +             evlist__for_each_entry(perf_evlist, ev) {
> > +                     if (metric_events[i]->leader == ev->leader &&
> > +                         !strcmp(metric_events[i]->name, ev->name)) {
>
> Do we really need this '!strcmp(metric_events[i]->name, ev->name)'?

When there are multiple metrics being computed then we end up parsing
multiple groups of events, something like
"{...}:W,{....}:W,{....}:W...". The code sorts the groups so that
those with the largest number of events appear at the front, it then
matches metrics against the list trying to reuse events at the front
of the list first (the selection must respect the groupings). It's not
a particularly sophisticated algorithm, but it reduces multiplexing
for say top down metrics. We need to remove the unused events/groups
from the evlist to achieve this benefit. If there were some events
like "{instructions,cycles}:W" but we only matched "instructions"
within the group, then we would want to eliminate "cycles" from the
group - the strcmp achieves this, otherwise we'd mark everything in
the group as in use. Now, the "cycles" will likely be marked live by
another metric, that's how we got the group in the first place, but
marking it live here strikes me as a little odd as the event wouldn't
be being used by the metric. A case where this problem could happen is
with --metric-no-group, where all the events multiplex freely with
each other as they aren't grouped. We'd like to eliminate duplicated
events in the evlist, but if we mark them all live this won't happen.

Sorry for the long explanation :-) TL;DR I think it is necessary.

Thanks,
Ian

> Thanks
> Jin Yao
>
> > +                             set_bit(ev->idx, evlist_used);
> > +                     }
> > +             }
> >       }
> >
> >       return metric_events[0];
> >

[PATCH 3/3] perf metricgroup: Fix typo in comment.

[Ian Rogers]

Add missing character.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/metricgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index d216a161f41c..050f3404c137 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -150,7 +150,7 @@ static void expr_ids__exit(struct expr_ids *ids)
 }
 
 /**
- * Find a group of events in perf_evlist that correpond to those from a parsed
+ * Find a group of events in perf_evlist that correspond to those from a parsed
  * metric expression. Note, as find_evsel_group is called in the same order as
  * perf_evlist was constructed, metric_no_merge doesn't need to test for
  * underfilling a group.
-- 
2.28.0.526.ge36021eeef-goog