Re: [PATCH 4/3] perf_counter: Correct PERF_SAMPLE_RAW output

[Frederic Weisbecker <fweisbec@gmail.com>]

On Mon, Aug 10, 2009 at 11:27:27AM +0200, Peter Zijlstra wrote:
> Subject: perf_counter: Correct PERF_SAMPLE_RAW output
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Mon Aug 10 11:16:52 CEST 2009
> 
> PERF_SAMPLE_* output switches should unconditionally output the
> correct format, as they are the only way to unambiguously parse the
> PERF_EVENT_SAMPLE data.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>


I economized this part because I thought ftrace events could
be parsed the same way we parse binary events through debugfs.
The size of an ftrace event can be deduced from its format.

But I must confess that would require a pre-parsing of dynamic
strings.

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>


> ---
>  include/linux/perf_counter.h |    2 ++
>  include/trace/ftrace.h       |    3 ++-
>  kernel/perf_counter.c        |   30 ++++++++++++++++++++++++------
>  3 files changed, 28 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6/include/linux/perf_counter.h
> ===================================================================
> --- linux-2.6.orig/include/linux/perf_counter.h
> +++ linux-2.6/include/linux/perf_counter.h
> @@ -369,6 +369,8 @@ enum perf_event_type {
>  	 *
>  	 *	{ u64			nr,
>  	 *	  u64			ips[nr];  } && PERF_SAMPLE_CALLCHAIN
> +	 *	{ u32			size;
> +	 *	  char                  data[size];}&& PERF_SAMPLE_RAW
>  	 * };
>  	 */
>  	PERF_EVENT_SAMPLE		= 9,
> Index: linux-2.6/include/trace/ftrace.h
> ===================================================================
> --- linux-2.6.orig/include/trace/ftrace.h
> +++ linux-2.6/include/trace/ftrace.h
> @@ -687,7 +687,8 @@ static void ftrace_profile_##call(proto)
>  	pc = preempt_count();						\
>  									\
>  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
> -	__entry_size = ALIGN(__data_size + sizeof(*entry), sizeof(u64));\
> +	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
> +			     sizeof(u64));				\
>  									\
>  	do {								\
>  		char raw_data[__entry_size];				\
> Index: linux-2.6/kernel/perf_counter.c
> ===================================================================
> --- linux-2.6.orig/kernel/perf_counter.c
> +++ linux-2.6/kernel/perf_counter.c
> @@ -2647,7 +2647,6 @@ void perf_counter_output(struct perf_cou
>  		u64 counter;
>  	} group_entry;
>  	struct perf_callchain_entry *callchain = NULL;
> -	struct perf_raw_record *raw = NULL;
>  	int callchain_size = 0;
>  	u64 time;
>  	struct {
> @@ -2717,9 +2716,15 @@ void perf_counter_output(struct perf_cou
>  	}
>  
>  	if (sample_type & PERF_SAMPLE_RAW) {
> -		raw = data->raw;
> -		if (raw)
> -			header.size += raw->size;
> +		int size = sizeof(u32);
> +
> +		if (data->raw)
> +			size += data->raw->size;
> +		else
> +			size += sizeof(u32);
> +
> +		WARN_ON_ONCE(size & (sizeof(u64)-1));
> +		header.size += size;
>  	}
>  
>  	ret = perf_output_begin(&handle, counter, header.size, nmi, 1);
> @@ -2785,8 +2790,21 @@ void perf_counter_output(struct perf_cou
>  		}
>  	}
>  
> -	if ((sample_type & PERF_SAMPLE_RAW) && raw)
> -		perf_output_copy(&handle, raw->data, raw->size);
> +	if (sample_type & PERF_SAMPLE_RAW) {
> +		if (data->raw) {
> +			perf_output_put(&handle, data->raw->size);
> +			perf_output_copy(&handle, data->raw->data, data->raw->size);
> +		} else {
> +			struct {
> +				u32	size;
> +				u32	data;
> +			} raw = {
> +				.size = sizeof(u32),
> +				.data = 0,
> +			};
> +			perf_output_put(&handle, raw);
> +		}
> +	}
>  
>  	perf_output_end(&handle);
>  }
>