Message-ID: <432b0083-3e22-adf8-2699-903c71e7dd57@linux.ibm.com>
Date:   Wed, 15 Mar 2023 08:21:03 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH 2/6] tools/perf/json: Add cache metrics for s390 z16
To:     Ian Rogers <irogers@google.com>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
        sumanthk@linux.ibm.com, svens@linux.ibm.com, gor@linux.ibm.com,
        hca@linux.ibm.com
References: <20230313080201.2440201-1-tmricht@linux.ibm.com>
 <20230313080201.2440201-2-tmricht@linux.ibm.com>
 <CAP-5=fW=xVYzkgQ4vUyzkiK-oQjUQ=hLwcLT6D8VjtVCXH5oSQ@mail.gmail.com>
 <ZA9sYL/re/aNVpo+@kernel.org>
 <1ee6884a-2d92-68d9-0917-3ae4f5390714@linux.ibm.com>
 <CAP-5=fUtJsvAtrhe4xESoQc8U15WJ8BWREbH51OKoA218uJLzw@mail.gmail.com>
Content-Language: en-US
From:   Thomas Richter <tmricht@linux.ibm.com>
Organization: IBM
In-Reply-To: <CAP-5=fUtJsvAtrhe4xESoQc8U15WJ8BWREbH51OKoA218uJLzw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
Precedence: bulk

On 3/14/23 17:34, Ian Rogers wrote:
> On Tue, Mar 14, 2023 at 1:20 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>>
>> On 3/13/23 19:33, Arnaldo Carvalho de Melo wrote:
>>> Em Mon, Mar 13, 2023 at 08:22:44AM -0700, Ian Rogers escreveu:
>>>> On Mon, Mar 13, 2023 at 1:30 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>>>>>
>>>>> Add metrics for s390 z16
>>>>> - Percentage sourced from Level 2 cache
>>>>> - Percentage sourced from Level 3 on same chip cache
>>>>> - Percentage sourced from Level 4 Local cache on same book
>>>>> - Percentage sourced from Level 4 Remote cache on different book
>>>>> - Percentage sourced from memory
>>>>>
>>>>> For details about the formulas see this documentation:
>>>>> https://www.ibm.com/support/pages/system/files/inline-files/CPU%20MF%20Formulas%20including%20z16%20-%20May%202022_1.pdf
>>>>>
>>>>> Outpuf after:
>>>>>  # ./perf stat -M l4rp -- dd if=/dev/zero of=/dev/null bs=10M count=10K
>>>>>  .... dd output deleted
>>>>>
>>>>>  Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=10M count=10K':
>>>>>
>>>>>                  0      IDCW_OFF_DRAWER_CHIP_HIT         #     0.00 l4rp
>>>>>            431,866      L1I_DIR_WRITES
>>>>>              2,395      IDCW_OFF_DRAWER_IV
>>>>>                  0      ICW_OFF_DRAWER
>>>>>                  0      IDCW_OFF_DRAWER_DRAWER_HIT
>>>>>              1,437      DCW_OFF_DRAWER
>>>>>        425,960,793      L1D_DIR_WRITES
>>>>>
>>>>>       12.165030699 seconds time elapsed
>>>>>
>>>>>        0.001037000 seconds user
>>>>>       12.162140000 seconds sys
>>>>>
>>>>>  #
>>>>>
>>>>> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>>>>> Acked-By: Sumanth Korikkar <sumanthk@linux.ibm.com>
>>>>
>>>> Acked-by: Ian Rogers <irogers@google.com>
>>>
>>> Thanks, applied the first two patches, please address the review
>>> suggestions for patches 3-6 and resubmit only those.
>>>
>>> The patches will be in the public perf-tools-next branch later today.
>>>
>>> - Arnaldo
>>>
>>
>> I would really prefer the current implementation without using "ScaleUnit": "100%"
>> The reason is that these formulars are given to me from the s390 Performance team.
>> They want to use the exact same formulars on all platforms running on s390
>> which includes z/OS and z/VM. This way they are sure to get the same numbers.
>>
>> Hope this background info helps.
> 
> For the series:
> Acked-by: Ian Rogers <irogers@google.com>
> 
> Using ScaleUnit won't change the result. A ScaleUnit of "100%" means
> scale the result up by multiplying by 100 and then apply the % after
> the value. Another nit is having metrics that place their units in the
> name, like _percent, is usually a sign the name can be better. Perhaps
> we can follow up with some clean up.
> 
> Thanks,
> Ian

Thanks Ian,
I put the ScaleUnit thing on my todo list and will provide a clean up...

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294