Re: [PATCHv3 00/19] perf metric: Add support to reuse metric

kajoljain
2020-07-26T09:14:26Z

On 7/25/20 5:21 PM, Jiri Olsa wrote:
> On Fri, Jul 24, 2020 at 11:22:28AM +0530, kajoljain wrote:
>
> SNIP
>
>>
>> Hi Jiri,
>> The change looks good to me. I tried with adding this patch on top of your perf/metric branch. It did resolve the issue of not printing
>> all chips data. And now I can see proper values for hv-24x7 metric events.
>>
>> I was also trying by adding new metric using the feature added in this patchset with something like this:
>>
>> diff --git a/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json b/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
>> index 8383a37647ad..dfe4bd63b587 100644
>> --- a/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
>> +++ b/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
>> @@ -16,6 +16,11 @@
>> "MetricName": "PowerBUS_Frequency",
>> "ScaleUnit": "2.5e-7GHz"
>> },
>> + {
>> + "MetricExpr": "Memory_WR_BW_Chip + Memory_RD_BW_Chip",
>> + "MetricName": "Total_Memory_BW",
>> + "ScaleUnit": "1.6e-2MB"
>> + },
>
> hum, we'll need special case this.. because Memory_WR_BW_Chip will
> unwind to Memory_WR_BW_Chip_[01] and Total_Memory_BW is not aware of
> that.. what's the expected behaviour in here?
>
> have Total_Memory_BW_[01] for each runtime arg?

Hi Jiri,
Yes right. So we want Total_Memory_BW to show sum results of both chip 0 and 1 seperately, which is missing here.
>
> I think this will need to come on top of this changes,
> it's already too big
>

Yes make sense. We can send separate patches on top of this patch set for this use case.

Other then that the whole patchset looks good to me with the change to rectify Paul A. Clarke concern.

Tested/Reviewed-By : Kajol Jain<kjain@linux.ibm.com>

Thanks,
Kajol Jain
> thanks,
> jirka
>
>>
>> I guess as we have dependency on '?' symbol, I am not able to see all chips data for Total_Memory_BW.
>> I am not sure if Its expected behavior?
>>
>> This is what I am getting:
>>
>> [root@ltc-zz189-lp4 perf]# ./perf stat --metric-only -M Total_Memory_BW,Memory_WR_BW_Chip,Memory_RD_BW_Chip -I 1000 -C 0
>> # time MB Total_Memory_BW MB Memory_RD_BW_Chip_1 MB Memory_WR_BW_Chip_1 MB Memory_WR_BW_Chip_0 MB Memory_RD_BW_Chip_0
>> 1.000067388 36.4 0.2 36.3 65.0 72.1
>> 2.000374276 36.2 0.3 35.9 65.4 77.9
>> 3.000543202 36.3 0.3 36.0 68.7 81.2
>> 4.000702855 36.3 0.3 36.0 70.9 93.3
>> 5.000856837 36.0 0.2 35.8 67.4 81.5
>> ^C 5.367865273 13.2 0.1 13.1 23.5 28.3
>> Performance counter stats for 'CPU(s) 0':
>> 194.4 1.3 193.1 361.0 434.3
>> 5.368039176 seconds time elapsed
>>
>> We can only get single chip data's sum in Total_Memory_BW. Please let me know if I am missing something.
>
> SNIP
>