Re: [PATCH RFC 06/10] perf,tools: New RECORD type PERF_RECORD_STAT_READ

[Jiri Olsa <jolsa@redhat.com>]

On Tue, Aug 18, 2015 at 05:25:42AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> Introduce a new user RECORD type PERF_RECORD_STAT_READ to store the
> stat event read result.
> 
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> ---
>  tools/perf/util/event.c   |  1 +
>  tools/perf/util/event.h   | 10 ++++++++++
>  tools/perf/util/session.c | 15 +++++++++++++++
>  tools/perf/util/tool.h    |  1 +
>  4 files changed, 27 insertions(+)
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 7ff6127..601f4be 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -37,6 +37,7 @@ static const char *perf_event__names[] = {
>  	[PERF_RECORD_AUXTRACE_INFO]		= "AUXTRACE_INFO",
>  	[PERF_RECORD_AUXTRACE]			= "AUXTRACE",
>  	[PERF_RECORD_AUXTRACE_ERROR]		= "AUXTRACE_ERROR",
> +	[PERF_RECORD_STAT_READ]			= "STAT_READ",
>  };
>  
>  const char *perf_event__name(unsigned int id)
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index f729df5..f6932cf 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -226,6 +226,7 @@ enum perf_user_event_type { /* above any possible kernel type */
>  	PERF_RECORD_AUXTRACE_INFO		= 70,
>  	PERF_RECORD_AUXTRACE			= 71,
>  	PERF_RECORD_AUXTRACE_ERROR		= 72,
> +	PERF_RECORD_STAT_READ			= 73,
>  	PERF_RECORD_HEADER_MAX
>  };
>  
> @@ -355,6 +356,14 @@ struct context_switch_event {
>  	u32 next_prev_tid;
>  };
>  
> +struct stat_read_event {
> +	struct perf_event_header header;
> +	u64	value;		/* counter value delta */
> +	u32	cpu;
> +	u32	pos_id;		/* event position in evlist */
> +	u64	time;		/* time stamp */
> +};

could we make this more generic like:

struct stat_event {
        struct perf_event_header        header;

        u64     id;
        u32     cpu;
        u32     thread;

        union {
                struct {
                        u64 val;
                        u64 ena;
                        u64 run;
                };
                u64 values[3];
        };
};


I'd agreed with the time to be added, but I think it should
contains raw values, not deltas (and all 3 them: val, ena, run)

also I haven't got the value of the pos_id, so far seems
like this could be obtained in report time and does not
need to be part of the event


thanks,
jirka