[PATCH V3 1/3] perf report: Support Retire Latency

[PATCH V3 1/3] perf report: Support Retire Latency

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The Retire Latency field is added in the var3_w of the
PERF_SAMPLE_WEIGHT_STRUCT. The Retire Latency reports pipeline stall
of this instruction compared to the previous instruction in cycles.
That's quite useful to display the information with perf mem report.

The p_stage_cyc for Power is also from the var3_w. Union the p_stage_cyc
and retire_lat to share the code.

Implement X86 specific codes to display the X86 specific header.

Add a new sort key retire_lat for the Retire Latency.

Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

The kernel patches have been merged. The V3 only includes the perf tool
patches. The V2 can be found at
https://lore.kernel.org/lkml/20230104201349.1451191-1-kan.liang@linux.intel.com/

No change from V2.

 tools/perf/Documentation/perf-report.txt |  2 ++
 tools/perf/arch/x86/util/event.c         | 21 +++++++++++++++++++++
 tools/perf/util/sample.h                 |  5 ++++-
 tools/perf/util/sort.c                   |  2 ++
 tools/perf/util/sort.h                   |  2 ++
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 9b0c0dbf9a77..c242e8da6b1a 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -115,6 +115,8 @@ OPTIONS
 	- p_stage_cyc: On powerpc, this presents the number of cycles spent in a
 	  pipeline stage. And currently supported only on powerpc.
 	- addr: (Full) virtual address of the sampled instruction
+	- retire_lat: On X86, this reports pipeline stall of this instruction compared
+	  to the previous instruction in cycles. And currently supported only on X86
 
 	By default, comm, dso and symbol keys are used.
 	(i.e. --sort comm,dso,symbol)
diff --git a/tools/perf/arch/x86/util/event.c b/tools/perf/arch/x86/util/event.c
index a3acefe6d0c6..e4288d09f3a0 100644
--- a/tools/perf/arch/x86/util/event.c
+++ b/tools/perf/arch/x86/util/event.c
@@ -89,6 +89,7 @@ void arch_perf_parse_sample_weight(struct perf_sample *data,
 	else {
 		data->weight = weight.var1_dw;
 		data->ins_lat = weight.var2_w;
+		data->retire_lat = weight.var3_w;
 	}
 }
 
@@ -100,5 +101,25 @@ void arch_perf_synthesize_sample_weight(const struct perf_sample *data,
 	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
 		*array &= 0xffffffff;
 		*array |= ((u64)data->ins_lat << 32);
+		*array |= ((u64)data->retire_lat << 48);
 	}
 }
+
+const char *arch_perf_header_entry(const char *se_header)
+{
+	if (!strcmp(se_header, "Local Pipeline Stage Cycle"))
+		return "Local Retire Latency";
+	else if (!strcmp(se_header, "Pipeline Stage Cycle"))
+		return "Retire Latency";
+
+	return se_header;
+}
+
+int arch_support_sort_key(const char *sort_key)
+{
+	if (!strcmp(sort_key, "p_stage_cyc"))
+		return 1;
+	if (!strcmp(sort_key, "local_p_stage_cyc"))
+		return 1;
+	return 0;
+}
diff --git a/tools/perf/util/sample.h b/tools/perf/util/sample.h
index 60ec79d4eea4..33b08e0ac746 100644
--- a/tools/perf/util/sample.h
+++ b/tools/perf/util/sample.h
@@ -92,7 +92,10 @@ struct perf_sample {
 	u8  cpumode;
 	u16 misc;
 	u16 ins_lat;
-	u16 p_stage_cyc;
+	union {
+		u16 p_stage_cyc;
+		u16 retire_lat;
+	};
 	bool no_hw_idx;		/* No hw_idx collected in branch_stack */
 	char insn[MAX_INSN];
 	void *raw_data;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index d7d0f997873a..4a648231fe72 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2133,6 +2133,8 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
 	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
 	DIM(SORT_ADDR, "addr", sort_addr),
+	DIM(SORT_LOCAL_RETIRE_LAT, "local_retire_lat", sort_local_p_stage_cyc),
+	DIM(SORT_GLOBAL_RETIRE_LAT, "retire_lat", sort_global_p_stage_cyc),
 };
 
 #undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 921715e6aec4..9a91d0df2833 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -237,6 +237,8 @@ enum sort_type {
 	SORT_LOCAL_PIPELINE_STAGE_CYC,
 	SORT_GLOBAL_PIPELINE_STAGE_CYC,
 	SORT_ADDR,
