Re: [PATCH 5/5] perf stat: Fix duplicate PMU name for interval print

From: Liang, Kan
Date: Tue Apr 24 2018 - 15:18:46 EST




On 4/24/2018 2:53 PM, Arnaldo Carvalho de Melo wrote:
Em Tue, Apr 24, 2018 at 11:20:14AM -0700, kan.liang@xxxxxxxxxxxxxxx escreveu:
From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>

PMU name is printed repeatedly for interval print, for example:

perf stat --no-merge -e 'unc_m_clockticks' -a -I 1000
# time counts unit events
1.001053069 243,702,144 unc_m_clockticks [uncore_imc_4]
1.001053069 244,268,304 unc_m_clockticks [uncore_imc_2]
1.001053069 244,427,386 unc_m_clockticks [uncore_imc_0]
1.001053069 244,583,760 unc_m_clockticks [uncore_imc_5]
1.001053069 244,738,971 unc_m_clockticks [uncore_imc_3]
1.001053069 244,880,309 unc_m_clockticks [uncore_imc_1]
2.002024821 240,818,200 unc_m_clockticks [uncore_imc_4] [uncore_imc_4]
2.002024821 240,767,812 unc_m_clockticks [uncore_imc_2] [uncore_imc_2]
2.002024821 240,764,215 unc_m_clockticks [uncore_imc_0] [uncore_imc_0]
2.002024821 240,759,504 unc_m_clockticks [uncore_imc_5] [uncore_imc_5]
2.002024821 240,755,992 unc_m_clockticks [uncore_imc_3] [uncore_imc_3]
2.002024821 240,750,403 unc_m_clockticks [uncore_imc_1] [uncore_imc_1]

For each print, the PMU name is unconditionally appended to the
counter->name.
Need to check the counter->name first. If the PMU name is already
appended, do nothing.

Fixes: 8c5421c016a4 ("perf pmu: Display pmu name when printing unmerged events in stat")
+++ b/tools/perf/builtin-stat.c
@@ -1296,6 +1296,8 @@ static void uniquify_event_name(struct perf_evsel *counter)
counter->name = new_name;
}
} else {
+ if (strstr(counter->name, counter->pmu_name))
+ return;
if (asprintf(&new_name,
"%s [%s]", counter->name, counter->pmu_name) > 0) {
free(counter->name);

Humm, do you have any problem with the patch below instead?

No. The patch as below looks good to me.

Thanks,
Kan


diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 2137c7d11767..8518342c5466 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1261,7 +1261,8 @@ static void uniquify_event_name(struct perf_evsel *counter)
char *new_name;
char *config;
- if (!counter->pmu_name || !strncmp(counter->name, counter->pmu_name,
+ if (counter->uniquified_name ||
+ !counter->pmu_name || !strncmp(counter->name, counter->pmu_name,
strlen(counter->pmu_name)))
return;
@@ -1279,6 +1280,8 @@ static void uniquify_event_name(struct perf_evsel *counter)
counter->name = new_name;
}
}
+
+ counter->uniquified_name = true;
}
static void collect_all_aliases(struct perf_evsel *counter,
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index d3ee3af618ef..92ec009a292d 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -115,6 +115,7 @@ struct perf_evsel {
unsigned int sample_size;
int id_pos;
int is_pos;
+ bool uniquified_name;
bool snapshot;
bool supported;
bool needs_swap;