[PATCH V2 0/2] perf parse-events: Do not break up AUX event group

[PATCH V2 0/2] perf parse-events: Do not break up AUX event group

[Adrian Hunter]

Hi

Here is a fix needed in v6.4, but dependent on Ian's patch
"perf evsel: Modify group pmu name for software events", refer:

https://lore.kernel.org/lkml/20230502223851.2234828-20-irogers@google.com/

There is also a test amendment to test the AUX event group fix for
Intel PT.


Changes in V2:

   Add an Intel PT test amendment for the AUX event group fix
   Change AUX event group fix as suggested by Ian


Adrian Hunter (2):
      perf test: test_intel_pt.sh: Test sample mode with named PMU
      perf parse-events: Do not break up AUX event group

 tools/perf/tests/shell/test_intel_pt.sh | 7 +++++++
 tools/perf/util/evsel.c                 | 9 +++------
 2 files changed, 10 insertions(+), 6 deletions(-)


Regards
Adrian

[PATCH V2 1/2] perf test: test_intel_pt.sh: Test sample mode with event with PMU name

[Adrian Hunter]

br_misp_retired.all_branches is supported on processors that support
Intel PT, so use it to test sample mode with an event that has been given
a PMU name.

Please note, the test fails prior to the fix "perf parse-events: Do not
break up AUX event group".

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 4ddb17cb83c5..3a8b9bffa022 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -506,6 +506,13 @@ test_sample()
 		echo "perf record failed with --aux-sample"
 		return 1
 	fi
+	# Check with event with PMU name
+	if perf_record_no_decode -o "${perfdatafile}" -e br_misp_retired.all_branches:u uname ; then
+		if ! perf_record_no_decode -o "${perfdatafile}" -e '{intel_pt//,br_misp_retired.all_branches/aux-sample-size=8192/}:u' uname ; then
+			echo "perf record failed with --aux-sample-size"
+			return 1
+		fi
+	fi
 	echo OK
 	return 0
 }
-- 
2.34.1

[PATCH V2 2/2] perf parse-events: Do not break up AUX event group

[Adrian Hunter]

Do not assume which events may have a PMU name, allowing the logic to
keep an AUX event group together.

Example:

 Before:

    $ perf record --no-bpf-event -c 10 -e '{intel_pt//,tlb_flush.stlb_any/aux-sample-size=8192/pp}:u' -- sleep 0.1
    WARNING: events were regrouped to match PMUs
    Cannot add AUX area sampling to a group leader

 After:

    $ perf record --no-bpf-event -c 10 -e '{intel_pt//,tlb_flush.stlb_any/aux-sample-size=8192/pp}:u' -- sleep 0.1
    [ perf record: Woken up 1 times to write data ]
    [ perf record: Captured and wrote 0.078 MB perf.data ]
    $ perf script -F-dso,+addr | grep -C5 tlb_flush.stlb_any | head -11
    sleep 20444 [003]  7939.510243:  1  branches:uH:  7f5350cc82a2 dl_main+0x9a2 => 7f5350cb38f0 _dl_add_to_namespace_list+0x0
    sleep 20444 [003]  7939.510243:  1  branches:uH:  7f5350cb3908 _dl_add_to_namespace_list+0x18 => 7f5350cbb080 rtld_mutex_dummy+0x0
    sleep 20444 [003]  7939.510243:  1  branches:uH:  7f5350cc8350 dl_main+0xa50 => 0 [unknown]
    sleep 20444 [003]  7939.510244:  1  branches:uH:  7f5350cc83ca dl_main+0xaca => 7f5350caeb60 _dl_process_pt_gnu_property+0x0
    sleep 20444 [003]  7939.510245:  1  branches:uH:  7f5350caeb60 _dl_process_pt_gnu_property+0x0 => 0 [unknown]
    sleep 20444  7939.510245:       10 tlb_flush.stlb_any/aux-sample-size=8192/pp: 0 7f5350caeb60 _dl_process_pt_gnu_property+0x0
    sleep 20444 [003]  7939.510254:  1  branches:uH:  7f5350cc87fe dl_main+0xefe => 7f5350ccd240 strcmp+0x0
    sleep 20444 [003]  7939.510254:  1  branches:uH:  7f5350cc8862 dl_main+0xf62 => 0 [unknown]
    sleep 20444 [003]  7939.510255:  1  branches:uH:  7f5350cc9cdc dl_main+0x23dc => 0 [unknown]
    sleep 20444 [003]  7939.510257:  1  branches:uH:  7f5350cc89f6 dl_main+0x10f6 => 7f5350cb9530 _dl_setup_hash+0x0
    sleep 20444 [003]  7939.510257:  1  branches:uH:  7f5350cc8a2d dl_main+0x112d => 7f5350cb3990 _dl_new_object+0x0

