* [PATCH v2] perf script: Fix overrun issue for dynamically-allocated pmu type number
@ 2020-12-09 0:58 Jin Yao
2020-12-11 6:10 ` Adrian Hunter
2021-01-20 21:50 ` Jiri Olsa
0 siblings, 2 replies; 6+ messages in thread
From: Jin Yao @ 2020-12-09 0:58 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin, adrian.hunter
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
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>
---
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)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf script: Fix overrun issue for dynamically-allocated pmu type number
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-20 21:50 ` Jiri Olsa
1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2020-12-11 6:10 UTC (permalink / raw)
To: Jin Yao, acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin
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>
> ---
> 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)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf script: Fix overrun issue for dynamically-allocated pmu type number
2020-12-11 6:10 ` Adrian Hunter
@ 2020-12-25 1:10 ` Jin, Yao
2021-01-19 0:49 ` Jin, Yao
0 siblings, 1 reply; 6+ messages in thread
From: Jin, Yao @ 2020-12-25 1:10 UTC (permalink / raw)
To: Adrian Hunter, acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin
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)
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf script: Fix overrun issue for dynamically-allocated pmu type number
2020-12-25 1:10 ` Jin, Yao
@ 2021-01-19 0:49 ` Jin, Yao
0 siblings, 0 replies; 6+ messages in thread
From: Jin, Yao @ 2021-01-19 0:49 UTC (permalink / raw)
To: Adrian Hunter, acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf script: Fix overrun issue for dynamically-allocated pmu type number
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
@ 2021-01-20 21:50 ` Jiri Olsa
2021-01-21 20:24 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2021-01-20 21:50 UTC (permalink / raw)
To: Jin Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, adrian.hunter,
Linux-kernel, ak, kan.liang, yao.jin
On Wed, Dec 09, 2020 at 08:58:28AM +0800, 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>
> ---
> 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.
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf script: Fix overrun issue for dynamically-allocated pmu type number
2021-01-20 21:50 ` Jiri Olsa
@ 2021-01-21 20:24 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-01-21 20:24 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jin Yao, jolsa, peterz, mingo, alexander.shishkin, adrian.hunter,
Linux-kernel, ak, kan.liang, yao.jin
Em Wed, Jan 20, 2021 at 10:50:04PM +0100, Jiri Olsa escreveu:
> On Wed, Dec 09, 2020 at 08:58:28AM +0800, Jin Yao wrote:
> > 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>
> > ---
> > 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.
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
Thanks, applied.
- Arnaldo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-21 20:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2021-01-20 21:50 ` Jiri Olsa
2021-01-21 20:24 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.