Skip to content

Commit

Permalink
perf/x86/intel/uncore: Fix client IMC events return huge result
Browse files Browse the repository at this point in the history
The client IMC bandwidth events currently return very large values:

  $ perf stat -e uncore_imc/data_reads/ -e uncore_imc/data_writes/ -I 10000 -a

  10.000117222 34,788.76 MiB uncore_imc/data_reads/
  10.000117222 8.26 MiB uncore_imc/data_writes/
  20.000374584 34,842.89 MiB uncore_imc/data_reads/
  20.000374584 10.45 MiB uncore_imc/data_writes/
  30.000633299 37,965.29 MiB uncore_imc/data_reads/
  30.000633299 323.62 MiB uncore_imc/data_writes/
  40.000891548 41,012.88 MiB uncore_imc/data_reads/
  40.000891548 6.98 MiB uncore_imc/data_writes/
  50.001142480 1,125,899,906,621,494.75 MiB uncore_imc/data_reads/
  50.001142480 6.97 MiB uncore_imc/data_writes/

The client IMC events are freerunning counters. They still use the
old event encoding format (0x1 for data_read and 0x2 for data write).
The counter bit width is calculated by common code, which assume that
the standard encoding format is used for the freerunning counters.
Error bit width information is calculated.

The patch intends to convert the old client IMC event encoding to the
standard encoding format.

Current common code uses event->attr.config which directly copy from
user space. We should not implicitly modify it for a converted event.
The event->hw.config is used to replace the event->attr.config in
common code.

For client IMC events, the event->attr.config is used to calculate a
converted event with standard encoding format in the custom
event_init(). The converted event is stored in event->hw.config.
For other events of freerunning counters, they already use the standard
encoding format. The same value as event->attr.config is assigned to
event->hw.config in common event_init().

Reported-by: Jin Yao <[email protected]>
Tested-by: Jin Yao <[email protected]>
Signed-off-by: Kan Liang <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Rik van Riel <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vince Weaver <[email protected]>
Cc: [email protected] # v4.18+
Fixes: 9aae178 ("perf/x86/intel/uncore: Clean up client IMC uncore")
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
  • Loading branch information
Kan Liang authored and Ingo Molnar committed Mar 9, 2019
1 parent 5768402 commit 8041ffd
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 7 deletions.
1 change: 1 addition & 0 deletions arch/x86/events/intel/uncore.c
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ static int uncore_pmu_event_init(struct perf_event *event)
/* fixed counters have event field hardcoded to zero */
hwc->config = 0ULL;
} else if (is_freerunning_event(event)) {
hwc->config = event->attr.config;
if (!check_valid_freerunning_event(box, event))
return -EINVAL;
event->hw.idx = UNCORE_PMC_IDX_FREERUNNING;
Expand Down
12 changes: 6 additions & 6 deletions arch/x86/events/intel/uncore.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ static inline
unsigned int uncore_freerunning_counter(struct intel_uncore_box *box,
struct perf_event *event)
{
unsigned int type = uncore_freerunning_type(event->attr.config);
unsigned int idx = uncore_freerunning_idx(event->attr.config);
unsigned int type = uncore_freerunning_type(event->hw.config);
unsigned int idx = uncore_freerunning_idx(event->hw.config);
struct intel_uncore_pmu *pmu = box->pmu;

return pmu->type->freerunning[type].counter_base +
Expand Down Expand Up @@ -377,15 +377,15 @@ static inline
unsigned int uncore_freerunning_bits(struct intel_uncore_box *box,
struct perf_event *event)
{
unsigned int type = uncore_freerunning_type(event->attr.config);
unsigned int type = uncore_freerunning_type(event->hw.config);

return box->pmu->type->freerunning[type].bits;
}

static inline int uncore_num_freerunning(struct intel_uncore_box *box,
struct perf_event *event)
{
unsigned int type = uncore_freerunning_type(event->attr.config);
unsigned int type = uncore_freerunning_type(event->hw.config);

return box->pmu->type->freerunning[type].num_counters;
}
Expand All @@ -399,8 +399,8 @@ static inline int uncore_num_freerunning_types(struct intel_uncore_box *box,
static inline bool check_valid_freerunning_event(struct intel_uncore_box *box,
struct perf_event *event)
{
unsigned int type = uncore_freerunning_type(event->attr.config);
unsigned int idx = uncore_freerunning_idx(event->attr.config);
unsigned int type = uncore_freerunning_type(event->hw.config);
unsigned int idx = uncore_freerunning_idx(event->hw.config);

return (type < uncore_num_freerunning_types(box, event)) &&
(idx < uncore_num_freerunning(box, event));
Expand Down
4 changes: 3 additions & 1 deletion arch/x86/events/intel/uncore_snb.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,11 @@ static int snb_uncore_imc_event_init(struct perf_event *event)

/* must be done before validate_group */
event->hw.event_base = base;
event->hw.config = cfg;
event->hw.idx = idx;

/* Convert to standard encoding format for freerunning counters */
event->hw.config = ((cfg - 1) << 8) | 0x10ff;

/* no group validation needed, we have free running counters */

return 0;
Expand Down

0 comments on commit 8041ffd

Please sign in to comment.