Re: [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix

[Jiri Olsa <jolsa@redhat.com>]

On Tue, Sep 02, 2014 at 11:29:29AM -0400, kan.liang@intel.com wrote:

SNIP

>  {
>  	YY_BUFFER_STATE buffer;
> @@ -906,7 +1006,10 @@ int parse_events(struct perf_evlist *evlist, const char *str)
>  	};
>  	int ret;
>  
> +	/* scan kernel pmu events from sysfs */
> +	scan_kernel_pmu_events_list();
>  	ret = parse_events__scanner(str, &data, PE_START_EVENTS);
> +	release_kernel_pmu_events_list();
>  	if (!ret) {
>  		int entries = data.idx - evlist->nr_entries;
>  		perf_evlist__splice_list_tail(evlist, &data.list, entries);
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index df094b4..d06fec4 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -35,6 +35,19 @@ extern int parse_filter(const struct option *opt, const char *str, int unset);
>  
>  #define EVENTS_HELP_MAX (128*1024)
>  
> +#define KERNEL_PMU_EVENT_MAX	1024

hum.. so roughly for 15 (+-) strings of size around 20 bytes we will
use 15K of memory.. seems like overkill

seems better to allocate each symbol string separatelly, and
update the release function

> +enum kernel_pmu_event_type {
> +	NONE_KERNEL_PMU_EVENT,		/* not a PMU EVENT */
> +	KERNEL_PMU_EVENT,		/* normal style PMU event */
> +	KERNEL_PMU_EVENT_PREFIX,	/* prefix of pre-suf style event */
> +	KERNEL_PMU_EVENT_SUFFIX,	/* suffix of pre-suf style event */
> +};
> +
> +struct kernel_pmu_event_symbol {
> +	char	symbol[KERNEL_PMU_EVENT_MAX];
> +	enum kernel_pmu_event_type	type;
> +};
> +

also, I think this is more pmu object related stuff.. how about:

enum perf_pmu_event_symbol_type
struct perf_pmu_event_symbol

  perf_pmu__parse_init
  perf_pmu__parse_cleanup
  perf_pmu__parse_check

with perf_pmu__parse_init being called from perf_pmu__parse_check
in case it's needed..

and perf_pmu__parse_cleanup being called from parse_events same as
release_kernel_pmu_events_list

jirka