+	SORT_LOCAL_RETIRE_LAT,
+	SORT_GLOBAL_RETIRE_LAT,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,
-- 
2.35.1

[PATCH V3 2/3] perf script: Support Retire Latency

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The Retire Latency field is added in the var3_w of the
PERF_SAMPLE_WEIGHT_STRUCT. The Retire Latency reports the number of
elapsed core clocks between the retirement of the instruction
indicated by the Instruction Pointer field of the PEBS record and the
retirement of the prior instruction. That's quite useful to display
the information with perf script.

Add a new field retire_lat for the Retire Latency information.

Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

Change from V2
- Rebase on top of tmp.perf/core
- Update perf-script.txt

 tools/perf/Documentation/perf-script.txt |  2 +-
 tools/perf/builtin-script.c              | 13 +++++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index a2ebadc9d948..777a0d8ba7d1 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -134,7 +134,7 @@ OPTIONS
         srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
         brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
         phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
-        machine_pid, vcpu, cgroup.
+        machine_pid, vcpu, cgroup, retire_lat.
         Field list can be prepended with the type, trace, sw or hw,
         to indicate to which event type the field list applies.
         e.g., -F sw:comm,tid,time,ip,sym  and -F trace:time,cpu,trace
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index cb6b34da4eef..3fe9b1c4caaf 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -132,6 +132,7 @@ enum perf_output_field {
 	PERF_OUTPUT_MACHINE_PID     = 1ULL << 37,
 	PERF_OUTPUT_VCPU            = 1ULL << 38,
 	PERF_OUTPUT_CGROUP          = 1ULL << 39,
+	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
 };
 
 struct perf_script {
@@ -203,6 +204,7 @@ struct output_option {
 	{.str = "machine_pid", .field = PERF_OUTPUT_MACHINE_PID},
 	{.str = "vcpu", .field = PERF_OUTPUT_VCPU},
 	{.str = "cgroup", .field = PERF_OUTPUT_CGROUP},
+	{.str = "retire_lat", .field = PERF_OUTPUT_RETIRE_LAT},
 };
 
 enum {
@@ -278,7 +280,7 @@ static struct {
 			      PERF_OUTPUT_ADDR | PERF_OUTPUT_DATA_SRC |
 			      PERF_OUTPUT_WEIGHT | PERF_OUTPUT_PHYS_ADDR |
 			      PERF_OUTPUT_DATA_PAGE_SIZE | PERF_OUTPUT_CODE_PAGE_SIZE |
-			      PERF_OUTPUT_INS_LAT,
+			      PERF_OUTPUT_INS_LAT | PERF_OUTPUT_RETIRE_LAT,
 
 		.invalid_fields = PERF_OUTPUT_TRACE | PERF_OUTPUT_BPF_OUTPUT,
 	},
@@ -551,6 +553,10 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
 		return -EINVAL;
 	}
 
+	if (PRINT_FIELD(RETIRE_LAT) &&
+	    evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_STRUCT, "WEIGHT_STRUCT", PERF_OUTPUT_RETIRE_LAT))
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -2187,6 +2193,9 @@ static void process_event(struct perf_script *script,
 	if (PRINT_FIELD(INS_LAT))
 		fprintf(fp, "%16" PRIu16, sample->ins_lat);
 
+	if (PRINT_FIELD(RETIRE_LAT))
+		fprintf(fp, "%16" PRIu16, sample->retire_lat);
+
 	if (PRINT_FIELD(IP)) {
 		struct callchain_cursor *cursor = NULL;
 
@@ -3876,7 +3885,7 @@ int cmd_script(int argc, const char **argv)
 		     "brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
 		     "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
 		     "phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
-		     "code_page_size,ins_lat,machine_pid,vcpu,cgroup",
+		     "code_page_size,ins_lat,machine_pid,vcpu,cgroup,retire_lat",
 		     parse_output_fields),
 	OPT_BOOLEAN('a', "all-cpus", &system_wide,
 		    "system-wide collection from all CPUs"),
-- 
2.35.1

[PATCH V3 3/3] perf test: Support the retire_lat check

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

Add test for the new field for Retire Latency in the X86 specific test.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

New patch since V2

 tools/perf/arch/x86/tests/sample-parsing.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/x86/tests/sample-parsing.c b/tools/perf/arch/x86/tests/sample-parsing.c
index 690c7c07e90d..a061e8619267 100644
--- a/tools/perf/arch/x86/tests/sample-parsing.c
+++ b/tools/perf/arch/x86/tests/sample-parsing.c
@@ -27,8 +27,10 @@ static bool samples_same(const struct perf_sample *s1,
 			 const struct perf_sample *s2,
 			 u64 type)
 {
-	if (type & PERF_SAMPLE_WEIGHT_STRUCT)
+	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
 		COMP(ins_lat);
+		COMP(retire_lat);
+	}
 
 	return true;
 }
