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<kj...@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
> 

Reply via email to