MIME-Version: 1.0
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>
In-Reply-To: <1ee6884a-2d92-68d9-0917-3ae4f5390714@linux.ibm.com>
From:   Ian Rogers <irogers@google.com>
Date:   Tue, 14 Mar 2023 09:34:46 -0700
Message-ID: <CAP-5=fUtJsvAtrhe4xESoQc8U15WJ8BWREbH51OKoA218uJLzw@mail.gmail.com>
Subject: Re: [PATCH 2/6] tools/perf/json: Add cache metrics for s390 z16
To:     Thomas Richter <tmricht@linux.ibm.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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Mar 14, 2023 at 1:20=E2=80=AFAM Thomas Richter <tmricht@linux.ibm.c=
om> 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=E2=80=AFAM 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=3D/dev/zero of=3D/dev/null bs=3D10M c=
ount=3D10K
> >>>  .... dd output deleted
> >>>
> >>>  Performance counter stats for 'dd if=3D/dev/zero of=3D/dev/null bs=
=3D10M count=3D10K':
> >>>
> >>>                  0      IDCW_OFF_DRAWER_CHIP_HIT         #     0.00 l=
4rp
> >>>            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 Performa=
nce team.
> They want to use the exact same formulars on all platforms running on s39=
0
> which includes z/OS and z/VM. This way they are sure to get the same numb=
ers.
>
> 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 a lot.
> --
> Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, German=
y
> --
> Vorsitzender des Aufsichtsrats: Gregor Pillen
> Gesch=C3=A4ftsf=C3=BChrung: David Faller
> Sitz der Gesellschaft: B=C3=B6blingen / Registergericht: Amtsgericht Stut=
tgart, HRB 243294
>