@@ -48,6 +50,7 @@ static int do_test(u64 sample_type)
 	struct perf_sample sample = {
 		.weight		= 101,
 		.ins_lat        = 102,
+		.retire_lat     = 103,
 	};
 	struct perf_sample sample_out;
 	size_t i, sz, bufsz;
-- 
2.35.1

Re: [PATCH V3 3/3] perf test: Support the retire_lat check

[Arnaldo Carvalho de Melo]

Em Thu, Feb 02, 2023 at 11:22:09AM -0800, kan.liang@linux.intel.com escreveu:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> Add test for the new field for Retire Latency in the X86 specific test.

Is this passing 'perf test' for you?

[root@quaco ~]# perf test -v "x86 sample parsing"
 74: x86 Sample parsing                                              :
--- start ---
test child forked, pid 72526
Samples differ at 'retire_lat'
parsing failed for sample_type 0x1000000
test child finished with -1
---- end ----
x86 Sample parsing: FAILED!
[root@quaco ~]#

- Arnaldo
 
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> ---
> 
> New patch since V2
> 
>  tools/perf/arch/x86/tests/sample-parsing.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/arch/x86/tests/sample-parsing.c b/tools/perf/arch/x86/tests/sample-parsing.c
> index 690c7c07e90d..a061e8619267 100644
> --- a/tools/perf/arch/x86/tests/sample-parsing.c
> +++ b/tools/perf/arch/x86/tests/sample-parsing.c
> @@ -27,8 +27,10 @@ static bool samples_same(const struct perf_sample *s1,
>  			 const struct perf_sample *s2,
>  			 u64 type)
>  {
> -	if (type & PERF_SAMPLE_WEIGHT_STRUCT)
> +	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
>  		COMP(ins_lat);
> +		COMP(retire_lat);
> +	}
>  
>  	return true;
>  }
> @@ -48,6 +50,7 @@ static int do_test(u64 sample_type)
>  	struct perf_sample sample = {
>  		.weight		= 101,
>  		.ins_lat        = 102,
> +		.retire_lat     = 103,
>  	};
>  	struct perf_sample sample_out;
>  	size_t i, sz, bufsz;
> -- 
> 2.35.1
> 

-- 

- Arnaldo

Re: [PATCH V3 3/3] perf test: Support the retire_lat check

[Liang, Kan]

On 2023-02-06 10:01 a.m., Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 02, 2023 at 11:22:09AM -0800, kan.liang@linux.intel.com escreveu:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> Add test for the new field for Retire Latency in the X86 specific test.
> 
> Is this passing 'perf test' for you?

Ah, it should be the original V2 missed the below change.

@@ -100,5 +101,25 @@ void arch_perf_synthesize_sample_weight(const
struct perf_sample *data,
 	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
 		*array &= 0xffffffff;
 		*array |= ((u64)data->ins_lat << 32);
+		*array |= ((u64)data->retire_lat << 48);
 	}
 }

Could you please remove the V2 and re-apply the V3?

$ sudo ./perf test -v "x86 sample parsing"
 74: x86 Sample parsing                                              :
--- start ---
test child forked, pid 3316797
test child finished with 0
---- end ----
x86 Sample parsing: Ok


Thanks,
Kan

