Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 16 Mar 2021 00:04:09 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Jin Yao <yao.jin@linux.intel.com>
Cc:     acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
        mingo@redhat.com, alexander.shishkin@linux.intel.com,
        Linux-kernel@vger.kernel.org, ak@linux.intel.com,
        kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v2 08/27] perf stat: Uniquify hybrid event name
Message-ID: <YE/n6euYqv7XBFht@krava>
References: <20210311070742.9318-1-yao.jin@linux.intel.com>
 <20210311070742.9318-9-yao.jin@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210311070742.9318-9-yao.jin@linux.intel.com>
Precedence: bulk

On Thu, Mar 11, 2021 at 03:07:23PM +0800, Jin Yao wrote:
> It would be useful to tell user the pmu which the event belongs to.
> perf-stat has supported '--no-merge' option and it can print the pmu
> name after the event name, such as:
> 
> "cycles [cpu_core]"
> 
> Now this option is enabled by default for hybrid platform but change
> the format to:
> 
> "cpu_core/cycles/"
> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>  tools/perf/builtin-stat.c      |  3 +++
>  tools/perf/util/stat-display.c | 12 ++++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 68ecf68699a9..6c0a21323814 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2390,6 +2390,9 @@ int cmd_stat(int argc, const char **argv)
>  
>  	evlist__check_cpu_maps(evsel_list);
>  
> +	if (perf_pmu__hybrid_exist())
> +		stat_config.no_merge = true;
> +
>  	/*
>  	 * Initialize thread_map with comm names,
>  	 * so we could print it out on output.
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 7f09cdaf5b60..ed37d8e7ea1a 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -526,6 +526,7 @@ static void uniquify_event_name(struct evsel *counter)
>  {
>  	char *new_name;
>  	char *config;
> +	int ret;
>  
>  	if (counter->uniquified_name ||
>  	    !counter->pmu_name || !strncmp(counter->name, counter->pmu_name,
> @@ -540,8 +541,15 @@ static void uniquify_event_name(struct evsel *counter)
>  			counter->name = new_name;
>  		}
>  	} else {
> -		if (asprintf(&new_name,
> -			     "%s [%s]", counter->name, counter->pmu_name) > 0) {
> +		if (perf_pmu__hybrid_exist()) {

I'm still not sure about the whole thing, but should you
check in here jut for count->pmu_name instead for hybrid
globaly?

jirka

> +			ret = asprintf(&new_name, "%s/%s/",
> +				       counter->pmu_name, counter->name);
> +		} else {
> +			ret = asprintf(&new_name, "%s [%s]",
> +				       counter->name, counter->pmu_name);
> +		}
> +
> +		if (ret) {
>  			free(counter->name);
>  			counter->name = new_name;
>  		}
> -- 
> 2.17.1
>