2022-07-02 02:35:41

by Xing Zhengjun

[permalink] [raw]
Subject: [PATCH 1/2] perf record: Fix "--per-thread" option for hybrid machines

From: Zhengjun Xing <[email protected]>

Commit b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to
collect metadata records") adds a dummy event on hybrid systems to fix the
symbol "unknown" issue when the workload is created in a P-core but runs
on an E-core. When "--per-thread" is enabled, the nr_cpus is reduced to 1,
adding a dummy event is useless for this issue, and it will also cause
"failed to mmap with 22 (Invalid argument)". This patch stops adding dummy
events when the option "--per-thread" is enabled, then the option can work
on hybrid machines.

Before:

# ./perf record -e cycles:u --per-thread sleep 1
failed to mmap with 22 (Invalid argument)

After:

# ./perf record -e cycles:u --per-thread sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.002 MB perf.data (6 samples) ]

Fixes: b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to collect metadata records")
Signed-off-by: Zhengjun Xing <[email protected]>
Reviewed-by: Kan Liang <[email protected]>
---
tools/perf/builtin-record.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e1edd4e98358..44ea2dd424fe 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1223,7 +1223,7 @@ static int record__open(struct record *rec)
* of waiting or event synthesis.
*/
if (opts->initial_delay || target__has_cpu(&opts->target) ||
- perf_pmu__has_hybrid()) {
+ (perf_pmu__has_hybrid() && !opts->target.per_thread)) {
pos = evlist__get_tracking_event(evlist);
if (!evsel__is_dummy_event(pos)) {
/* Set up dummy event. */
--
2.25.1


2022-07-02 02:43:27

by Xing Zhengjun

[permalink] [raw]
Subject: [PATCH 2/2] perf test: Fix test case 87 ("perf record tests") for hybrid systems

From: Zhengjun Xing <[email protected]>

The test case 87 ("perf record tests") failed on hybrid systems,the event
"cpu/br_inst_retired.near_call/p" is only for non-hybrid system. Correct
the test event to support both non-hybrid and hybrid systems.

Before:
# ./perf test 87
87: perf record tests : FAILED!

After:
# ./perf test 87
87: perf record tests : Ok

Fixes: 24f378e66021 ("perf test: Add basic perf record tests")
Signed-off-by: Zhengjun Xing <[email protected]>
Reviewed-by: Kan Liang <[email protected]>
---
tools/perf/tests/shell/record.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 00c7285ce1ac..301f95427159 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -61,7 +61,7 @@ test_register_capture() {
echo "Register capture test [Skipped missing registers]"
return
fi
- if ! perf record -o - --intr-regs=di,r8,dx,cx -e cpu/br_inst_retired.near_call/p \
+ if ! perf record -o - --intr-regs=di,r8,dx,cx -e br_inst_retired.near_call:p \
-c 1000 --per-thread true 2> /dev/null \
| perf script -F ip,sym,iregs -i - 2> /dev/null \
| egrep -q "DI:"
--
2.25.1

2022-07-04 13:26:52

by Liang, Kan

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf record: Fix "--per-thread" option for hybrid machines



On 7/1/2022 10:35 PM, [email protected] wrote:
> From: Zhengjun Xing <[email protected]>
>
> Commit b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to
> collect metadata records") adds a dummy event on hybrid systems to fix the
> symbol "unknown" issue when the workload is created in a P-core but runs
> on an E-core. When "--per-thread" is enabled, the nr_cpus is reduced to 1,
> adding a dummy event is useless for this issue, and it will also cause

A dummy event is required since a mmap event may be loaded at runtime on
any CPU. Thanks Adrian to point it out.

Thanks,
Kan

> "failed to mmap with 22 (Invalid argument)". This patch stops adding dummy
> events when the option "--per-thread" is enabled, then the option can work
> on hybrid machines.
>
> Before:
>
> # ./perf record -e cycles:u --per-thread sleep 1
> failed to mmap with 22 (Invalid argument)
>
> After:
>
> # ./perf record -e cycles:u --per-thread sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.002 MB perf.data (6 samples) ]
>
> Fixes: b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to collect metadata records")
> Signed-off-by: Zhengjun Xing <[email protected]>
> Reviewed-by: Kan Liang <[email protected]>
> ---
> tools/perf/builtin-record.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index e1edd4e98358..44ea2dd424fe 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1223,7 +1223,7 @@ static int record__open(struct record *rec)
> * of waiting or event synthesis.
> */
> if (opts->initial_delay || target__has_cpu(&opts->target) ||
> - perf_pmu__has_hybrid()) {
> + (perf_pmu__has_hybrid() && !opts->target.per_thread)) {
> pos = evlist__get_tracking_event(evlist);
> if (!evsel__is_dummy_event(pos)) {
> /* Set up dummy event. */

2022-07-06 12:08:28

by Xing Zhengjun

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf record: Fix "--per-thread" option for hybrid machines



On 7/4/2022 9:22 PM, Liang, Kan wrote:
>
>
> On 7/1/2022 10:35 PM, [email protected] wrote:
>> From: Zhengjun Xing <[email protected]>
>>
>> Commit b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems to
>> collect metadata records") adds a dummy event on hybrid systems to fix
>> the
>> symbol "unknown" issue when the workload is created in a P-core but runs
>> on an E-core. When "--per-thread" is enabled, the nr_cpus is reduced
>> to 1,
>>   adding a dummy event is useless for this issue, and it will also cause
>
> A dummy event is required since a mmap event may be loaded at runtime on
> any CPU. Thanks Adrian to point it out.
>

There will be a kernel patch to fix the "mmap fail" issue, I will drop
this one and continue to fix other "per-thread" issues based on the
kernel patch.


> Thanks,
> Kan
>
>> "failed to mmap with 22 (Invalid argument)". This patch stops adding
>> dummy
>> events when the option "--per-thread" is enabled, then the option can
>> work
>> on hybrid machines.
>>
>> Before:
>>
>>   # ./perf record -e cycles:u --per-thread  sleep 1
>>   failed to mmap with 22 (Invalid argument)
>>
>> After:
>>
>>   # ./perf record -e cycles:u --per-thread  sleep 1
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.002 MB perf.data (6 samples) ]
>>
>> Fixes: b91e5492f9d7 ("perf record: Add a dummy event on hybrid systems
>> to collect metadata records")
>> Signed-off-by: Zhengjun Xing <[email protected]>
>> Reviewed-by: Kan Liang <[email protected]>
>> ---
>>   tools/perf/builtin-record.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index e1edd4e98358..44ea2dd424fe 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -1223,7 +1223,7 @@ static int record__open(struct record *rec)
>>        * of waiting or event synthesis.
>>        */
>>       if (opts->initial_delay || target__has_cpu(&opts->target) ||
>> -        perf_pmu__has_hybrid()) {
>> +        (perf_pmu__has_hybrid() && !opts->target.per_thread)) {
>>           pos = evlist__get_tracking_event(evlist);
>>           if (!evsel__is_dummy_event(pos)) {
>>               /* Set up dummy event. */

--
Zhengjun Xing