> 
> [root@quaco ~]# perf test -v "x86 sample parsing"
>  74: x86 Sample parsing                                              :
> --- start ---
> test child forked, pid 72526
> Samples differ at 'retire_lat'
> parsing failed for sample_type 0x1000000
> test child finished with -1
> ---- end ----
> x86 Sample parsing: FAILED!
> [root@quaco ~]#
> 
> - Arnaldo
>  
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
>>
>> New patch since V2
>>
>>  tools/perf/arch/x86/tests/sample-parsing.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/arch/x86/tests/sample-parsing.c b/tools/perf/arch/x86/tests/sample-parsing.c
>> index 690c7c07e90d..a061e8619267 100644
>> --- a/tools/perf/arch/x86/tests/sample-parsing.c
>> +++ b/tools/perf/arch/x86/tests/sample-parsing.c
>> @@ -27,8 +27,10 @@ static bool samples_same(const struct perf_sample *s1,
>>  			 const struct perf_sample *s2,
>>  			 u64 type)
>>  {
>> -	if (type & PERF_SAMPLE_WEIGHT_STRUCT)
>> +	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
>>  		COMP(ins_lat);
>> +		COMP(retire_lat);
>> +	}
>>  
>>  	return true;
>>  }
>> @@ -48,6 +50,7 @@ static int do_test(u64 sample_type)
>>  	struct perf_sample sample = {
>>  		.weight		= 101,
>>  		.ins_lat        = 102,
>> +		.retire_lat     = 103,
>>  	};
>>  	struct perf_sample sample_out;
>>  	size_t i, sz, bufsz;
>> -- 
>> 2.35.1
>>
> 

Re: [PATCH V3 3/3] perf test: Support the retire_lat check

[Arnaldo Carvalho de Melo]

Em Mon, Feb 06, 2023 at 12:01:44PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Feb 02, 2023 at 11:22:09AM -0800, kan.liang@linux.intel.com escreveu:
> > From: Kan Liang <kan.liang@linux.intel.com>
> > 
> > Add test for the new field for Retire Latency in the X86 specific test.
> 
> Is this passing 'perf test' for you?
> 
> [root@quaco ~]# perf test -v "x86 sample parsing"
>  74: x86 Sample parsing                                              :
> --- start ---
> test child forked, pid 72526
> Samples differ at 'retire_lat'
> parsing failed for sample_type 0x1000000
> test child finished with -1
> ---- end ----
> x86 Sample parsing: FAILED!
> [root@quaco ~]#

In tools/perf/arch/x86/util/event.c  you have:

void arch_perf_parse_sample_weight(struct perf_sample *data,
                                   const __u64 *array, u64 type)
{
        union perf_sample_weight weight;

        weight.full = *array;
        if (type & PERF_SAMPLE_WEIGHT)
                data->weight = weight.full;
        else {
                data->weight = weight.var1_dw;
                data->ins_lat = weight.var2_w;
                data->retire_lat = weight.var3_w;
        }
}

void arch_perf_synthesize_sample_weight(const struct perf_sample *data,
                                        __u64 *array, u64 type)
{
        *array = data->weight;

        if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
                *array &= 0xffffffff;
                *array |= ((u64)data->ins_lat << 32);
        }
}

didn't you forget to encode data->retire_lat at
arch_perf_synthesize_sample_weight()?

- Arnaldo

> > Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> > ---
> > 
> > New patch since V2
> > 
> >  tools/perf/arch/x86/tests/sample-parsing.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/arch/x86/tests/sample-parsing.c b/tools/perf/arch/x86/tests/sample-parsing.c
> > index 690c7c07e90d..a061e8619267 100644
> > --- a/tools/perf/arch/x86/tests/sample-parsing.c
> > +++ b/tools/perf/arch/x86/tests/sample-parsing.c
> > @@ -27,8 +27,10 @@ static bool samples_same(const struct perf_sample *s1,
> >  			 const struct perf_sample *s2,
> >  			 u64 type)
> >  {
> > -	if (type & PERF_SAMPLE_WEIGHT_STRUCT)
> > +	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
> >  		COMP(ins_lat);
> > +		COMP(retire_lat);
> > +	}
> >  
> >  	return true;
> >  }
> > @@ -48,6 +50,7 @@ static int do_test(u64 sample_type)
> >  	struct perf_sample sample = {
> >  		.weight		= 101,
> >  		.ins_lat        = 102,
> > +		.retire_lat     = 103,
> >  	};
> >  	struct perf_sample sample_out;
> >  	size_t i, sz, bufsz;
> > -- 
> > 2.35.1
> > 
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo

