Re: [Patch v3 12/22] perf/x86/intel: Update dyn_constranit base on PEBS event precise level

Liang, Kan
2025-04-16T19:45:27Z

On 2025-04-16 11:32 a.m., Peter Zijlstra wrote:
> On Tue, Apr 15, 2025 at 12:31:03PM -0400, Liang, Kan wrote:
>
>>> This can land us in EVENT_CONSTRAINT_OVERLAP territory, no?
>
>> The dyn_constraint is a supplement of the static constraints. It doesn't
>> overwrite the static constraints.
>
> That doesn't matter.
>
>> In the intel_get_event_constraints(), perf always gets the static
>> constraints first. If the dyn_constraint is defined, it gets the common
>> mask of the static constraints and the dynamic constraints. All
>> constraint rules will be complied.
>>
>> if (event->hw.dyn_constraint != ~0ULL) {
>> c2 = dyn_constraint(cpuc, c2, idx);
>> c2->idxmsk64 &= event->hw.dyn_constraint;
>> c2->weight = hweight64(c2->idxmsk64);
>> }
>
> Read the comment that goes with EVENT_CONSTRAINT_OVERLAP().
>
> Suppose we have (from intel_lnc_event_constraints[]):
>
> INTEL_UEVENT_CONSTRAINT(0x012a, 0xf)
> INTEL_EVENT_CONSTRAINT(0x2e, 0x3ff)
>
> Then since the first is fully contained in the latter, there is no
> problem.
>
> Now imagine PEBS gets a dynamic constraint of 0x3c (just because), and
> then you try and create a PEBS event along with the above two events,
> and all of a sudden you have:
>
> 0x000f
> 0x003c
> 0x03ff
>
> And that is exactly the problem case.
>
> Also, looking at that LNC table, please explain:
>
> INTEL_UEVENT_CONSTRAINT(0x01cd, 0x3fc)
>
> that looks like the exact thing I've asked to never do, exactly because
> of the above problem :-(

I see. I think we can check the constraint table and update the overlap
bit accordingly. Similar to what we did in the
intel_pmu_check_event_constraints() for the fixed counters.

I'm thinking something as below (Just a POC, not tested.)

For the static table, set the overlap for the events that may trigger
the issue at init time.
For the dynamic constraint, add a dyn_overlap_mask to track if overlap
is required for the feature (The below only supports the branch
counters. The ACR and ARCH PEBS can be added later.) If it's required,
set a flag PERF_X86_EVENT_OVERLAP for the event when the dyn_constraint
is applied. The overlap bit will be set at runtime.

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 16f8aea33243..76a03a0c28e9 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3825,6 +3825,8 @@ intel_get_event_constraints(struct cpu_hw_events
*cpuc, int idx,
c2 = dyn_constraint(cpuc, c2, idx);
c2->idxmsk64 &= event->hw.dyn_constraint;
c2->weight = hweight64(c2->idxmsk64);+ if (event->hw.flags &
PERF_X86_EVENT_OVERLAP)
+ c2->overlap = 1;
}

return c2;
@@ -4197,6 +4199,12 @@ static inline void
intel_pmu_set_acr_caused_constr(struct perf_event *event,
event->hw.dyn_constraint &= hybrid(event->pmu, acr_cause_mask64);
}

+enum dyn_overlap_bits {
+ DYN_OVERLAP_BRANCH_CNTR
+};
+
+static unsigned long dyn_overlap_mask;
+
static int intel_pmu_hw_config(struct perf_event *event)
{
int ret = x86_pmu_hw_config(event);
@@ -4261,6 +4269,8 @@ static int intel_pmu_hw_config(struct perf_event
*event)
if (branch_sample_counters(leader)) {
num++;
leader->hw.dyn_constraint &= x86_pmu.lbr_counters;
+ if (test_bit(DYN_OVERLAP_BRANCH_CNTR, &dyn_overlap_mask);
+ leader->hw.flags |= PERF_X86_EVENT_OVERLAP;
}
leader->hw.flags |= PERF_X86_EVENT_BRANCH_COUNTERS;

@@ -4270,6 +4280,8 @@ static int intel_pmu_hw_config(struct perf_event
*event)
if (branch_sample_counters(sibling)) {
num++;
sibling->hw.dyn_constraint &= x86_pmu.lbr_counters;
+ if (test_bit(DYN_OVERLAP_BRANCH_CNTR, &dyn_overlap_mask);
+ sibling->hw.flags |= PERF_X86_EVENT_OVERLAP;
}
}

@@ -6638,6 +6650,29 @@ static void
intel_pmu_check_event_constraints(struct event_constraint *event_con
if (!event_constraints)
return;

+ for_each_event_constraint(c, event_constraints) {
+ if (c->weight == 1 || c->overlap)
+ continue;
+
+ /*
+ * The counter mask of an event is not a subset of
+ * the counter mask of a constraint with an equal
+ * or higher weight. The overlap flag must be set.
+ */
+ for_each_event_constraint(c2, event_constraints) {
+ if ((c2->weight >= c->weight) &&
+ (c2->idxmsk64 | c->idxmsk64) != c2->idxmsk64) {
+ c->overlap = 1;
+ break;
+ }
+ }
+
+ /* Check for the dynamic constraint */
+ if (c->weight >= HWEIGHT(x86_pmu.lbr_counters) &&
+ (c->idxmsk64 | x86_pmu.lbr_counters) != c->idxmsk64)
+ __set_bit(DYN_OVERLAP_BRANCH_CNTR, &dyn_overlap_mask);
+ }
+
/*
* event on fixed counter2 (REF_CYCLES) only works on this
* counter, so do not extend mask to generic counters

Thanks,
Kan