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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <906aa237-e300-aaf5-2de3-0041985ae646@linux.ibm.com>
Date:   Sun, 26 Jul 2020 14:44:05 +0530
From:   kajoljain <kjain@...ux.ibm.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Jiri Olsa <jolsa@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Michael Petlan <mpetlan@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        John Garry <john.garry@...wei.com>,
        "Paul A. Clarke" <pc@...ibm.com>,
        Stephane Eranian <eranian@...gle.com>,
        Ian Rogers <irogers@...gle.com>
Subject: Re: [PATCHv3 00/19] perf metric: Add support to reuse metric



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