Re: [PATCH V3 3/3] perf test: Support the retire_lat check

[Arnaldo Carvalho de Melo]

Em Mon, Feb 06, 2023 at 10:17:46AM -0500, Liang, Kan escreveu:
> 
> 
> On 2023-02-06 10:01 a.m., Arnaldo Carvalho de Melo wrote:
> > Em Thu, Feb 02, 2023 at 11:22:09AM -0800, kan.liang@linux.intel.com escreveu:
> >> From: Kan Liang <kan.liang@linux.intel.com>
> >>
> >> Add test for the new field for Retire Latency in the X86 specific test.
> > 
> > Is this passing 'perf test' for you?
> 
> Ah, it should be the original V2 missed the below change.

Can you please send this as a separate patch as I already merged
torvalds/master and added more csets on top, so to just fix it and
force push now would be bad.

Please use what is in my perf/core branch and add a Fixes for that v2
patch.

Thanks,

- Arnaldo
 
> @@ -100,5 +101,25 @@ void arch_perf_synthesize_sample_weight(const
> struct perf_sample *data,
>  	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
>  		*array &= 0xffffffff;
>  		*array |= ((u64)data->ins_lat << 32);
> +		*array |= ((u64)data->retire_lat << 48);
>  	}
>  }
> 
> Could you please remove the V2 and re-apply the V3?
 
> $ sudo ./perf test -v "x86 sample parsing"
>  74: x86 Sample parsing                                              :
> --- start ---
> test child forked, pid 3316797
> test child finished with 0
> ---- end ----
> x86 Sample parsing: Ok
> 
> 
> Thanks,
> Kan
> 
> > 
> > [root@quaco ~]# perf test -v "x86 sample parsing"
> >  74: x86 Sample parsing                                              :
> > --- start ---
> > test child forked, pid 72526
> > Samples differ at 'retire_lat'
> > parsing failed for sample_type 0x1000000
> > test child finished with -1
> > ---- end ----
> > x86 Sample parsing: FAILED!
> > [root@quaco ~]#
> > 
> > - Arnaldo
> >  
> >> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> >> ---
> >>
> >> New patch since V2
> >>
> >>  tools/perf/arch/x86/tests/sample-parsing.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/perf/arch/x86/tests/sample-parsing.c b/tools/perf/arch/x86/tests/sample-parsing.c
> >> index 690c7c07e90d..a061e8619267 100644
> >> --- a/tools/perf/arch/x86/tests/sample-parsing.c
> >> +++ b/tools/perf/arch/x86/tests/sample-parsing.c
> >> @@ -27,8 +27,10 @@ static bool samples_same(const struct perf_sample *s1,
> >>  			 const struct perf_sample *s2,
> >>  			 u64 type)
> >>  {
> >> -	if (type & PERF_SAMPLE_WEIGHT_STRUCT)
> >> +	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
> >>  		COMP(ins_lat);
> >> +		COMP(retire_lat);
> >> +	}
> >>  
> >>  	return true;
> >>  }
> >> @@ -48,6 +50,7 @@ static int do_test(u64 sample_type)
> >>  	struct perf_sample sample = {
> >>  		.weight		= 101,
> >>  		.ins_lat        = 102,
> >> +		.retire_lat     = 103,
> >>  	};
> >>  	struct perf_sample sample_out;
> >>  	size_t i, sz, bufsz;
> >> -- 
> >> 2.35.1
> >>
> > 

-- 

- Arnaldo

Re: [PATCH V3 3/3] perf test: Support the retire_lat check

[Arnaldo Carvalho de Melo]

