Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[Jiri Olsa]

On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:

SNIP

>  static const char *perf_ns__names[] = {
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index db2de6413518..d404f50260f8 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -244,6 +244,19 @@ enum perf_user_event_type { /* above any possible kernel type */
>  	PERF_RECORD_STAT_ROUND			= 77,
>  	PERF_RECORD_EVENT_UPDATE		= 78,
>  	PERF_RECORD_TIME_CONV			= 79,
> +	PERF_RECORD_HEADER_HOSTNAME		= 80,
> +	PERF_RECORD_HEADER_OSRELEASE		= 81,
> +	PERF_RECORD_HEADER_VERSION		= 82,
> +	PERF_RECORD_HEADER_ARCH			= 83,
> +	PERF_RECORD_HEADER_NRCPUS		= 84,
> +	PERF_RECORD_HEADER_CPUDESC		= 85,
> +	PERF_RECORD_HEADER_CPUID		= 86,
> +	PERF_RECORD_HEADER_TOTAL_MEM		= 87,
> +	PERF_RECORD_HEADER_CMDLINE		= 88,
> +	PERF_RECORD_HEADER_EVENT_DESC		= 89,
> +	PERF_RECORD_HEADER_CPU_TOPOLOGY		= 90,
> +	PERF_RECORD_HEADER_NUMA_TOPOLOGY	= 91,
> +	PERF_RECORD_HEADER_PMU_MAPPINGS		= 92,
>  	PERF_RECORD_HEADER_MAX
>  };

SNIP

> +
> +/*
> + * we use a mapping table to go from record type to feature header
> + * because we have no guarantee that new record types may not be added
> + * after the feature header.
> + */
> +#define REC2FEAT(a)	[PERF_RECORD_HEADER_##a] = HEADER_##a
> +
> +static const int rec2feat[PERF_RECORD_HEADER_MAX] = {
> +	REC2FEAT(HOSTNAME),
> +	REC2FEAT(OSRELEASE),
> +	REC2FEAT(VERSION),
> +	REC2FEAT(ARCH),
> +	REC2FEAT(NRCPUS),
> +	REC2FEAT(CPUDESC),
> +	REC2FEAT(CPUID),
> +	REC2FEAT(TOTAL_MEM),
> +	REC2FEAT(EVENT_DESC),
> +	REC2FEAT(CMDLINE),
> +	REC2FEAT(CPU_TOPOLOGY),
> +	REC2FEAT(NUMA_TOPOLOGY),
> +	REC2FEAT(PMU_MAPPINGS),
>  };

hum, how about instead of adding all those new events we add
just one: PERF_RECORD_FEATURE, which would carry the id of the
feature, like:

struct feature_event {
	struct perf_event_header	header;
	u64	id;
	char	data[]; /* size bytes of raw data specific to the feature */
};


this way we could also omit the reverse event to feature map

thanks,
jirka