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 3:56 p.m., Peter Zijlstra wrote:
> On Wed, Apr 16, 2025 at 03:45:24PM -0400, Liang, Kan wrote:
> 
>> 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.)
> 
> I'll try and digest in more detail tomorrow, but having overlap it *not*
> a good thing. Which is why I've always asked to make sure this
> doesn't happen :/
>

I've checked all the existing event_constraints[] tables and features,
e.g., auto counter reload.
On the Lion Cove core, the MEM_TRANS_RETIRED.LOAD_LATENCY_GT event has a
constraint mask of 0x3fc. The counter mask for the auto counter reload
feature is 0xfc. On the Golden Cove, the
MEM_TRANS_RETIRED.LOAD_LATENCY_GT event has a constraint mask of 0xfe.

Other constraints (except the one with weight 1) are 0x3, 0xf, 0xff, and
0x3ff.

But I don't think it can trigger the issue which mentioned in the commit
bc1738f6ee83 ("perf, x86: Fix event scheduler for constraints with
overlapping counters"). Because the scheduler always start from 0. The
non-overlapping bits are always scheduled first.
For example, 0xf and 0x3fc. The events with 0xf (has low weights) must
be scheduled first, which occupy the non-overlapping counter 0 and
counter 1. There is no scheduling problem for the events with 0x3fc then.

I think we are good for the static constraints of the existing platforms.

> At the very least we should WARN if we find the dynamic constraint gets
> us there.
> 
So the problem is only with the dynamic constraint.

It looks like only checking the weight and subset is not enough. (It may
trigger false positive for the 0xf and 0x3fc case.)
I think the last bit of the mask should be taken into account as well.

WEIGHT(A) <= WEIGHT(B) &&
A | B != B &&
LAST_BIT(A) > LAST_BIT(B)

Thanks,Kan