Date:   Tue, 14 Mar 2023 18:36:24 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Ian Rogers <irogers@google.com>
Cc:     Thomas Richter <tmricht@linux.ibm.com>,
        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
Subject: Re: [PATCH 2/6] tools/perf/json: Add cache metrics for s390 z16
Message-ID: <ZBDo2GiuUTrHhd2L@kernel.org>
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>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAP-5=fUtJsvAtrhe4xESoQc8U15WJ8BWREbH51OKoA218uJLzw@mail.gmail.com>
Precedence: bulk

Em Tue, Mar 14, 2023 at 09:34:46AM -0700, Ian Rogers escreveu:
> 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>

Thanks, applied.

- Arnaldo

 
> 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, Germany
> > --
> > Vorsitzender des Aufsichtsrats: Gregor Pillen
> > Geschäftsführung: David Faller
> > Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
> >

-- 

- Arnaldo