[PATCH 1/3] perf stat: Check return value of asprintf() properly

[PATCH 1/3] perf stat: Check return value of asprintf() properly

[Namhyung Kim]

It returns -1 on error, so checking with 0 would not work.

Fixes: 12279429d862 ("perf stat: Uniquify hybrid event name")
Cc: Jin Yao <yao.jin@linux.intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/stat-display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index b759dfd633b4..04afd41b6067 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -564,7 +564,7 @@ static void uniquify_event_name(struct evsel *counter)
 				       counter->name, counter->pmu_name);
 		}
 
-		if (ret) {
+		if (ret > 0) {
 			free(counter->name);
 			counter->name = new_name;
 		}
-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH 2/3] perf tools: Clone evsel->use_config_name

[Namhyung Kim]

The evsel__clone() should copy all fields in the evsel which are set
during the event parsing.  But it missed the use_config_name field.

Fixes: 12279429d862 ("perf stat: Uniquify hybrid event name")
Cc: Jin Yao <yao.jin@linux.intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/evsel.c | 1 +
 tools/perf/util/evsel.h | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 4a3cd1b5bb33..a8d8463f8ee5 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -428,6 +428,7 @@ struct evsel *evsel__clone(struct evsel *orig)
 	evsel->auto_merge_stats = orig->auto_merge_stats;
 	evsel->collect_stat = orig->collect_stat;
 	evsel->weak_group = orig->weak_group;
+	evsel->use_config_name = orig->use_config_name;
 
 	if (evsel__copy_config_terms(evsel, orig) < 0)
 		goto out_err;
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 75cf5dbfe208..bdad52a06438 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -83,8 +83,10 @@ struct evsel {
 		bool			collect_stat;
 		bool			weak_group;
 		bool			bpf_counter;
+		bool			use_config_name;
 		int			bpf_fd;
 		struct bpf_object	*bpf_obj;
+		struct list_head	config_terms;
 	};
 
 	/*
@@ -116,10 +118,8 @@ struct evsel {
 	bool			merged_stat;
 	bool			reset_group;
 	bool			errored;
-	bool			use_config_name;
 	struct hashmap		*per_pkg_mask;
 	struct evsel		*leader;
-	struct list_head	config_terms;
 	int			err;
 	int			cpu_iter;
 	struct {
-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH 3/3] perf stat: Honor event config name on --no-merge

[Namhyung Kim]

If user gave an event name explicitly, it should be displayed in the
output as is.  But with --no-merge option it adds a pmu name at the
end so might confuse users.

Actually this is true for hybrid pmus, I think we should do the same
for others.

Cc: Jin Yao <yao.jin@linux.intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/stat-display.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 04afd41b6067..f7cb9bba673b 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -541,7 +541,7 @@ static void uniquify_event_name(struct evsel *counter)
 	char *config;
 	int ret = 0;
 
-	if (counter->uniquified_name ||
+	if (counter->uniquified_name || counter->use_config_name ||
 	    !counter->pmu_name || !strncmp(counter->name, counter->pmu_name,
 					   strlen(counter->pmu_name)))
 		return;
@@ -555,10 +555,8 @@ static void uniquify_event_name(struct evsel *counter)
 		}
 	} else {
 		if (perf_pmu__has_hybrid()) {
-			if (!counter->use_config_name) {
-				ret = asprintf(&new_name, "%s/%s/",
-					       counter->pmu_name, counter->name);
-			}
+			ret = asprintf(&new_name, "%s/%s/",
+				       counter->pmu_name, counter->name);
 		} else {
 			ret = asprintf(&new_name, "%s [%s]",
 				       counter->name, counter->pmu_name);
-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

Re: [PATCH 1/3] perf stat: Check return value of asprintf() properly

[Ian Rogers]

On Wed, Jun 2, 2021 at 2:22 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> It returns -1 on error, so checking with 0 would not work.
>
> Fixes: 12279429d862 ("perf stat: Uniquify hybrid event name")
> Cc: Jin Yao <yao.jin@linux.intel.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/stat-display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index b759dfd633b4..04afd41b6067 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -564,7 +564,7 @@ static void uniquify_event_name(struct evsel *counter)
>                                        counter->name, counter->pmu_name);
>                 }
>
> -               if (ret) {
> +               if (ret > 0) {
>                         free(counter->name);
>                         counter->name = new_name;
>                 }
> --
> 2.32.0.rc0.204.g9fa02ecfa5-goog
>

Re: [PATCH 1/3] perf stat: Check return value of asprintf() properly

[Arnaldo Carvalho de Melo]

Em Thu, Jun 03, 2021 at 09:47:58PM -0700, Ian Rogers escreveu:
> On Wed, Jun 2, 2021 at 2:22 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > It returns -1 on error, so checking with 0 would not work.
> >
> > Fixes: 12279429d862 ("perf stat: Uniquify hybrid event name")
> > Cc: Jin Yao <yao.jin@linux.intel.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Acked-by: Ian Rogers <irogers@google.com>

Thanks, applied both.

- Arnaldo

 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/util/stat-display.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index b759dfd633b4..04afd41b6067 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -564,7 +564,7 @@ static void uniquify_event_name(struct evsel *counter)
> >                                        counter->name, counter->pmu_name);
> >                 }
> >
> > -               if (ret) {
> > +               if (ret > 0) {
> >                         free(counter->name);
> >                         counter->name = new_name;
> >                 }
> > --
> > 2.32.0.rc0.204.g9fa02ecfa5-goog
> >

-- 

- Arnaldo