Re: [PATCH] perf tools: Support arch specific PERF_SAMPLE_WEIGHT_STRUCT processing

[Arnaldo Carvalho de Melo]

Em Fri, Feb 05, 2021 at 08:01:52AM -0800, kan.liang@xxxxxxxxxxxxxxx escreveu:
> From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> 
> For X86, the var2_w field of PERF_SAMPLE_WEIGHT_STRUCT stands for the
> instruction latency. Current perf forces the var2_w to the data->ins_lat
> in the generic code. It works well for now because X86 is the only
> architecture that supports the PERF_SAMPLE_WEIGHT_STRUCT, but it may
> bring problems once other architectures support the sample type.
> For example, the var2_w may be used to capture something else on
> PowerPC.
> 
> Create two architecture specific functions to parse and synthesize
> the weight related samples. Move the X86 specific codes to the X86
> version functions. Other architectures can implement their own functions
> later separately.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> ---
>  tools/perf/arch/x86/util/event.c   | 25 +++++++++++++++++++++++++
>  tools/perf/util/event.h            |  3 +++
>  tools/perf/util/evsel.c            | 17 ++++++++---------
>  tools/perf/util/synthetic-events.c | 12 +++++++-----
>  4 files changed, 43 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/util/event.c b/tools/perf/arch/x86/util/event.c
> index 047dc00..9b31734 100644
> --- a/tools/perf/arch/x86/util/event.c
> +++ b/tools/perf/arch/x86/util/event.c
> @@ -75,3 +75,28 @@ int perf_event__synthesize_extra_kmaps(struct perf_tool *tool,
>  }
>  
>  #endif
> +
> +void arch_perf_parse_sample_weight(struct perf_sample *data,
> +				   const __u64 *array, u64 type)
> +{
> +	union perf_sample_weight weight;
> +
> +	weight.full = *array;
> +	if (type & PERF_SAMPLE_WEIGHT)
> +		data->weight = weight.full;
> +	else {
> +		data->weight = weight.var1_dw;
> +		data->ins_lat = weight.var2_w;
> +	}
> +}
> +
> +void arch_perf_synthesize_sample_weight(const struct perf_sample *data,
> +					__u64 *array, u64 type)
> +{
> +	*array = data->weight;
> +
> +	if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
> +		*array &= 0xffffffff;
> +		*array |= ((u64)data->ins_lat << 32);
> +	}
> +}
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index 60752e4..9f123aa 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -414,4 +414,7 @@ extern unsigned int proc_map_timeout;
>  #define PAGE_SIZE_NAME_LEN	32
>  char *get_page_size_name(u64 size, char *str);
>  
> +void arch_perf_parse_sample_weight(struct perf_sample *data, const __u64 *array, u64 type);
> +void arch_perf_synthesize_sample_weight(const struct perf_sample *data, __u64 *array, u64 type);
> +
>  #endif /* __PERF_RECORD_H */
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 30b5452..1da8903 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -2106,6 +2106,13 @@ perf_event__check_size(union perf_event *event, unsigned int sample_size)
>  	return 0;
>  }
>  
> +void __weak arch_perf_parse_sample_weight(struct perf_sample *data,
> +					  const __u64 *array,
> +					  u64 type __maybe_unused)
> +{
> +	data->weight = *array;
> +}
> +
>  int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
>  			struct perf_sample *data)
>  {
> @@ -2347,16 +2354,8 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
>  	}
>  
>  	if (type & PERF_SAMPLE_WEIGHT_TYPE) {
> -		union perf_sample_weight weight;
> -
>  		OVERFLOW_CHECK_u64(array);
> -		weight.full = *array;
> -		if (type & PERF_SAMPLE_WEIGHT)
> -			data->weight = weight.full;
> -		else {
> -			data->weight = weight.var1_dw;
> -			data->ins_lat = weight.var2_w;
> -		}
> +		arch_perf_parse_sample_weight(data, array, type);
>  		array++;
>  	}
>  
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index c6f9db3..412f4c3 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -1507,6 +1507,12 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type,
>  	return result;
>  }
>  
> +void __weak arch_perf_synthesize_sample_weight(const struct perf_sample *data,
> +					       __u64 *array, u64 type __maybe_unused)
> +{
> +	*array = data->weight;
> +}
> +
>  int perf_event__synthesize_sample(union perf_event *event, u64 type, u64 read_format,
>  				  const struct perf_sample *sample)
>  {
> @@ -1643,11 +1649,7 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type, u64 read_fo
>  	}
>  
>  	if (type & PERF_SAMPLE_WEIGHT_TYPE) {
> -		*array = sample->weight;
> -		if (type & PERF_SAMPLE_WEIGHT_STRUCT) {
> -			*array &= 0xffffffff;
> -			*array |= ((u64)sample->ins_lat << 32);
> -		}
> +		arch_perf_synthesize_sample_weight(sample, array, type);
>  		array++;
>  	}
>  
> -- 
> 2.7.4
> 

-- 

- Arnaldo