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

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

[zhengjun.xing]

From: Zhengjun Xing <zhengjun.xing@linux.intel.com>

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 <zhengjun.xing@linux.intel.com>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
---
 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

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

[zhengjun.xing]

From: Zhengjun Xing <zhengjun.xing@linux.intel.com>

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 <zhengjun.xing@linux.intel.com>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
---
 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

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

[Liang, Kan]

On 7/1/2022 10:35 PM, zhengjun.xing@linux.intel.com wrote:
> From: Zhengjun Xing <zhengjun.xing@linux.intel.com>
> 
> 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 <zhengjun.xing@linux.intel.com>
> Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
> ---
>   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. */

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

[Xing Zhengjun]

On 7/4/2022 9:22 PM, Liang, Kan wrote:
> 
> 
> On 7/1/2022 10:35 PM, zhengjun.xing@linux.intel.com wrote:
>> From: Zhengjun Xing <zhengjun.xing@linux.intel.com>
>>
>> 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 <zhengjun.xing@linux.intel.com>
>> Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
>>   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