Em Mon, Feb 06, 2023 at 12:32:54PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Feb 06, 2023 at 10:17:46AM -0500, Liang, Kan escreveu:
> > 
> > 
> > On 2023-02-06 10:01 a.m., Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Feb 02, 2023 at 11:22:09AM -0800, kan.liang@linux.intel.com escreveu:
> > >> From: Kan Liang <kan.liang@linux.intel.com>
> > >>
> > >> Add test for the new field for Retire Latency in the X86 specific test.
> > > 
> > > Is this passing 'perf test' for you?
> > 
> > Ah, it should be the original V2 missed the below change.
> 
> Can you please send this as a separate patch as I already merged
> torvalds/master and added more csets on top, so to just fix it and
> force push now would be bad.
> 
> Please use what is in my perf/core branch and add a Fixes for that v2
> patch.

BTW, the 3rd patch with the test is already on the tmp.perf/core branch,
that will move to perf/core after the next round of container build
tests.

- Arnaldo
 
> Thanks,
> 
> - Arnaldo
>  
> > @@ -100,5 +101,25 @@ void arch_perf_synthesize_sample_weight(const
> > struct perf_sample *data,
> >  	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
> >  		*array &= 0xffffffff;
> >  		*array |= ((u64)data->ins_lat << 32);
> > +		*array |= ((u64)data->retire_lat << 48);
> >  	}
> >  }
> > 
> > Could you please remove the V2 and re-apply the V3?
>  
> > $ sudo ./perf test -v "x86 sample parsing"
> >  74: x86 Sample parsing                                              :
> > --- start ---
> > test child forked, pid 3316797
> > test child finished with 0
> > ---- end ----
> > x86 Sample parsing: Ok
> > 
> > 
> > Thanks,
> > Kan
> > 
> > > 
> > > [root@quaco ~]# perf test -v "x86 sample parsing"
> > >  74: x86 Sample parsing                                              :
> > > --- start ---
> > > test child forked, pid 72526
> > > Samples differ at 'retire_lat'
> > > parsing failed for sample_type 0x1000000
> > > test child finished with -1
> > > ---- end ----
> > > x86 Sample parsing: FAILED!
> > > [root@quaco ~]#
> > > 
> > > - Arnaldo
> > >  
> > >> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> > >> ---
> > >>
> > >> New patch since V2
> > >>
> > >>  tools/perf/arch/x86/tests/sample-parsing.c | 5 ++++-
> > >>  1 file changed, 4 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/tools/perf/arch/x86/tests/sample-parsing.c b/tools/perf/arch/x86/tests/sample-parsing.c
> > >> index 690c7c07e90d..a061e8619267 100644
> > >> --- a/tools/perf/arch/x86/tests/sample-parsing.c
> > >> +++ b/tools/perf/arch/x86/tests/sample-parsing.c
> > >> @@ -27,8 +27,10 @@ static bool samples_same(const struct perf_sample *s1,
> > >>  			 const struct perf_sample *s2,
> > >>  			 u64 type)
> > >>  {
> > >> -	if (type & PERF_SAMPLE_WEIGHT_STRUCT)
> > >> +	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
> > >>  		COMP(ins_lat);
> > >> +		COMP(retire_lat);
> > >> +	}
> > >>  
> > >>  	return true;
> > >>  }
> > >> @@ -48,6 +50,7 @@ static int do_test(u64 sample_type)
> > >>  	struct perf_sample sample = {
> > >>  		.weight		= 101,
> > >>  		.ins_lat        = 102,
> > >> +		.retire_lat     = 103,
> > >>  	};
> > >>  	struct perf_sample sample_out;
> > >>  	size_t i, sz, bufsz;
> > >> -- 
> > >> 2.35.1
> > >>
> > > 
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo

Re: [PATCH V3 3/3] perf test: Support the retire_lat check

[Liang, Kan]

On 2023-02-06 10:34 a.m., Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 06, 2023 at 12:32:54PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Mon, Feb 06, 2023 at 10:17:46AM -0500, Liang, Kan escreveu:
>>>
>>>
>>> On 2023-02-06 10:01 a.m., Arnaldo Carvalho de Melo wrote:
>>>> Em Thu, Feb 02, 2023 at 11:22:09AM -0800, kan.liang@linux.intel.com escreveu:
>>>>> From: Kan Liang <kan.liang@linux.intel.com>
>>>>>
>>>>> Add test for the new field for Retire Latency in the X86 specific test.
>>>>
>>>> Is this passing 'perf test' for you?
>>>
>>> Ah, it should be the original V2 missed the below change.
>>
>> Can you please send this as a separate patch as I already merged
>> torvalds/master and added more csets on top, so to just fix it and
>> force push now would be bad.
>>
>> Please use what is in my perf/core branch and add a Fixes for that v2
>> patch.
> 
> BTW, the 3rd patch with the test is already on the tmp.perf/core branch,
> that will move to perf/core after the next round of container build
> tests.
> 

