Skip to content

Commit 034f7ee

Browse files
Jin Yaoacmel
Jin Yao
authored and
acmel
committed
perf stat: Fix wrong skipping for per-die aggregation
Uncore becomes die-scope on Xeon Cascade Lake-AP and perf has supported --per-die aggregation yet. One issue is found in check_per_pkg() for uncore events running on AP system. On cascade Lake-AP, we have: S0-D0 S0-D1 S1-D0 S1-D1 But in check_per_pkg(), S0-D1 and S1-D1 are skipped because the mask bits for S0 and S1 have been set for S0-D0 and S1-D0. It doesn't check die_id. So the counting for S0-D1 and S1-D1 are set to zero. That's not correct. root@lkp-csl-2ap4 ~# ./perf stat -a -I 1000 -e llc_misses.mem_read --per-die -- sleep 5 1.001460963 S0-D0 1 1317376 Bytes llc_misses.mem_read 1.001460963 S0-D1 1 998016 Bytes llc_misses.mem_read 1.001460963 S1-D0 1 970496 Bytes llc_misses.mem_read 1.001460963 S1-D1 1 1291264 Bytes llc_misses.mem_read 2.003488021 S0-D0 1 1082048 Bytes llc_misses.mem_read 2.003488021 S0-D1 1 1919040 Bytes llc_misses.mem_read 2.003488021 S1-D0 1 890752 Bytes llc_misses.mem_read 2.003488021 S1-D1 1 2380800 Bytes llc_misses.mem_read 3.005613270 S0-D0 1 1126080 Bytes llc_misses.mem_read 3.005613270 S0-D1 1 2898176 Bytes llc_misses.mem_read 3.005613270 S1-D0 1 870912 Bytes llc_misses.mem_read 3.005613270 S1-D1 1 3388608 Bytes llc_misses.mem_read 4.007627598 S0-D0 1 1124608 Bytes llc_misses.mem_read 4.007627598 S0-D1 1 3884416 Bytes llc_misses.mem_read 4.007627598 S1-D0 1 921088 Bytes llc_misses.mem_read 4.007627598 S1-D1 1 4451840 Bytes llc_misses.mem_read 5.001479927 S0-D0 1 963328 Bytes llc_misses.mem_read 5.001479927 S0-D1 1 4831936 Bytes llc_misses.mem_read 5.001479927 S1-D0 1 895104 Bytes llc_misses.mem_read 5.001479927 S1-D1 1 5496640 Bytes llc_misses.mem_read From above output, we can see S0-D1 and S1-D1 don't report the interval values, they are continued to grow. That's because check_per_pkg() wrongly decides to use zero counts for S0-D1 and S1-D1. So in check_per_pkg(), we should use hashmap(socket,die) to decide if the cpu counts needs to skip. Only considering socket is not enough. Now with this patch, root@lkp-csl-2ap4 ~# ./perf stat -a -I 1000 -e llc_misses.mem_read --per-die -- sleep 5 1.001586691 S0-D0 1 1229440 Bytes llc_misses.mem_read 1.001586691 S0-D1 1 976832 Bytes llc_misses.mem_read 1.001586691 S1-D0 1 938304 Bytes llc_misses.mem_read 1.001586691 S1-D1 1 1227328 Bytes llc_misses.mem_read 2.003776312 S0-D0 1 1586752 Bytes llc_misses.mem_read 2.003776312 S0-D1 1 875392 Bytes llc_misses.mem_read 2.003776312 S1-D0 1 855616 Bytes llc_misses.mem_read 2.003776312 S1-D1 1 949376 Bytes llc_misses.mem_read 3.006512788 S0-D0 1 1338880 Bytes llc_misses.mem_read 3.006512788 S0-D1 1 920064 Bytes llc_misses.mem_read 3.006512788 S1-D0 1 877184 Bytes llc_misses.mem_read 3.006512788 S1-D1 1 1020736 Bytes llc_misses.mem_read 4.008895291 S0-D0 1 926592 Bytes llc_misses.mem_read 4.008895291 S0-D1 1 906368 Bytes llc_misses.mem_read 4.008895291 S1-D0 1 892224 Bytes llc_misses.mem_read 4.008895291 S1-D1 1 987712 Bytes llc_misses.mem_read 5.001590993 S0-D0 1 962624 Bytes llc_misses.mem_read 5.001590993 S0-D1 1 912512 Bytes llc_misses.mem_read 5.001590993 S1-D0 1 891200 Bytes llc_misses.mem_read 5.001590993 S1-D1 1 978432 Bytes llc_misses.mem_read On no-die system, die_id is 0, actually it's hashmap(socket,0), original behavior is not changed. Reported-by: Ying Huang <ying.huang@intel.com> Signed-off-by: Jin Yao <yao.jin@linux.intel.com> Acked-by: Jiri Olsa <jolsa@kernel.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andi Kleen <ak@linux.intel.com> Cc: Jin Yao <yao.jin@intel.com> Cc: Kan Liang <kan.liang@linux.intel.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ying Huang <ying.huang@intel.com> Link: http://lore.kernel.org/lkml/20210128013417.25597-1-yao.jin@linux.intel.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
1 parent 33dc525 commit 034f7ee

4 files changed

Lines changed: 59 additions & 11 deletions

File tree

