Skip to content

Commit acade63

Browse files
Kan LiangPeter Zijlstra
authored andcommitted
perf/x86/intel: Apply mid ACK for small core
A warning as below may be occasionally triggered in an ADL machine when these conditions occur: - Two perf record commands run one by one. Both record a PEBS event. - Both runs on small cores. - They have different adaptive PEBS configuration (PEBS_DATA_CFG). [ ] WARNING: CPU: 4 PID: 9874 at arch/x86/events/intel/ds.c:1743 setup_pebs_adaptive_sample_data+0x55e/0x5b0 [ ] RIP: 0010:setup_pebs_adaptive_sample_data+0x55e/0x5b0 [ ] Call Trace: [ ] <NMI> [ ] intel_pmu_drain_pebs_icl+0x48b/0x810 [ ] perf_event_nmi_handler+0x41/0x80 [ ] </NMI> [ ] __perf_event_task_sched_in+0x2c2/0x3a0 Different from the big core, the small core requires the ACK right before re-enabling counters in the NMI handler, otherwise a stale PEBS record may be dumped into the later NMI handler, which trigger the warning. Add a new mid_ack flag to track the case. Add all PMI handler bits in the struct x86_hybrid_pmu to track the bits for different types of PMUs. Apply mid ACK for the small cores on an Alder Lake machine. The existing hybrid() macro has a compile error when taking address of a bit-field variable. Add a new macro hybrid_bit() to get the bit-field value of a given PMU. Fixes: f83d2f9 ("perf/x86/intel: Add Alder Lake Hybrid support") Reported-by: Ammy Yi <[email protected]> Signed-off-by: Kan Liang <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Reviewed-by: Andi Kleen <[email protected]> Tested-by: Ammy Yi <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent df51fe7 commit acade63

File tree

2 files changed

+30
-8
lines changed

2 files changed

+30
-8
lines changed

arch/x86/events/intel/core.c

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2904,24 +2904,28 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
29042904
*/
29052905
static int intel_pmu_handle_irq(struct pt_regs *regs)
29062906
{
2907-
struct cpu_hw_events *cpuc;
2907+
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
2908+
bool late_ack = hybrid_bit(cpuc->pmu, late_ack);
2909+
bool mid_ack = hybrid_bit(cpuc->pmu, mid_ack);
29082910
int loops;
29092911
u64 status;
29102912
int handled;
29112913
int pmu_enabled;
29122914

2913-
cpuc = this_cpu_ptr(&cpu_hw_events);
2914-
29152915
/*
29162916
* Save the PMU state.
29172917
* It needs to be restored when leaving the handler.
29182918
*/
29192919
pmu_enabled = cpuc->enabled;
29202920
/*
2921-
* No known reason to not always do late ACK,
2922-
* but just in case do it opt-in.
2921+
* In general, the early ACK is only applied for old platforms.
2922+
* For the big core starts from Haswell, the late ACK should be
2923+
* applied.
2924+
* For the small core after Tremont, we have to do the ACK right
2925+
* before re-enabling counters, which is in the middle of the
2926+
* NMI handler.
29232927
*/
2924-
if (!x86_pmu.late_ack)
2928+
if (!late_ack && !mid_ack)
29252929
apic_write(APIC_LVTPC, APIC_DM_NMI);
29262930
intel_bts_disable_local();
29272931
cpuc->enabled = 0;
@@ -2958,6 +2962,8 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
29582962
goto again;
29592963

29602964
done:
2965+
if (mid_ack)
2966+
apic_write(APIC_LVTPC, APIC_DM_NMI);
29612967
/* Only restore PMU state when it's active. See x86_pmu_disable(). */
29622968
cpuc->enabled = pmu_enabled;
29632969
if (pmu_enabled)
@@ -2969,7 +2975,7 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
29692975
* have been reset. This avoids spurious NMIs on
29702976
* Haswell CPUs.
29712977
*/
2972-
if (x86_pmu.late_ack)
2978+
if (late_ack)
29732979
apic_write(APIC_LVTPC, APIC_DM_NMI);
29742980
return handled;
29752981
}
@@ -6129,7 +6135,6 @@ __init int intel_pmu_init(void)
61296135
static_branch_enable(&perf_is_hybrid);
61306136
x86_pmu.num_hybrid_pmus = X86_HYBRID_NUM_PMUS;
61316137

6132-
x86_pmu.late_ack = true;
61336138
x86_pmu.pebs_aliases = NULL;
61346139
x86_pmu.pebs_prec_dist = true;
61356140
x86_pmu.pebs_block = true;
@@ -6167,6 +6172,7 @@ __init int intel_pmu_init(void)
61676172
pmu = &x86_pmu.hybrid_pmu[X86_HYBRID_PMU_CORE_IDX];
61686173
pmu->name = "cpu_core";
61696174
pmu->cpu_type = hybrid_big;
6175+
pmu->late_ack = true;
61706176
if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU)) {
61716177
pmu->num_counters = x86_pmu.num_counters + 2;
61726178
pmu->num_counters_fixed = x86_pmu.num_counters_fixed + 1;
@@ -6192,6 +6198,7 @@ __init int intel_pmu_init(void)
61926198
pmu = &x86_pmu.hybrid_pmu[X86_HYBRID_PMU_ATOM_IDX];
61936199
pmu->name = "cpu_atom";
61946200
pmu->cpu_type = hybrid_small;
6201+
pmu->mid_ack = true;
61956202
pmu->num_counters = x86_pmu.num_counters;
61966203
pmu->num_counters_fixed = x86_pmu.num_counters_fixed;
61976204
pmu->max_pebs_events = x86_pmu.max_pebs_events;

arch/x86/events/perf_event.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,10 @@ struct x86_hybrid_pmu {
656656
struct event_constraint *event_constraints;
657657
struct event_constraint *pebs_constraints;
658658
struct extra_reg *extra_regs;
659+
660+
unsigned int late_ack :1,
661+
mid_ack :1,
662+
enabled_ack :1;
659663
};
660664

661665
static __always_inline struct x86_hybrid_pmu *hybrid_pmu(struct pmu *pmu)
@@ -686,6 +690,16 @@ extern struct static_key_false perf_is_hybrid;
686690
__Fp; \
687691
}))
688692

693+
#define hybrid_bit(_pmu, _field) \
694+
({ \
695+
bool __Fp = x86_pmu._field; \
696+
\
697+
if (is_hybrid() && (_pmu)) \
698+
__Fp = hybrid_pmu(_pmu)->_field; \
699+
\
700+
__Fp; \
701+
})
702+
689703
enum hybrid_pmu_type {
690704
hybrid_big = 0x40,
691705
hybrid_small = 0x20,
@@ -755,6 +769,7 @@ struct x86_pmu {
755769

756770
/* PMI handler bits */
757771
unsigned int late_ack :1,
772+
mid_ack :1,
758773
enabled_ack :1;
759774
/*
760775
* sysfs attrs

0 commit comments

Comments
 (0)