Thanks. I will sent a V4 to fix the 'perf test' issue.

Thanks,
Kan
> - Arnaldo
>  
>> Thanks,
>>
>> - Arnaldo
>>  
>>> @@ -100,5 +101,25 @@ void arch_perf_synthesize_sample_weight(const
>>> struct perf_sample *data,
>>>  	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
>>>  		*array &= 0xffffffff;
>>>  		*array |= ((u64)data->ins_lat << 32);
>>> +		*array |= ((u64)data->retire_lat << 48);
>>>  	}
>>>  }
>>>
>>> Could you please remove the V2 and re-apply the V3?
>>  
>>> $ sudo ./perf test -v "x86 sample parsing"
>>>  74: x86 Sample parsing                                              :
>>> --- start ---
>>> test child forked, pid 3316797
>>> test child finished with 0
>>> ---- end ----
>>> x86 Sample parsing: Ok
>>>
>>>
>>> Thanks,
>>> Kan
>>>
>>>>
>>>> [root@quaco ~]# perf test -v "x86 sample parsing"
>>>>  74: x86 Sample parsing                                              :
>>>> --- start ---
>>>> test child forked, pid 72526
>>>> Samples differ at 'retire_lat'
>>>> parsing failed for sample_type 0x1000000
>>>> test child finished with -1
>>>> ---- end ----
>>>> x86 Sample parsing: FAILED!
>>>> [root@quaco ~]#
>>>>
>>>> - Arnaldo
>>>>  
>>>>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>>>>> ---
>>>>>
>>>>> New patch since V2
>>>>>
>>>>>  tools/perf/arch/x86/tests/sample-parsing.c | 5 ++++-
>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tools/perf/arch/x86/tests/sample-parsing.c b/tools/perf/arch/x86/tests/sample-parsing.c
>>>>> index 690c7c07e90d..a061e8619267 100644
>>>>> --- a/tools/perf/arch/x86/tests/sample-parsing.c
>>>>> +++ b/tools/perf/arch/x86/tests/sample-parsing.c
>>>>> @@ -27,8 +27,10 @@ static bool samples_same(const struct perf_sample *s1,
>>>>>  			 const struct perf_sample *s2,
>>>>>  			 u64 type)
>>>>>  {
>>>>> -	if (type & PERF_SAMPLE_WEIGHT_STRUCT)
>>>>> +	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
>>>>>  		COMP(ins_lat);
>>>>> +		COMP(retire_lat);
>>>>> +	}
>>>>>  
>>>>>  	return true;
>>>>>  }
>>>>> @@ -48,6 +50,7 @@ static int do_test(u64 sample_type)
>>>>>  	struct perf_sample sample = {
>>>>>  		.weight		= 101,
>>>>>  		.ins_lat        = 102,
>>>>> +		.retire_lat     = 103,
>>>>>  	};
>>>>>  	struct perf_sample sample_out;
>>>>>  	size_t i, sz, bufsz;
>>>>> -- 
>>>>> 2.35.1
>>>>>
>>>>
>>
>> -- 
>>
>> - Arnaldo
> 

Re: [PATCH V3 3/3] perf test: Support the retire_lat check

[Liang, Kan]

