* problem with 'perf script -F weight' @ 2021-09-28 19:20 Jiri Olsa 2021-09-28 20:38 ` Andi Kleen 2021-09-28 21:35 ` Liang, Kan 0 siblings, 2 replies; 5+ messages in thread From: Jiri Olsa @ 2021-09-28 19:20 UTC (permalink / raw) To: Kan Liang Cc: linux-perf-users, Peter Zijlstra, Joe Mario, Andi Kleen, Jin Yao, Arnaldo Carvalho de Melo hi, Joe reported broken -F weight in perf script with (on x86): # ./perf mem record # ./perf script -F weight Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot print 'weight' field. the problem seems to be introduced with the WEIGHT_STRUCT change: ea8d0ed6eae3 perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT which enables WEIGHT_STRUCT sample_type on x86 instead of WEIGHT, and leaves 'perf script -F weight' in the dark I'm not sure what the fix should look like.. should we allow user to use just 'WEIGHT' sample type? or is there a way to get original weight from 'WEIGHT_STRUCT' data, so we could fix just perf script? FYI when I switch back the WEIGHT_STRUCT to WEIGHT again it seems to work as before.. as expected ;-) any thoughts? thanks jirka --- diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c index 2f733cdc8dbb..6bdd744894ad 100644 --- a/tools/perf/arch/x86/util/evsel.c +++ b/tools/perf/arch/x86/util/evsel.c @@ -4,5 +4,5 @@ void arch_evsel__set_sample_weight(struct evsel *evsel) { - evsel__set_sample_bit(evsel, WEIGHT_STRUCT); + evsel__set_sample_bit(evsel, WEIGHT); } ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: problem with 'perf script -F weight' 2021-09-28 19:20 problem with 'perf script -F weight' Jiri Olsa @ 2021-09-28 20:38 ` Andi Kleen 2021-09-28 21:35 ` Liang, Kan 1 sibling, 0 replies; 5+ messages in thread From: Andi Kleen @ 2021-09-28 20:38 UTC (permalink / raw) To: Jiri Olsa, Kan Liang Cc: linux-perf-users, Peter Zijlstra, Joe Mario, Jin Yao, Arnaldo Carvalho de Melo On 9/28/2021 12:20 PM, Jiri Olsa wrote: > hi, > Joe reported broken -F weight in perf script with (on x86): > > # ./perf mem record > # ./perf script -F weight > Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot print 'weight' field. > > the problem seems to be introduced with the WEIGHT_STRUCT change: > ea8d0ed6eae3 perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT > > which enables WEIGHT_STRUCT sample_type on x86 instead of WEIGHT, > and leaves 'perf script -F weight' in the dark > > I'm not sure what the fix should look like.. should we allow user > to use just 'WEIGHT' sample type? or is there a way to get original > weight from 'WEIGHT_STRUCT' data, so we could fix just perf script? We would need to support both in perf script, to handle both perf.data generated by old and new kernels on both big and little endian So probably try one first and then the other while parsing The same is true for all other users, so probably there needs to be a library function for this somewhere. -Andi ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: problem with 'perf script -F weight' 2021-09-28 19:20 problem with 'perf script -F weight' Jiri Olsa 2021-09-28 20:38 ` Andi Kleen @ 2021-09-28 21:35 ` Liang, Kan 2021-09-29 6:08 ` Jiri Olsa 1 sibling, 1 reply; 5+ messages in thread From: Liang, Kan @ 2021-09-28 21:35 UTC (permalink / raw) To: Jiri Olsa Cc: linux-perf-users, Peter Zijlstra, Joe Mario, Andi Kleen, Jin Yao, Arnaldo Carvalho de Melo On 9/28/2021 3:20 PM, Jiri Olsa wrote: > Joe reported broken -F weight in perf script with (on x86): > > # ./perf mem record > # ./perf script -F weight > Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot print 'weight' field. > > the problem seems to be introduced with the WEIGHT_STRUCT change: > ea8d0ed6eae3 perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT > > which enables WEIGHT_STRUCT sample_type on x86 instead of WEIGHT, > and leaves 'perf script -F weight' in the dark Right, I missed the support for the perf script. Because the default perf script worked. It didn't ring the bell. :( > > I'm not sure what the fix should look like.. should we allow user > to use just 'WEIGHT' sample type? or is there a way to get original > weight from 'WEIGHT_STRUCT' data, so we could fix just perf script? Just fix the perf script is enough. Could you please try the below patch? (I probably need to split the patch into two patches, since it fixed two issues, one is this error, the other is to print the instruction latency. Anyway, let's try it first.) From 99443f49a6236ef6e3bea3930792967a7863f753 Mon Sep 17 00:00:00 2001 From: Kan Liang <kan.liang@linux.intel.com> Date: Tue, 28 Sep 2021 13:57:18 -0700 Subject: [PATCH] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support -F weight in perf script is broken. # ./perf mem record # ./perf script -F weight Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot print 'weight' field. The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The lower 32 bits are exactly the same for both sample type. The higher 32 bits may be different for different architecture. For a new kernel on x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other ARCHs, the PERF_SAMPLE_WEIGHT is used. With -F weight, current perf script will check the input string "weight" with the corresponding sample type. However, the commit ID ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't update the sample type for perf script. The check fails with a new kernel on x86. Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to replace PERF_SAMPLE_WEIGHT. Also, print the instruction latency for PERF_SAMPLE_WEIGHT_STRUCT type. Reported-by: Joe Mario <jmario@redhat.com> Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") Signed-off-by: Kan Liang <kan.liang@linux.intel.com> --- tools/perf/builtin-script.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 6211d0b..fd73bbc 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session) return -EINVAL; if (PRINT_FIELD(WEIGHT) && - evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", PERF_OUTPUT_WEIGHT)) + evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", PERF_OUTPUT_WEIGHT)) return -EINVAL; if (PRINT_FIELD(SYM) && @@ -2036,8 +2036,11 @@ static void process_event(struct perf_script *script, if (PRINT_FIELD(DATA_SRC)) data_src__fprintf(sample->data_src, fp); - if (PRINT_FIELD(WEIGHT)) + if (PRINT_FIELD(WEIGHT)) { fprintf(fp, "%16" PRIu64, sample->weight); + if (attr->sample_type & PERF_SAMPLE_WEIGHT_STRUCT) + fprintf(fp, ",0x%" PRIx16, sample->ins_lat); + } if (PRINT_FIELD(IP)) { struct callchain_cursor *cursor = NULL; -- 2.7.4 Thanks, Kan ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: problem with 'perf script -F weight' 2021-09-28 21:35 ` Liang, Kan @ 2021-09-29 6:08 ` Jiri Olsa 2021-09-29 15:13 ` Liang, Kan 0 siblings, 1 reply; 5+ messages in thread From: Jiri Olsa @ 2021-09-29 6:08 UTC (permalink / raw) To: Liang, Kan Cc: linux-perf-users, Peter Zijlstra, Joe Mario, Andi Kleen, Jin Yao, Arnaldo Carvalho de Melo On Tue, Sep 28, 2021 at 05:35:51PM -0400, Liang, Kan wrote: > > > On 9/28/2021 3:20 PM, Jiri Olsa wrote: > > Joe reported broken -F weight in perf script with (on x86): > > > > # ./perf mem record > > # ./perf script -F weight > > Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot print 'weight' field. > > > > the problem seems to be introduced with the WEIGHT_STRUCT change: > > ea8d0ed6eae3 perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT > > > > which enables WEIGHT_STRUCT sample_type on x86 instead of WEIGHT, > > and leaves 'perf script -F weight' in the dark > > Right, I missed the support for the perf script. Because the default perf > script worked. It didn't ring the bell. :( > > > > > I'm not sure what the fix should look like.. should we allow user > > to use just 'WEIGHT' sample type? or is there a way to get original > > weight from 'WEIGHT_STRUCT' data, so we could fix just perf script? > > Just fix the perf script is enough. > Could you please try the below patch? > > (I probably need to split the patch into two patches, since it fixed two > issues, one is this error, the other is to print the instruction latency. > Anyway, let's try it first.) > > From 99443f49a6236ef6e3bea3930792967a7863f753 Mon Sep 17 00:00:00 2001 > From: Kan Liang <kan.liang@linux.intel.com> > Date: Tue, 28 Sep 2021 13:57:18 -0700 > Subject: [PATCH] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support > > -F weight in perf script is broken. > > # ./perf mem record > # ./perf script -F weight > Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot > print 'weight' field. > > The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the > PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The > lower 32 bits are exactly the same for both sample type. The higher 32 > bits may be different for different architecture. For a new kernel on > x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other > ARCHs, the PERF_SAMPLE_WEIGHT is used. > > With -F weight, current perf script will check the input string "weight" > with the corresponding sample type. However, the commit ID ea8d0ed6eae3 > ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't update the > sample type for perf script. The check fails with a new kernel on x86. > > Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to > replace PERF_SAMPLE_WEIGHT. > > Also, print the instruction latency for PERF_SAMPLE_WEIGHT_STRUCT type. > > Reported-by: Joe Mario <jmario@redhat.com> > Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") > Signed-off-by: Kan Liang <kan.liang@linux.intel.com> > --- > tools/perf/builtin-script.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > index 6211d0b..fd73bbc 100644 > --- a/tools/perf/builtin-script.c > +++ b/tools/perf/builtin-script.c > @@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct > perf_session *session) > return -EINVAL; > > if (PRINT_FIELD(WEIGHT) && > - evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", > PERF_OUTPUT_WEIGHT)) > + evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", > PERF_OUTPUT_WEIGHT)) > return -EINVAL; > > if (PRINT_FIELD(SYM) && > @@ -2036,8 +2036,11 @@ static void process_event(struct perf_script *script, > if (PRINT_FIELD(DATA_SRC)) > data_src__fprintf(sample->data_src, fp); > > - if (PRINT_FIELD(WEIGHT)) > + if (PRINT_FIELD(WEIGHT)) { > fprintf(fp, "%16" PRIu64, sample->weight); > + if (attr->sample_type & PERF_SAMPLE_WEIGHT_STRUCT) > + fprintf(fp, ",0x%" PRIx16, sample->ins_lat); this will add extra number to weight.. should we add separate column for that -F 'insn-lat' ? users expect just single value for -F weight thanks, jirka > + } > > if (PRINT_FIELD(IP)) { > struct callchain_cursor *cursor = NULL; > -- > 2.7.4 > > Thanks, > Kan > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: problem with 'perf script -F weight' 2021-09-29 6:08 ` Jiri Olsa @ 2021-09-29 15:13 ` Liang, Kan 0 siblings, 0 replies; 5+ messages in thread From: Liang, Kan @ 2021-09-29 15:13 UTC (permalink / raw) To: Jiri Olsa Cc: linux-perf-users, Peter Zijlstra, Joe Mario, Andi Kleen, Jin Yao, Arnaldo Carvalho de Melo On 9/29/2021 2:08 AM, Jiri Olsa wrote: > On Tue, Sep 28, 2021 at 05:35:51PM -0400, Liang, Kan wrote: >> >> >> On 9/28/2021 3:20 PM, Jiri Olsa wrote: >>> Joe reported broken -F weight in perf script with (on x86): >>> >>> # ./perf mem record >>> # ./perf script -F weight >>> Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot print 'weight' field. >>> >>> the problem seems to be introduced with the WEIGHT_STRUCT change: >>> ea8d0ed6eae3 perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT >>> >>> which enables WEIGHT_STRUCT sample_type on x86 instead of WEIGHT, >>> and leaves 'perf script -F weight' in the dark >> >> Right, I missed the support for the perf script. Because the default perf >> script worked. It didn't ring the bell. :( >> >>> >>> I'm not sure what the fix should look like.. should we allow user >>> to use just 'WEIGHT' sample type? or is there a way to get original >>> weight from 'WEIGHT_STRUCT' data, so we could fix just perf script? >> >> Just fix the perf script is enough. >> Could you please try the below patch? >> >> (I probably need to split the patch into two patches, since it fixed two >> issues, one is this error, the other is to print the instruction latency. >> Anyway, let's try it first.) >> >> From 99443f49a6236ef6e3bea3930792967a7863f753 Mon Sep 17 00:00:00 2001 >> From: Kan Liang <kan.liang@linux.intel.com> >> Date: Tue, 28 Sep 2021 13:57:18 -0700 >> Subject: [PATCH] perf script: Fix PERF_SAMPLE_WEIGHT_STRUCT support >> >> -F weight in perf script is broken. >> >> # ./perf mem record >> # ./perf script -F weight >> Samples for 'dummy:HG' event do not have WEIGHT attribute set. Cannot >> print 'weight' field. >> >> The sample type, PERF_SAMPLE_WEIGHT_STRUCT, is an alternative of the >> PERF_SAMPLE_WEIGHT sample type. They share the same space, weight. The >> lower 32 bits are exactly the same for both sample type. The higher 32 >> bits may be different for different architecture. For a new kernel on >> x86, the PERF_SAMPLE_WEIGHT_STRUCT is used. For an old kernel or other >> ARCHs, the PERF_SAMPLE_WEIGHT is used. >> >> With -F weight, current perf script will check the input string "weight" >> with the corresponding sample type. However, the commit ID ea8d0ed6eae3 >> ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") didn't update the >> sample type for perf script. The check fails with a new kernel on x86. >> >> Use PERF_SAMPLE_WEIGHT_TYPE, which supports both sample types, to >> replace PERF_SAMPLE_WEIGHT. >> >> Also, print the instruction latency for PERF_SAMPLE_WEIGHT_STRUCT type. >> >> Reported-by: Joe Mario <jmario@redhat.com> >> Fixes: ea8d0ed6eae3 ("perf tools: Support PERF_SAMPLE_WEIGHT_STRUCT") >> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> >> --- >> tools/perf/builtin-script.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c >> index 6211d0b..fd73bbc 100644 >> --- a/tools/perf/builtin-script.c >> +++ b/tools/perf/builtin-script.c >> @@ -459,7 +459,7 @@ static int evsel__check_attr(struct evsel *evsel, struct >> perf_session *session) >> return -EINVAL; >> >> if (PRINT_FIELD(WEIGHT) && >> - evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT, "WEIGHT", >> PERF_OUTPUT_WEIGHT)) >> + evsel__check_stype(evsel, PERF_SAMPLE_WEIGHT_TYPE, "WEIGHT", >> PERF_OUTPUT_WEIGHT)) >> return -EINVAL; >> >> if (PRINT_FIELD(SYM) && >> @@ -2036,8 +2036,11 @@ static void process_event(struct perf_script *script, >> if (PRINT_FIELD(DATA_SRC)) >> data_src__fprintf(sample->data_src, fp); >> >> - if (PRINT_FIELD(WEIGHT)) >> + if (PRINT_FIELD(WEIGHT)) { >> fprintf(fp, "%16" PRIu64, sample->weight); >> + if (attr->sample_type & PERF_SAMPLE_WEIGHT_STRUCT) >> + fprintf(fp, ",0x%" PRIx16, sample->ins_lat); > > this will add extra number to weight.. should we add separate column > for that -F 'insn-lat' ? users expect just single value for -F weight > Sure, I will send out a patch set to LKML for review shortly. Thanks, Kan ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-29 15:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-09-28 19:20 problem with 'perf script -F weight' Jiri Olsa 2021-09-28 20:38 ` Andi Kleen 2021-09-28 21:35 ` Liang, Kan 2021-09-29 6:08 ` Jiri Olsa 2021-09-29 15:13 ` Liang, Kan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).