Suggested-by: Ian Rogers <irogers@google.com>
Fixes: 347c2f0a0988 ("perf parse-events: Sort and group parsed events")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evsel.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index e18752fd9245..9fdcde5471df 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -828,11 +828,9 @@ bool evsel__name_is(struct evsel *evsel, const char *name)
 
 const char *evsel__group_pmu_name(const struct evsel *evsel)
 {
-	struct evsel *leader, *pos;
+	struct evsel *leader = evsel__leader(evsel);
+	struct evsel *pos;
 
-	/* If the pmu_name is set use it. pmu_name isn't set for CPU and software events. */
-	if (evsel->pmu_name)
-		return evsel->pmu_name;
 	/*
 	 * Software events may be in a group with other uncore PMU events. Use
 	 * the pmu_name of the first non-software event to avoid breaking the
@@ -841,7 +839,6 @@ const char *evsel__group_pmu_name(const struct evsel *evsel)
 	 * Aux event leaders, like intel_pt, expect a group with events from
 	 * other PMUs, so substitute the AUX event's PMU in this case.
 	 */
-	leader  = evsel__leader(evsel);
 	if (evsel->core.attr.type == PERF_TYPE_SOFTWARE || evsel__is_aux_event(leader)) {
 		/* Starting with the leader, find the first event with a named PMU. */
 		for_each_group_evsel(pos, leader) {
@@ -850,7 +847,7 @@ const char *evsel__group_pmu_name(const struct evsel *evsel)
 		}
 	}
 
-	return "cpu";
+	return evsel->pmu_name ?: "cpu";
 }
 
 const char *evsel__metric_id(const struct evsel *evsel)
-- 
2.34.1

Re: [PATCH V2 2/2] perf parse-events: Do not break up AUX event group

[Ian Rogers]

On Mon, May 8, 2023 at 2:40 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> Do not assume which events may have a PMU name, allowing the logic to
> keep an AUX event group together.
>
> Example:
>
>  Before:
>
>     $ perf record --no-bpf-event -c 10 -e '{intel_pt//,tlb_flush.stlb_any/aux-sample-size=8192/pp}:u' -- sleep 0.1
>     WARNING: events were regrouped to match PMUs
>     Cannot add AUX area sampling to a group leader
>
>  After:
>
>     $ perf record --no-bpf-event -c 10 -e '{intel_pt//,tlb_flush.stlb_any/aux-sample-size=8192/pp}:u' -- sleep 0.1
>     [ perf record: Woken up 1 times to write data ]
>     [ perf record: Captured and wrote 0.078 MB perf.data ]
>     $ perf script -F-dso,+addr | grep -C5 tlb_flush.stlb_any | head -11
>     sleep 20444 [003]  7939.510243:  1  branches:uH:  7f5350cc82a2 dl_main+0x9a2 => 7f5350cb38f0 _dl_add_to_namespace_list+0x0
>     sleep 20444 [003]  7939.510243:  1  branches:uH:  7f5350cb3908 _dl_add_to_namespace_list+0x18 => 7f5350cbb080 rtld_mutex_dummy+0x0
>     sleep 20444 [003]  7939.510243:  1  branches:uH:  7f5350cc8350 dl_main+0xa50 => 0 [unknown]
>     sleep 20444 [003]  7939.510244:  1  branches:uH:  7f5350cc83ca dl_main+0xaca => 7f5350caeb60 _dl_process_pt_gnu_property+0x0
>     sleep 20444 [003]  7939.510245:  1  branches:uH:  7f5350caeb60 _dl_process_pt_gnu_property+0x0 => 0 [unknown]
>     sleep 20444  7939.510245:       10 tlb_flush.stlb_any/aux-sample-size=8192/pp: 0 7f5350caeb60 _dl_process_pt_gnu_property+0x0
>     sleep 20444 [003]  7939.510254:  1  branches:uH:  7f5350cc87fe dl_main+0xefe => 7f5350ccd240 strcmp+0x0
>     sleep 20444 [003]  7939.510254:  1  branches:uH:  7f5350cc8862 dl_main+0xf62 => 0 [unknown]
>     sleep 20444 [003]  7939.510255:  1  branches:uH:  7f5350cc9cdc dl_main+0x23dc => 0 [unknown]
>     sleep 20444 [003]  7939.510257:  1  branches:uH:  7f5350cc89f6 dl_main+0x10f6 => 7f5350cb9530 _dl_setup_hash+0x0
>     sleep 20444 [003]  7939.510257:  1  branches:uH:  7f5350cc8a2d dl_main+0x112d => 7f5350cb3990 _dl_new_object+0x0
>
> Suggested-by: Ian Rogers <irogers@google.com>
> Fixes: 347c2f0a0988 ("perf parse-events: Sort and group parsed events")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Tested-by: Ian Rogers <irogers@google.com>

Thanks!
Ian

