Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 20 Apr 2022 13:44:31 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Florian Fischer <florian.fischer@muhq.space>
Cc:     linux-perf-users@vger.kernel.org, Ian Rogers <irogers@google.com>,
        Xing Zhengjun <zhengjun.xing@linux.intel.com>,
        linux-kernel@vger.kernel.org,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH v4 3/3] perf list: print all available tool events
Message-ID: <YmA4byoEa3QBNeST@kernel.org>
References: <20220420102354.468173-1-florian.fischer@muhq.space>
 <20220420102354.468173-4-florian.fischer@muhq.space>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220420102354.468173-4-florian.fischer@muhq.space>
Precedence: bulk

Em Wed, Apr 20, 2022 at 12:23:54PM +0200, Florian Fischer escreveu:
> Introduce names for the new tool events 'user_time' and 'system_time'.

Before:

  duration_time                                      [Tool event]

After:

$ perf list | grep Tool
  duration_time OR                                   [Tool event]
  user_time OR                                       [Tool event]
  system_time OR                                     [Tool event]
$ 

So now we have that extraneous "OR", can you please take a look?
Probably needs to check if there is an alias?

- Arnaldo
 
> Signed-off-by: Florian Fischer <florian.fischer@muhq.space>
> ---
>  tools/perf/util/evsel.c        | 19 ++++++++++------
>  tools/perf/util/evsel.h        |  1 +
>  tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++-----
>  3 files changed, 47 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 2a1729e7aee4..d38722560e80 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -597,6 +597,17 @@ static int evsel__sw_name(struct evsel *evsel, char *bf, size_t size)
>  	return r + evsel__add_modifiers(evsel, bf + r, size - r);
>  }
>  
> +const char *evsel__tool_names[PERF_TOOL_MAX] = {
> +	"duration_time",
> +	"user_time",
> +	"system_time",
> +};
> +
> +static int evsel__tool_name(enum perf_tool_event ev, char *bf, size_t size)
> +{
> +	return scnprintf(bf, size, "%s", evsel__tool_names[ev]);
> +}
> +
>  static int __evsel__bp_name(char *bf, size_t size, u64 addr, u64 type)
>  {
>  	int r;
> @@ -723,12 +734,6 @@ static int evsel__raw_name(struct evsel *evsel, char *bf, size_t size)
>  	return ret + evsel__add_modifiers(evsel, bf + ret, size - ret);
>  }
>  
> -static int evsel__tool_name(char *bf, size_t size)
> -{
> -	int ret = scnprintf(bf, size, "duration_time");
> -	return ret;
> -}
> -
>  const char *evsel__name(struct evsel *evsel)
>  {
>  	char bf[128];
> @@ -754,7 +759,7 @@ const char *evsel__name(struct evsel *evsel)
>  
>  	case PERF_TYPE_SOFTWARE:
>  		if (evsel->tool_event)
> -			evsel__tool_name(bf, sizeof(bf));
> +			evsel__tool_name(evsel->tool_event, bf, sizeof(bf));
>  		else
>  			evsel__sw_name(evsel, bf, sizeof(bf));
>  		break;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 7e2209b47b39..45d674812239 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -262,6 +262,7 @@ extern const char *evsel__hw_cache_op[PERF_COUNT_HW_CACHE_OP_MAX][EVSEL__MAX_ALI
>  extern const char *evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX][EVSEL__MAX_ALIASES];
>  extern const char *evsel__hw_names[PERF_COUNT_HW_MAX];
>  extern const char *evsel__sw_names[PERF_COUNT_SW_MAX];
> +extern const char *evsel__tool_names[PERF_TOOL_MAX];
>  extern char *evsel__bpf_counter_events;
>  bool evsel__match_bpf_counter_events(const char *name);
>  
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 064ec368a90b..c0e88d3b3e3c 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -154,6 +154,21 @@ struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
>  	},
>  };
>  
> +struct event_symbol event_symbols_tool[PERF_TOOL_MAX] = {
> +	[PERF_TOOL_DURATION_TIME] = {
> +		.symbol = "duration_time",
> +		.alias  = "",
> +	},
> +	[PERF_TOOL_USER_TIME] = {
> +		.symbol = "user_time",
> +		.alias  = "",
> +	},
> +	[PERF_TOOL_SYSTEM_TIME] = {
> +		.symbol = "system_time",
> +		.alias  = "",
> +	},
> +};
> +
>  #define __PERF_EVENT_FIELD(config, name) \
>  	((config & PERF_EVENT_##name##_MASK) >> PERF_EVENT_##name##_SHIFT)
>  
> @@ -3058,21 +3073,34 @@ int print_hwcache_events(const char *event_glob, bool name_only)
>  	return evt_num;
>  }
>  
> -static void print_tool_event(const char *name, const char *event_glob,
> +static void print_tool_event(const struct event_symbol *syms, const char *event_glob,
>  			     bool name_only)
>  {
> -	if (event_glob && !strglobmatch(name, event_glob))
> +	if (syms->symbol == NULL)
> +		return;
> +
> +	if (event_glob && !(strglobmatch(syms->symbol, event_glob) ||
> +	      (syms->alias && strglobmatch(syms->alias, event_glob))))
>  		return;
> +
>  	if (name_only)
> -		printf("%s ", name);
> -	else
> +		printf("%s ", syms->symbol);
> +	else {
> +		char name[MAX_NAME_LEN];
> +		if (syms->alias)
> +			snprintf(name, MAX_NAME_LEN, "%s OR %s", syms->symbol, syms->alias);
> +		else
> +			strlcpy(name, syms->symbol, MAX_NAME_LEN);
>  		printf("  %-50s [%s]\n", name, "Tool event");
> -
> +	}
>  }
>  
>  void print_tool_events(const char *event_glob, bool name_only)
>  {
> -	print_tool_event("duration_time", event_glob, name_only);
> +	// Start at 1 because the first enum entry symbols no tool event
> +	for (int i = 1; i < PERF_TOOL_MAX; ++i) {
> +		print_tool_event(event_symbols_tool + i, event_glob, name_only);
> +	}
>  	if (pager_in_use())
>  		printf("\n");
>  }
> -- 
> 2.36.0

-- 

- Arnaldo