On 2023-02-06 10:49 a.m., Liang, Kan wrote:
> 
> 
> On 2023-02-06 10:34 a.m., Arnaldo Carvalho de Melo wrote:
>> Em Mon, Feb 06, 2023 at 12:32:54PM -0300, Arnaldo Carvalho de Melo escreveu:
>>> Em Mon, Feb 06, 2023 at 10:17:46AM -0500, Liang, Kan escreveu:
>>>>
>>>>
>>>> On 2023-02-06 10:01 a.m., Arnaldo Carvalho de Melo wrote:
>>>>> Em Thu, Feb 02, 2023 at 11:22:09AM -0800, kan.liang@linux.intel.com escreveu:
>>>>>> From: Kan Liang <kan.liang@linux.intel.com>
>>>>>>
>>>>>> Add test for the new field for Retire Latency in the X86 specific test.
>>>>>
>>>>> Is this passing 'perf test' for you?
>>>>
>>>> Ah, it should be the original V2 missed the below change.
>>>
>>> Can you please send this as a separate patch as I already merged
>>> torvalds/master and added more csets on top, so to just fix it and
>>> force push now would be bad.
>>>
>>> Please use what is in my perf/core branch and add a Fixes for that v2
>>> patch.
>>
>> BTW, the 3rd patch with the test is already on the tmp.perf/core branch,
>> that will move to perf/core after the next round of container build
>> tests.
>>
> 
> Thanks. I will sent a V4 to fix the 'perf test' issue.
> 

The V4 is here. It includes a fix for the 'perf test' issue and a fix
for the perf script document.
https://lore.kernel.org/lkml/20230206162100.3329395-1-kan.liang@linux.intel.com/

Sorry again for the inconvenience.

Thanks,
Kan

> Thanks,
> Kan
>> - Arnaldo
>>  
>>> Thanks,
>>>
>>> - Arnaldo
>>>  
>>>> @@ -100,5 +101,25 @@ void arch_perf_synthesize_sample_weight(const
>>>> struct perf_sample *data,
>>>>  	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
>>>>  		*array &= 0xffffffff;
>>>>  		*array |= ((u64)data->ins_lat << 32);
>>>> +		*array |= ((u64)data->retire_lat << 48);
>>>>  	}
>>>>  }
>>>>
>>>> Could you please remove the V2 and re-apply the V3?
>>>  
>>>> $ sudo ./perf test -v "x86 sample parsing"
>>>>  74: x86 Sample parsing                                              :
>>>> --- start ---
>>>> test child forked, pid 3316797
>>>> test child finished with 0
>>>> ---- end ----
>>>> x86 Sample parsing: Ok
>>>>
>>>>
>>>> Thanks,
>>>> Kan
>>>>
>>>>>
>>>>> [root@quaco ~]# perf test -v "x86 sample parsing"
>>>>>  74: x86 Sample parsing                                              :
>>>>> --- start ---
>>>>> test child forked, pid 72526
>>>>> Samples differ at 'retire_lat'
>>>>> parsing failed for sample_type 0x1000000
>>>>> test child finished with -1
>>>>> ---- end ----
>>>>> x86 Sample parsing: FAILED!
>>>>> [root@quaco ~]#
>>>>>
>>>>> - Arnaldo
>>>>>  
>>>>>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>>>>>> ---
>>>>>>
>>>>>> New patch since V2
>>>>>>
>>>>>>  tools/perf/arch/x86/tests/sample-parsing.c | 5 ++++-
>>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/tools/perf/arch/x86/tests/sample-parsing.c b/tools/perf/arch/x86/tests/sample-parsing.c
>>>>>> index 690c7c07e90d..a061e8619267 100644
>>>>>> --- a/tools/perf/arch/x86/tests/sample-parsing.c
>>>>>> +++ b/tools/perf/arch/x86/tests/sample-parsing.c
>>>>>> @@ -27,8 +27,10 @@ static bool samples_same(const struct perf_sample *s1,
>>>>>>  			 const struct perf_sample *s2,
>>>>>>  			 u64 type)
>>>>>>  {
>>>>>> -	if (type & PERF_SAMPLE_WEIGHT_STRUCT)
>>>>>> +	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
>>>>>>  		COMP(ins_lat);
>>>>>> +		COMP(retire_lat);
>>>>>> +	}
>>>>>>  
>>>>>>  	return true;
>>>>>>  }
>>>>>> @@ -48,6 +50,7 @@ static int do_test(u64 sample_type)
>>>>>>  	struct perf_sample sample = {
>>>>>>  		.weight		= 101,
>>>>>>  		.ins_lat        = 102,
>>>>>> +		.retire_lat     = 103,
>>>>>>  	};
>>>>>>  	struct perf_sample sample_out;
>>>>>>  	size_t i, sz, bufsz;
>>>>>> -- 
>>>>>> 2.35.1
>>>>>>
>>>>>
>>>
>>> -- 
>>>
>>> - Arnaldo
>>