‎tools/perf/util/evsel.c‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include "string2.h"
4747
#include "memswap.h"
4848
#include "util.h"
49+
#include "hashmap.h"
4950
#include "../perf-sys.h"
5051
#include "util/parse-branch-options.h"
5152
#include <internal/xyarray.h>
@@ -1390,7 +1391,9 @@ void evsel__exit(struct evsel *evsel)
13901391
zfree(&evsel->group_name);
13911392
zfree(&evsel->name);
13921393
zfree(&evsel->pmu_name);
1393-
zfree(&evsel->per_pkg_mask);
1394+
evsel__zero_per_pkg(evsel);
1395+
hashmap__free(evsel->per_pkg_mask);
1396+
evsel->per_pkg_mask = NULL;
13941397
zfree(&evsel->metric_events);
13951398
perf_evsel__object.fini(evsel);
13961399
}
@@ -2781,3 +2784,16 @@ int evsel__store_ids(struct evsel *evsel, struct evlist *evlist)
27812784

27822785
return store_evsel_ids(evsel, evlist);
27832786
}
2787+
2788+
void evsel__zero_per_pkg(struct evsel *evsel)
2789+
{
2790+
struct hashmap_entry *cur;
2791+
size_t bkt;
2792+
2793+
if (evsel->per_pkg_mask) {
2794+
hashmap__for_each_entry(evsel->per_pkg_mask, cur, bkt)
2795+
free((char *)cur->key);
2796+
2797+
hashmap__clear(evsel->per_pkg_mask);
2798+
}
2799+
}

‎tools/perf/util/evsel.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ struct perf_stat_evsel;
1919
union perf_event;
2020
struct bpf_counter_ops;
2121
struct target;
22+
struct hashmap;
2223

2324
typedef int (evsel__sb_cb_t)(union perf_event *event, void *data);
2425

@@ -112,7 +113,7 @@ struct evsel {
112113
bool merged_stat;
113114
bool reset_group;
114115
bool errored;
115-
unsigned long *per_pkg_mask;
116+
struct hashmap *per_pkg_mask;
116117
struct evsel *leader;
117118
struct list_head config_terms;
118119
int err;
@@ -433,4 +434,5 @@ struct perf_env *evsel__env(struct evsel *evsel);
433434

434435
int evsel__store_ids(struct evsel *evsel, struct evlist *evlist);
435436

437+
void evsel__zero_per_pkg(struct evsel *evsel);
436438
#endif /* __PERF_EVSEL_H */

‎tools/perf/util/python-ext-sources‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,4 @@ util/symbol_fprintf.c
3636
util/units.c
3737
util/affinity.c
3838
util/rwsem.c
39+
util/hashmap.c

‎tools/perf/util/stat.c‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "evlist.h"
1414
#include "evsel.h"
1515
#include "thread_map.h"
16+
#include "hashmap.h"
1617
#include <linux/zalloc.h>
1718

1819
void update_stats(struct stats *stats, u64 val)
@@ -277,18 +278,29 @@ void evlist__save_aggr_prev_raw_counts(struct evlist *evlist)
277278
}
278279
}
279280

280-
static void zero_per_pkg(struct evsel *counter)
281+
static size_t pkg_id_hash(const void *__key, void *ctx __maybe_unused)
281282
{
282-
if (counter->per_pkg_mask)
283-
memset(counter->per_pkg_mask, 0, cpu__max_cpu());
283+
uint64_t *key = (uint64_t *) __key;
284+
285+
return *key & 0xffffffff;
286+
}
287+
288+
static bool pkg_id_equal(const void *__key1, const void *__key2,
289+
void *ctx __maybe_unused)
290+
{
291+
uint64_t *key1 = (uint64_t *) __key1;
292+
uint64_t *key2 = (uint64_t *) __key2;
293+
294+
return *key1 == *key2;
284295
}
285296

286297
static int check_per_pkg(struct evsel *counter,
287298
struct perf_counts_values *vals, int cpu, bool *skip)
288299
{
289-
unsigned long *mask = counter->per_pkg_mask;
300+
struct hashmap *mask = counter->per_pkg_mask;
290301
struct perf_cpu_map *cpus = evsel__cpus(counter);
291-
int s;
302+
int s, d, ret = 0;
303+
uint64_t *key;
292304

293305
*skip = false;
294306

@@ -299,7 +311,7 @@ static int check_per_pkg(struct evsel *counter,
299311
return 0;
300312

301313
if (!mask) {
302-
mask = zalloc(cpu__max_cpu());
314+
mask = hashmap__new(pkg_id_hash, pkg_id_equal, NULL);
303315
if (!mask)
304316
return -ENOMEM;
305317

@@ -321,8 +333,25 @@ static int check_per_pkg(struct evsel *counter,
321333
if (s < 0)
322334
return -1;
323335

324-
*skip = test_and_set_bit(s, mask) == 1;
325-
return 0;
336+
/*
337+
* On multi-die system, die_id > 0. On no-die system, die_id = 0.
338+
* We use hashmap(socket, die) to check the used socket+die pair.
339+
*/
340+
d = cpu_map__get_die(cpus, cpu, NULL).die;
341+
if (d < 0)
342+
return -1;
343+
344+
key = malloc(sizeof(*key));
345+
if (!key)
346+
return -ENOMEM;
347+
348+
*key = (uint64_t)d << 32 | s;
349+
if (hashmap__find(mask, (void *)key, NULL))
350+
*skip = true;
351+
else
352+
ret = hashmap__add(mask, (void *)key, (void *)1);
353+
354+
return ret;
326355
}
327356

328357
static int
@@ -422,7 +451,7 @@ int perf_stat_process_counter(struct perf_stat_config *config,
422451
}
423452

424453
if (counter->per_pkg)
425-
zero_per_pkg(counter);
454+
evsel__zero_per_pkg(counter);
426455

427456
ret = process_counter_maps(config, counter);
428457
if (ret)

0 commit comments

Comments
 (0)