From: "Jin, Yao" <yao.jin@linux.intel.com>
To: Adrian Hunter <adrian.hunter@intel.com>,
acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
mingo@redhat.com, alexander.shishkin@linux.intel.com
Cc: Linux-kernel@vger.kernel.org, ak@linux.intel.com,
kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v2] perf script: Fix overrun issue for dynamically-allocated pmu type number
Date: Fri, 25 Dec 2020 09:10:23 +0800 [thread overview]
Message-ID: <18fde68f-0d24-5bcb-38e1-11dcf6f70bc1@linux.intel.com> (raw)
In-Reply-To: <3fcdc860-d858-0166-de23-34fc6fe5c1cd@intel.com>
Hi Arnaldo, Jiri,
On 12/11/2020 2:10 PM, Adrian Hunter wrote:
> On 9/12/20 2:58 am, Jin Yao wrote:
>> When unpacking the event which is from dynamic pmu, the array
>> output[OUTPUT_TYPE_MAX] may be overrun. For example, type number of
>> SKL uncore_imc is 10, but OUTPUT_TYPE_MAX is 7 now (OUTPUT_TYPE_MAX =
>> PERF_TYPE_MAX + 1).
>>
>> /* In builtin-script.c */
>> process_event()
>> {
>> unsigned int type = output_type(attr->type);
>>
>> if (output[type].fields == 0)
>> return;
>> }
>>
>> output[10] is overrun.
>>
>> Create a type OUTPUT_TYPE_OTHER for dynamic pmu events, then
>> output_type(attr->type) will return OUTPUT_TYPE_OTHER here.
>>
>> Note that if PERF_TYPE_MAX ever changed, then there would be a conflict
>> between old perf.data files that had a dynamicaliy allocated PMU number
>> that would then be the same as a fixed PERF_TYPE.
>>
>> Example:
>>
>> perf record --switch-events -C 0 -e "{cpu-clock,uncore_imc/data_reads/,uncore_imc/data_writes/}:SD" -a -- sleep 1
>> perf script
>>
>> Before:
>> swapper 0 [000] 1479253.987551: 277766 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1479253.987797: 246709 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1479253.988127: 329883 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1479253.988273: 146393 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1479253.988523: 249977 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1479253.988877: 354090 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1479253.989023: 145940 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1479253.989383: 359856 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1479253.989523: 140082 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>>
>> After:
>> swapper 0 [000] 1397040.402011: 272384 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1397040.402011: 5396 uncore_imc/data_reads/:
>> swapper 0 [000] 1397040.402011: 967 uncore_imc/data_writes/:
>> swapper 0 [000] 1397040.402259: 249153 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1397040.402259: 7231 uncore_imc/data_reads/:
>> swapper 0 [000] 1397040.402259: 1297 uncore_imc/data_writes/:
>> swapper 0 [000] 1397040.402508: 249108 cpu-clock: ffffffff9d4ddb6f cpuidle_enter_state+0xdf ([kernel.kallsyms])
>> swapper 0 [000] 1397040.402508: 5333 uncore_imc/data_reads/:
>> swapper 0 [000] 1397040.402508: 1008 uncore_imc/data_writes/:
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>
Can this patch be accepted? :)
Thanks
Jin Yao
>> ---
>> v2:
>> Remove Fixes tag because this issue has always been here, not caused by
>> 1405720d4f26 ("perf script: Add 'synth' event type for synthesized events").
>> No functional change in v2.
>>
>> tools/perf/builtin-script.c | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>> index 1c322c129185..5d8a64836228 100644
>> --- a/tools/perf/builtin-script.c
>> +++ b/tools/perf/builtin-script.c
>> @@ -183,6 +183,7 @@ struct output_option {
>>
>> enum {
>> OUTPUT_TYPE_SYNTH = PERF_TYPE_MAX,
>> + OUTPUT_TYPE_OTHER,
>> OUTPUT_TYPE_MAX
>> };
>>
>> @@ -279,6 +280,18 @@ static struct {
>>
>> .invalid_fields = PERF_OUTPUT_TRACE | PERF_OUTPUT_BPF_OUTPUT,
>> },
>> +
>> + [OUTPUT_TYPE_OTHER] = {
>> + .user_set = false,
>> +
>> + .fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |
>> + PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
>> + PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
>> + PERF_OUTPUT_SYM | PERF_OUTPUT_SYMOFFSET |
>> + PERF_OUTPUT_DSO | PERF_OUTPUT_PERIOD,
>> +
>> + .invalid_fields = PERF_OUTPUT_TRACE | PERF_OUTPUT_BPF_OUTPUT,
>> + },
>> };
>>
>> struct evsel_script {
>> @@ -339,8 +352,11 @@ static inline int output_type(unsigned int type)
>> case PERF_TYPE_SYNTH:
>> return OUTPUT_TYPE_SYNTH;
>> default:
>> - return type;
>> + if (type < PERF_TYPE_MAX)
>> + return type;
>> }
>> +
>> + return OUTPUT_TYPE_OTHER;
>> }
>>
>> static inline unsigned int attr_type(unsigned int type)
>>
>
next prev parent reply other threads:[~2020-12-25 1:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-09 0:58 [PATCH v2] perf script: Fix overrun issue for dynamically-allocated pmu type number Jin Yao
2020-12-11 6:10 ` Adrian Hunter
2020-12-25 1:10 ` Jin, Yao [this message]
2021-01-19 0:49 ` Jin, Yao
2021-01-20 21:50 ` Jiri Olsa
2021-01-21 20:24 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=18fde68f-0d24-5bcb-38e1-11dcf6f70bc1@linux.intel.com \
--to=yao.jin@linux.intel.com \
--cc=Linux-kernel@vger.kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@intel.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yao.jin@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).