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: Tue, 19 Jan 2021 08:49:52 +0800 [thread overview]
Message-ID: <934aa658-1147-8882-99c2-68561b42d63d@linux.intel.com> (raw)
In-Reply-To: <18fde68f-0d24-5bcb-38e1-11dcf6f70bc1@linux.intel.com>
Hi Arnaldo, Jiri,
On 12/25/2020 9:10 AM, Jin, Yao wrote:
> 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
>
Can this bug fix be accepted or anything else I need to improve?
Thanks
Jin Yao
next prev parent reply other threads:[~2021-01-19 0:51 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
2021-01-19 0:49 ` Jin, Yao [this message]
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=934aa658-1147-8882-99c2-68561b42d63d@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).