All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.