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

["Liang, Kan" <kan.liang@linux.intel.com>]

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