* [PATCH V3 1/3] perf report: Support Retire Latency @ 2023-02-02 19:22 kan.liang 2023-02-02 19:22 ` [PATCH V3 2/3] perf script: " kan.liang 2023-02-02 19:22 ` [PATCH V3 3/3] perf test: Support the retire_lat check kan.liang 0 siblings, 2 replies; 10+ messages in thread From: kan.liang @ 2023-02-02 19:22 UTC (permalink / raw) To: acme, peterz, mingo, linux-kernel; +Cc: ak, eranian, irogers, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH V3 2/3] perf script: Support Retire Latency 2023-02-02 19:22 [PATCH V3 1/3] perf report: Support Retire Latency kan.liang @ 2023-02-02 19:22 ` kan.liang 2023-02-02 19:22 ` [PATCH V3 3/3] perf test: Support the retire_lat check kan.liang 1 sibling, 0 replies; 10+ messages in thread From: kan.liang @ 2023-02-02 19:22 UTC (permalink / raw) To: acme, peterz, mingo, linux-kernel; +Cc: ak, eranian, irogers, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH V3 3/3] perf test: Support the retire_lat check 2023-02-02 19:22 [PATCH V3 1/3] perf report: Support Retire Latency kan.liang 2023-02-02 19:22 ` [PATCH V3 2/3] perf script: " kan.liang @ 2023-02-02 19:22 ` kan.liang 2023-02-06 15:01 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 10+ messages in thread From: kan.liang @ 2023-02-02 19:22 UTC (permalink / raw) To: acme, peterz, mingo, linux-kernel; +Cc: ak, eranian, irogers, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH V3 3/3] perf test: Support the retire_lat check 2023-02-02 19:22 ` [PATCH V3 3/3] perf test: Support the retire_lat check kan.liang @ 2023-02-06 15:01 ` Arnaldo Carvalho de Melo 2023-02-06 15:17 ` Liang, Kan 2023-02-06 15:23 ` Arnaldo Carvalho de Melo 0 siblings, 2 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-02-06 15:01 UTC (permalink / raw) To: kan.liang; +Cc: peterz, mingo, linux-kernel, ak, eranian, irogers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 3/3] perf test: Support the retire_lat check 2023-02-06 15:01 ` Arnaldo Carvalho de Melo @ 2023-02-06 15:17 ` Liang, Kan 2023-02-06 15:32 ` Arnaldo Carvalho de Melo 2023-02-06 15:23 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 10+ messages in thread From: Liang, Kan @ 2023-02-06 15:17 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: peterz, mingo, linux-kernel, ak, eranian, irogers 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 >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 3/3] perf test: Support the retire_lat check 2023-02-06 15:17 ` Liang, Kan @ 2023-02-06 15:32 ` Arnaldo Carvalho de Melo 2023-02-06 15:34 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-02-06 15:32 UTC (permalink / raw) To: Liang, Kan; +Cc: peterz, mingo, linux-kernel, ak, eranian, irogers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 3/3] perf test: Support the retire_lat check 2023-02-06 15:32 ` Arnaldo Carvalho de Melo @ 2023-02-06 15:34 ` Arnaldo Carvalho de Melo 2023-02-06 15:49 ` Liang, Kan 0 siblings, 1 reply; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-02-06 15:34 UTC (permalink / raw) To: Liang, Kan; +Cc: peterz, mingo, linux-kernel, ak, eranian, irogers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 3/3] perf test: Support the retire_lat check 2023-02-06 15:34 ` Arnaldo Carvalho de Melo @ 2023-02-06 15:49 ` Liang, Kan 2023-02-06 16:25 ` Liang, Kan 0 siblings, 1 reply; 10+ messages in thread From: Liang, Kan @ 2023-02-06 15:49 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: peterz, mingo, linux-kernel, ak, eranian, irogers 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 3/3] perf test: Support the retire_lat check 2023-02-06 15:49 ` Liang, Kan @ 2023-02-06 16:25 ` Liang, Kan 0 siblings, 0 replies; 10+ messages in thread From: Liang, Kan @ 2023-02-06 16:25 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: peterz, mingo, linux-kernel, ak, eranian, irogers 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 >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3 3/3] perf test: Support the retire_lat check 2023-02-06 15:01 ` Arnaldo Carvalho de Melo 2023-02-06 15:17 ` Liang, Kan @ 2023-02-06 15:23 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-02-06 15:23 UTC (permalink / raw) To: kan.liang; +Cc: peterz, mingo, linux-kernel, ak, eranian, irogers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-02-06 16:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-02 19:22 [PATCH V3 1/3] perf report: Support Retire Latency kan.liang 2023-02-02 19:22 ` [PATCH V3 2/3] perf script: " kan.liang 2023-02-02 19:22 ` [PATCH V3 3/3] perf test: Support the retire_lat check kan.liang 2023-02-06 15:01 ` Arnaldo Carvalho de Melo 2023-02-06 15:17 ` Liang, Kan 2023-02-06 15:32 ` Arnaldo Carvalho de Melo 2023-02-06 15:34 ` Arnaldo Carvalho de Melo 2023-02-06 15:49 ` Liang, Kan 2023-02-06 16:25 ` Liang, Kan 2023-02-06 15:23 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox