Skip to content

Commit 75be6f7

Browse files
Kan LiangIngo Molnar
authored andcommitted
perf/x86/uncore: Fix event group support
The events in the same group don't start or stop simultaneously. Here is the ftrace when enabling event group for uncore_iio_0: # perf stat -e "{uncore_iio_0/event=0x1/,uncore_iio_0/event=0xe/}" <idle>-0 [000] d.h. 8959.064832: read_msr: a41, value b2b0b030 //Read counter reg of IIO unit0 counter0 <idle>-0 [000] d.h. 8959.064835: write_msr: a48, value 400001 //Write Ctrl reg of IIO unit0 counter0 to enable counter0. <------ Although counter0 is enabled, Unit Ctrl is still freezed. Nothing will count. We are still good here. <idle>-0 [000] d.h. 8959.064836: read_msr: a40, value 30100 //Read Unit Ctrl reg of IIO unit0 <idle>-0 [000] d.h. 8959.064838: write_msr: a40, value 30000 //Write Unit Ctrl reg of IIO unit0 to enable all counters in the unit by clear Freeze bit <------Unit0 is un-freezed. Counter0 has been enabled. Now it starts counting. But counter1 has not been enabled yet. The issue starts here. <idle>-0 [000] d.h. 8959.064846: read_msr: a42, value 0 //Read counter reg of IIO unit0 counter1 <idle>-0 [000] d.h. 8959.064847: write_msr: a49, value 40000e //Write Ctrl reg of IIO unit0 counter1 to enable counter1. <------ Now, counter1 just starts to count. Counter0 has been running for a while. Current code un-freezes the Unit Ctrl right after the first counter is enabled. The subsequent group events always loses some counter values. Implement pmu_enable and pmu_disable support for uncore, which can help to batch hardware accesses. No one uses uncore_enable_box and uncore_disable_box. Remove them. Signed-off-by: Kan Liang <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Arnaldo Carvalho de Melo <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Stephane Eranian <[email protected]> Cc: Thomas Gleixner <[email protected]> Cc: Vince Weaver <[email protected]> Cc: [email protected] Cc: [email protected] Fixes: 087bfbb ("perf/x86: Add generic Intel uncore PMU support") Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Ingo Molnar <[email protected]>
1 parent e431e79 commit 75be6f7

File tree

2 files changed

+38
-18
lines changed

2 files changed

+38
-18
lines changed

arch/x86/events/intel/uncore.c

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -502,10 +502,8 @@ void uncore_pmu_event_start(struct perf_event *event, int flags)
502502
local64_set(&event->hw.prev_count, uncore_read_counter(box, event));
503503
uncore_enable_event(box, event);
504504

505-
if (box->n_active == 1) {
506-
uncore_enable_box(box);
505+
if (box->n_active == 1)
507506
uncore_pmu_start_hrtimer(box);
508-
}
509507
}
510508

511509
void uncore_pmu_event_stop(struct perf_event *event, int flags)
@@ -529,10 +527,8 @@ void uncore_pmu_event_stop(struct perf_event *event, int flags)
529527
WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
530528
hwc->state |= PERF_HES_STOPPED;
531529

532-
if (box->n_active == 0) {
533-
uncore_disable_box(box);
530+
if (box->n_active == 0)
534531
uncore_pmu_cancel_hrtimer(box);
535-
}
536532
}
537533

538534
if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
@@ -778,6 +774,40 @@ static int uncore_pmu_event_init(struct perf_event *event)
778774
return ret;
779775
}
780776

777+
static void uncore_pmu_enable(struct pmu *pmu)
778+
{
779+
struct intel_uncore_pmu *uncore_pmu;
780+
struct intel_uncore_box *box;
781+
782+
uncore_pmu = container_of(pmu, struct intel_uncore_pmu, pmu);
783+
if (!uncore_pmu)
784+
return;
785+
786+
box = uncore_pmu_to_box(uncore_pmu, smp_processor_id());
787+
if (!box)
788+
return;
789+
790+
if (uncore_pmu->type->ops->enable_box)
791+
uncore_pmu->type->ops->enable_box(box);
792+
}
793+
794+
static void uncore_pmu_disable(struct pmu *pmu)
795+
{
796+
struct intel_uncore_pmu *uncore_pmu;
797+
struct intel_uncore_box *box;
798+
799+
uncore_pmu = container_of(pmu, struct intel_uncore_pmu, pmu);
800+
if (!uncore_pmu)
801+
return;
802+
803+
box = uncore_pmu_to_box(uncore_pmu, smp_processor_id());
804+
if (!box)
805+
return;
806+
807+
if (uncore_pmu->type->ops->disable_box)
808+
uncore_pmu->type->ops->disable_box(box);
809+
}
810+
781811
static ssize_t uncore_get_attr_cpumask(struct device *dev,
782812
struct device_attribute *attr, char *buf)
783813
{
@@ -803,6 +833,8 @@ static int uncore_pmu_register(struct intel_uncore_pmu *pmu)
803833
pmu->pmu = (struct pmu) {
804834
.attr_groups = pmu->type->attr_groups,
805835
.task_ctx_nr = perf_invalid_context,
836+
.pmu_enable = uncore_pmu_enable,
837+
.pmu_disable = uncore_pmu_disable,
806838
.event_init = uncore_pmu_event_init,
807839
.add = uncore_pmu_event_add,
808840
.del = uncore_pmu_event_del,

arch/x86/events/intel/uncore.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -441,18 +441,6 @@ static inline int uncore_freerunning_hw_config(struct intel_uncore_box *box,
441441
return -EINVAL;
442442
}
443443

444-
static inline void uncore_disable_box(struct intel_uncore_box *box)
445-
{
446-
if (box->pmu->type->ops->disable_box)
447-
box->pmu->type->ops->disable_box(box);
448-
}
449-
450-
static inline void uncore_enable_box(struct intel_uncore_box *box)
451-
{
452-
if (box->pmu->type->ops->enable_box)
453-
box->pmu->type->ops->enable_box(box);
454-
}
455-
456444
static inline void uncore_disable_event(struct intel_uncore_box *box,
457445
struct perf_event *event)
458446
{

0 commit comments

Comments
 (0)