> ---
>  tools/perf/util/evsel.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index e18752fd9245..9fdcde5471df 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -828,11 +828,9 @@ bool evsel__name_is(struct evsel *evsel, const char *name)
>
>  const char *evsel__group_pmu_name(const struct evsel *evsel)
>  {
> -       struct evsel *leader, *pos;
> +       struct evsel *leader = evsel__leader(evsel);
> +       struct evsel *pos;
>
> -       /* If the pmu_name is set use it. pmu_name isn't set for CPU and software events. */
> -       if (evsel->pmu_name)
> -               return evsel->pmu_name;
>         /*
>          * Software events may be in a group with other uncore PMU events. Use
>          * the pmu_name of the first non-software event to avoid breaking the
> @@ -841,7 +839,6 @@ const char *evsel__group_pmu_name(const struct evsel *evsel)
>          * Aux event leaders, like intel_pt, expect a group with events from
>          * other PMUs, so substitute the AUX event's PMU in this case.
>          */
> -       leader  = evsel__leader(evsel);
>         if (evsel->core.attr.type == PERF_TYPE_SOFTWARE || evsel__is_aux_event(leader)) {
>                 /* Starting with the leader, find the first event with a named PMU. */
>                 for_each_group_evsel(pos, leader) {
> @@ -850,7 +847,7 @@ const char *evsel__group_pmu_name(const struct evsel *evsel)
>                 }
>         }
>
> -       return "cpu";
> +       return evsel->pmu_name ?: "cpu";
>  }
>
>  const char *evsel__metric_id(const struct evsel *evsel)
> --
> 2.34.1
>

Re: [PATCH V2 0/2] perf parse-events: Do not break up AUX event group

[Arnaldo Carvalho de Melo]

Em Mon, May 08, 2023 at 12:39:50PM +0300, Adrian Hunter escreveu:
> Hi
> 
> Here is a fix needed in v6.4, but dependent on Ian's patch
> "perf evsel: Modify group pmu name for software events", refer:
> 
> https://lore.kernel.org/lkml/20230502223851.2234828-20-irogers@google.com/
> 
> There is also a test amendment to test the AUX event group fix for
> Intel PT.

Ok, so I think I can add your Tested-by for that "perf evsel: Modify
group pmu name for software events" patch, ok?

- Arnaldo
 
> 
> Changes in V2:
> 
>    Add an Intel PT test amendment for the AUX event group fix
>    Change AUX event group fix as suggested by Ian
> 
> 
> Adrian Hunter (2):
>       perf test: test_intel_pt.sh: Test sample mode with named PMU
>       perf parse-events: Do not break up AUX event group
> 
>  tools/perf/tests/shell/test_intel_pt.sh | 7 +++++++
>  tools/perf/util/evsel.c                 | 9 +++------
>  2 files changed, 10 insertions(+), 6 deletions(-)

Re: [PATCH V2 0/2] perf parse-events: Do not break up AUX event group

[Arnaldo Carvalho de Melo]

Em Mon, May 08, 2023 at 12:39:50PM +0300, Adrian Hunter escreveu:
> Hi
> 
> Here is a fix needed in v6.4, but dependent on Ian's patch
> "perf evsel: Modify group pmu name for software events", refer:

Thanks, applied to perf-core, for v6.4.

- Arnaldo
 
> https://lore.kernel.org/lkml/20230502223851.2234828-20-irogers@google.com/
> 
> There is also a test amendment to test the AUX event group fix for
> Intel PT.
> 
> 
> Changes in V2:
> 
>    Add an Intel PT test amendment for the AUX event group fix
>    Change AUX event group fix as suggested by Ian
> 
> 
> Adrian Hunter (2):
>       perf test: test_intel_pt.sh: Test sample mode with named PMU
>       perf parse-events: Do not break up AUX event group
> 
>  tools/perf/tests/shell/test_intel_pt.sh | 7 +++++++
>  tools/perf/util/evsel.c                 | 9 +++------
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> 
> Regards
> Adrian

-- 

- Arnaldo

Re: [PATCH V2 0/2] perf parse-events: Do not break up AUX event group

[Adrian Hunter]

On 10/05/23 20:13, Arnaldo Carvalho de Melo wrote:
> Em Mon, May 08, 2023 at 12:39:50PM +0300, Adrian Hunter escreveu:
>> Hi
>>
>> Here is a fix needed in v6.4, but dependent on Ian's patch
>> "perf evsel: Modify group pmu name for software events", refer:
>>
>> https://lore.kernel.org/lkml/20230502223851.2234828-20-irogers@google.com/
>>
>> There is also a test amendment to test the AUX event group fix for
>> Intel PT.
> 
> Ok, so I think I can add your Tested-by for that "perf evsel: Modify
> group pmu name for software events" patch, ok?

Yes, thank you!

> 
> - Arnaldo
>  
>>
>> Changes in V2:
>>
>>    Add an Intel PT test amendment for the AUX event group fix
>>    Change AUX event group fix as suggested by Ian
>>
>>
>> Adrian Hunter (2):
>>       perf test: test_intel_pt.sh: Test sample mode with named PMU
>>       perf parse-events: Do not break up AUX event group
>>
>>  tools/perf/tests/shell/test_intel_pt.sh | 7 +++++++
>>  tools/perf/util/evsel.c                 | 9 +++------
>>  2 files changed, 10 insertions(+), 6 deletions(-)