On Thu, Mar 07, 2024, Dapeng Mi wrote:
> On 3/7/2024 7:01 AM, Sean Christopherson wrote:
> > @@ -293,12 +293,9 @@ static void check_pebs_records(u64 bitmask, u64 pebs_data_cfg)
> > do {
> > pebs_rec = (struct pebs_basic *)cur_record;
> > pebs_record_size = pebs_rec->format_size >> RECORD_SIZE_OFFSET;
> > - pebs_idx_match =
> > - pebs_rec->applicable_counters & bitmask;
> > - pebs_size_match =
> > - pebs_record_size == get_adaptive_pebs_record_size(pebs_data_cfg);
> > - data_cfg_match =
> > - (pebs_rec->format_size & GENMASK_ULL(47, 0)) == pebs_data_cfg;
> > + pebs_idx_match = pebs_rec->applicable_counters & bitmask;
> > + pebs_size_match = pebs_record_size == get_pebs_record_size(pebs_data_cfg, use_adaptive);
> > + data_cfg_match = (pebs_rec->format_size & GENMASK_ULL(47, 0)) == pebs_data_cfg;
>
> Since there is already a macro RECORD_SIZE_OFFSET, we'd better use
> "RECORD_SIZE_OFFSET - 1" to replace the magic number 47.
Very belatedly, I disagree. That the data configuration mask isn't derived from
record size. The fact that the record size is the _only_ info that's excluded
is coincidental (sort of, that's not quite the right word), i.e. if we want to
use a #define, then we should add an explicit define, not abuse RECORD_SIZE_OFFSET.
