Re: [PATCH v2 2/2] perf test: Add event group test

[Liang, Kan]

On 2022-12-01 10:29 a.m., Ravi Bangoria wrote:
>>>   /* Uncore pmus that support more than 3 counters */
>>>   static struct uncore_pmus {
>>>       char *name;
>>>       unsigned long config;
>>>   } uncore_pmus[] = {
>>>       { "amd_l3",         0x0  },
>>>       { "amd_df",         0x0  },
>>>       { "uncore_imc_xxx", 0xff },   /* Intel */
>>
>> IMC seems a safe choice. AFAIK, we should have at least uncore_imc_0 for
>> all the existing Intel platforms. { "uncore_imc_0", 0x1 }
> 
> Ok. Ian said he don't see uncore_imc_0 on his tigerlake machine. Are you
> sure uncore_imc_0 should be present on all existing Intel platforms?

For TGL and older client platforms, there is only free running IMC
counters. For other uncore PMUs on the old client platforms, I cannot
guarantee that then always have more then 2 counters. I think you can
skip the uncore test for these old platforms if you need at least 3
counters.


> 
>>>       { "intel_xxx_pmu2", 0xff },   /* Intel */
>>
>> Intel doesn't have such uncore PMUs.
> 
> Yeah this was just for example purpose.
> 
>>>       { "abc_pmu1",       0x0  },   /* Arm */
>>>       { "hv_24x7",        0xa  },   /* PowerPC */
>>>       { ...                    },
>>>   };
>>>
>>>   perf_pmus__for_each_pmu(pmu) {
>>>       if (pmu present in uncore_pmus[])
>>>           type[2] = pmu->type;
>>>           config[2] = pmu->config;>   }
>>
>>
>> Not sure the uncore_pmus[] can cover all possible names for all
>> architectures.
> 
> It doesn't need to cover _all_ possible names. It just needs to cover
> minimal set of names which can cover all platforms for that architecture.
>>> Maybe we should fall back to the first uncore PMU and try again if
>> nothing match the uncore_pmus[].
> 
> That's a good point. However, this can endup with the same problem you
> mentioned: it may trigger false alarm on some platform. So better to
> skip the test(and let someone add proper pmu in this list) rather than
> proving false negative result?

OK. Skipping the test for this case sounds good to me.

Thanks,
Kan
> 
> Thanks,
> Ravi