Re: [PATCH v4 0/5] powerpc/perf: Add json file support for hv_24x7 core level events

[kajoljain <kjain@linux.ibm.com>]

On 7/21/20 1:01 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jul 20, 2020 at 09:21:33PM +0200, Jiri Olsa escreveu:
>> On Mon, Jul 20, 2020 at 12:31:22PM +0530, Kajol Jain wrote:
>>> Patchset enhance current runtime parameter support. It introduces new
>>> fields like "PerChip" and "PerCore" similar to the field "PerPkg" which is
>>> used to specify perpkg events. 
>>>
>>> The "PerCore" and "PerChip" specifies whether its core or chip events.
>>> Based on which we can decide which runtime parameter user want to
>>> access. Now character  '?' can refers different parameter based on user
>>> requirement.
>>>
>>> Initially, every time we want to add new terms like chip, core, thread
>>> etc, we need to create corrsponding fields in pmu_events and event
>>> struct.
>>> This patchset adds an enum called 'aggr_mode_class' which store all these
>>> aggregation like perpkg/percore. It also adds new field 'AggregationMode'
>>> to capture these terms.
>>> Now, if user wants to add any new term, they just need to add it in
>>> the enum defined. I try to test it with  my current setup.
>>>
>>> I also need to replace PerPkg field to AggregationMode in all the
>>> x86 uncore json files. It will great if Andi and team can test it
>>> and let me know if they have any concerns.
>>>
>>> Changelog:
>>> v3 -> v4:
>>> - Include pmu-events.h header file in jevents.c and remove
>>>   redecalaration of enum aggr_mode_class as Suggested by Jiri.
>>> - Add Acked-by tag.
>>
>> looks good to me, but still missing reaction from people maintaining
>> intel's jsons
> 
> Yeah, since this is something trying to be generic enough to describe
> events from different arches, we should not double-fail into
> generalizing this :-\

Hi,
   Yes that make sense. In this patchset we are not doing any logical change from intel side,
as in all the json file "PerPkg" value map to '1', we are using same value in the
enum to not break that use case. And just replacing 'PerPkg' to new added field
`AggregationMode'. 

But you are right, it will be good to have Ack from intel people. Any comments how we can push these
changes?

Thanks,
Kajol Jain
> 
> - Arnaldo
>