[PATCH] perf/x86/amd/core: Rename branch-brs event to not break the parser

[PATCH] perf/x86/amd/core: Rename branch-brs event to not break the parser

[Michael Petlan]

The problem is described in detail in 146edff3d7ed ("perf test: Parse
events workaround for dash/minus"). In short, although perf-list offers
"branch-brs" event to be used, when used with "-e" switch, the parser
does not accept it and needs to be enclosed to "cpu//", such as:

```
  $ perf stat -e branch-brs true
  event syntax error: 'branch-brs'
                             \___ parser error

  $ perf stat -e cpu/branch-brs/ true

   Performance counter stats for 'true':

              46,179      cpu/branch-brs/
```

This is confusing for the users.

Rename the event using underscore instead of dash to avoid the problem.

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
---
 arch/x86/events/amd/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index abadd5f23425..f4c5d4100035 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -1286,21 +1286,21 @@ amd_branches_is_visible(struct kobject *kobj, struct attribute *attr, int i)
 }
 
 static struct attribute_group group_caps_amd_branches = {
 	.name  = "caps",
 	.attrs = amd_pmu_branches_attrs,
 	.is_visible = amd_branches_is_visible,
 };
 
 #ifdef CONFIG_PERF_EVENTS_AMD_BRS
 
-EVENT_ATTR_STR(branch-brs, amd_branch_brs,
+EVENT_ATTR_STR(branch_brs, amd_branch_brs,
 	       "event=" __stringify(AMD_FAM19H_BRS_EVENT)"\n");
 
 static struct attribute *amd_brs_events_attrs[] = {
 	EVENT_PTR(amd_branch_brs),
 	NULL,
 };
 
 static umode_t
 amd_brs_is_visible(struct kobject *kobj, struct attribute *attr, int i)
 {
-- 
2.18.4

Re: [PATCH] perf/x86/amd/core: Rename branch-brs event to not break the parser

[Ian Rogers]

On Tue, Sep 12, 2023 at 6:35 AM Michael Petlan <mpetlan@redhat.com> wrote:
>
> The problem is described in detail in 146edff3d7ed ("perf test: Parse
> events workaround for dash/minus"). In short, although perf-list offers
> "branch-brs" event to be used, when used with "-e" switch, the parser
> does not accept it and needs to be enclosed to "cpu//", such as:
>
> ```
>   $ perf stat -e branch-brs true
>   event syntax error: 'branch-brs'
>                              \___ parser error
>
>   $ perf stat -e cpu/branch-brs/ true
>
>    Performance counter stats for 'true':
>
>               46,179      cpu/branch-brs/
> ```
>
> This is confusing for the users.
>
> Rename the event using underscore instead of dash to avoid the problem.
>
> Signed-off-by: Michael Petlan <mpetlan@redhat.com>

I rewrote the event parsing and this was fixed in 70c90e4a6b2f ("perf
parse-events: Avoid scanning PMUs before parsing"):
https://lore.kernel.org/r/20230502223851.2234828-19-irogers@google.com
which was included in the perf tool in Linux 6.5.

Thanks,
Ian

> ---
>  arch/x86/events/amd/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index abadd5f23425..f4c5d4100035 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -1286,21 +1286,21 @@ amd_branches_is_visible(struct kobject *kobj, struct attribute *attr, int i)
>  }
>
>  static struct attribute_group group_caps_amd_branches = {
>         .name  = "caps",
>         .attrs = amd_pmu_branches_attrs,
>         .is_visible = amd_branches_is_visible,
>  };
>
>  #ifdef CONFIG_PERF_EVENTS_AMD_BRS
>
> -EVENT_ATTR_STR(branch-brs, amd_branch_brs,
> +EVENT_ATTR_STR(branch_brs, amd_branch_brs,
>                "event=" __stringify(AMD_FAM19H_BRS_EVENT)"\n");
>
>  static struct attribute *amd_brs_events_attrs[] = {
>         EVENT_PTR(amd_branch_brs),
>         NULL,
>  };
>
>  static umode_t
>  amd_brs_is_visible(struct kobject *kobj, struct attribute *attr, int i)
>  {
> --
> 2.18.4
>

Re: [PATCH] perf/x86/amd/core: Rename branch-brs event to not break the parser

[Michael Petlan]

[-- Attachment #1: Type: text/plain, Size: 2277 bytes --]

On Tue, 12 Sep 2023, Ian Rogers wrote:
> On Tue, Sep 12, 2023 at 6:35 AM Michael Petlan <mpetlan@redhat.com> wrote:
> >
> > The problem is described in detail in 146edff3d7ed ("perf test: Parse
> > events workaround for dash/minus"). In short, although perf-list offers
> > "branch-brs" event to be used, when used with "-e" switch, the parser
> > does not accept it and needs to be enclosed to "cpu//", such as:
> >
> > ```
> >   $ perf stat -e branch-brs true
> >   event syntax error: 'branch-brs'
> >                              \___ parser error
> >
> >   $ perf stat -e cpu/branch-brs/ true
> >
> >    Performance counter stats for 'true':
> >
> >               46,179      cpu/branch-brs/
> > ```
> >
> > This is confusing for the users.
> >
> > Rename the event using underscore instead of dash to avoid the problem.
> >
> > Signed-off-by: Michael Petlan <mpetlan@redhat.com>
> 
> I rewrote the event parsing and this was fixed in 70c90e4a6b2f ("perf
> parse-events: Avoid scanning PMUs before parsing"):
> https://lore.kernel.org/r/20230502223851.2234828-19-irogers@google.com
> which was included in the perf tool in Linux 6.5.
> 
> Thanks,
> Ian

OK, great to know, thanks!
Michael

> 
> > ---
> >  arch/x86/events/amd/core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> > index abadd5f23425..f4c5d4100035 100644
> > --- a/arch/x86/events/amd/core.c
> > +++ b/arch/x86/events/amd/core.c
> > @@ -1286,21 +1286,21 @@ amd_branches_is_visible(struct kobject *kobj, struct attribute *attr, int i)
> >  }
> >
> >  static struct attribute_group group_caps_amd_branches = {
> >         .name  = "caps",
> >         .attrs = amd_pmu_branches_attrs,
> >         .is_visible = amd_branches_is_visible,
> >  };
> >
> >  #ifdef CONFIG_PERF_EVENTS_AMD_BRS
> >
> > -EVENT_ATTR_STR(branch-brs, amd_branch_brs,
> > +EVENT_ATTR_STR(branch_brs, amd_branch_brs,
> >                "event=" __stringify(AMD_FAM19H_BRS_EVENT)"\n");
> >
> >  static struct attribute *amd_brs_events_attrs[] = {
> >         EVENT_PTR(amd_branch_brs),
> >         NULL,
> >  };
> >
> >  static umode_t
> >  amd_brs_is_visible(struct kobject *kobj, struct attribute *attr, int i)
> >  {
> > --
> > 2.18.4
> >
> 
>