2024-01-31 17:39:21

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH/RFC] lib: add CPU MHz benchmark test

Hi Geert,

On Wed, Jan 31, 2024 at 05:46:48PM +0100, Geert Uytterhoeven wrote:
> When working on SoC bring-up, (a full) userspace may not be available,
> making it hard to benchmark the CPU performance of the system under
> development. Still, one may want to have a rough idea of the (relative)
> performance of one or more CPU cores, especially when working on e.g.
> the clock driver that controls the CPU core clock(s).
>
> Hence add the CPU MHz benchmark test[1], which estimates the clock
> frequency of the CPU core it is running on, and make it available as a
> Linux kernel test module.
>
> When built-in, this benchmark can be run without any userspace present.

That's a great idea, I never thought about turning it into a module!

> Parallel runs (run on multiple CPU cores) are supported, just kick the
> "run" file multiple times.

Hmmm does it mean it will run on the CPU that writes this "run" ?
Because this could allow one to start tests using e.g.:

taskset -c $CPU tee /sys/.../run <<< y

But we could also wonder if it wouldn't be easier to either send "y"
to /sys/.../cpu0/run or may just send the CPU number to "run" instead
of "y". In my experience with this tool, you most always want to easily
control the CPU number because SoCs these days are not symmetrical at
all.

> This has been tested on the folowing CPU cores:
> - ARM: Cortex A7, A9, and A15,
> - ARM64: Cortex A53, A55, A57, and A76,
> - m68k: MC68040,
> - MIPS: TX4927,
> - RISC-V: AndesTech AX45, Kendryte K210, SiFive U54 and U74, VexRiscV.
> - SuperH: SH7751R.
> The reported figures are usually within 1-2% of the actual CPU clock
> rate.

Nice!

> Known issues:
> - The reported value is off on the following systems:
> - RBTX4927: 120 MHz (should be 200 MHz, userspace mhz is OK)
> user: count=76500 us50=19990 us250=96885 diff=76895 cpu_MHz=198.973
> kernel: 43663 19943 93024 119
> msleep(1000) does sleep 1s, and ktime_get() advances accordingly
> - RZ/Five: 1971 MHz (should be 1000 MHz, userspace mhz not tested)
> kernel: 679625 20001 88962 1971
> msleep(1000) does sleep 1s, and ktime_get() advances accordingly
> - VexRiscV: 12 MHz (should be 64 MHz, userspace mhz not tested)
> I assume this is due to different optimization flags.
> I haven't compared the generated code yet.

That's strange. I only ever managed to get off results at -O0, but at
-O1/-Og/-Os/-O2/-O3/-Ofast I've always got consistent results on all
the machines I've tested. Especially seeing results higher than the
real value is troubling. One possibility would be that one some archs
there's not enough registers and the compiler needs to use a variable
in the stack, but that could only explain the lower perf, not the higher
one. But indeed, it could be interesting to have a look at the code to
understand why it's doing that. If we figure that there's an inherent
limitation on some rare archs, in the worst case we could place a
warning there.

> - On fast systems with a large clock granularity (e.g. ARAnyM running
> Linux/m68k), the measured durations for the short and long loops may
> be identical, causing division-by-zero exceptions.
> The same happens with the userspace version, cfr.
> https://github.com/wtarreau/mhz/issues/5.

I've responded there and we definitely need to address this, thanks!

Another point is that it would be nice if there was a way to present
the result in a form that a script can retrieve from the directory,
maybe the last measurement or something like this. I know that scripts
are commonly used to check for a machine's correct behavior, and I try
to encourage users to verify that it's working well, so anything we can
do that makes it easier to use would be welcome.

But overall, I like it! You've got my ack.

Hmmm I don't know if this is intended, the SPDX tag says MIT but the
MODULE_LICENSE at the top says MIT/GPL. I can't say I care that much but
I preferred to report it in case it's an accident ;-)

Thanks!
Willy


2024-02-01 08:50:13

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH/RFC] lib: add CPU MHz benchmark test

Hi Willy,

On Wed, Jan 31, 2024 at 6:39 PM Willy Tarreau <[email protected]> wrote:
> On Wed, Jan 31, 2024 at 05:46:48PM +0100, Geert Uytterhoeven wrote:
> > When working on SoC bring-up, (a full) userspace may not be available,
> > making it hard to benchmark the CPU performance of the system under
> > development. Still, one may want to have a rough idea of the (relative)
> > performance of one or more CPU cores, especially when working on e.g.
> > the clock driver that controls the CPU core clock(s).
> >
> > Hence add the CPU MHz benchmark test[1], which estimates the clock
> > frequency of the CPU core it is running on, and make it available as a
> > Linux kernel test module.
> >
> > When built-in, this benchmark can be run without any userspace present.
>
> That's a great idea, I never thought about turning it into a module!
>
> > Parallel runs (run on multiple CPU cores) are supported, just kick the
> > "run" file multiple times.
>
> Hmmm does it mean it will run on the CPU that writes this "run" ?
> Because this could allow one to start tests using e.g.:
>
> taskset -c $CPU tee /sys/.../run <<< y

That does indeed work.

> But we could also wonder if it wouldn't be easier to either send "y"
> to /sys/.../cpu0/run or may just send the CPU number to "run" instead
> of "y".

That would complicate the code a lot.

> In my experience with this tool, you most always want to easily
> control the CPU number because SoCs these days are not symmetrical at
> all.

That's why it prints the CPU number ;-)

On multi-core systems, you can also do e.g.

for i in $(seq $(nproc)); do echo yes >
/sys/module/test_mhz/parameters/run & done

and collect the results for all CPU cores.

BTW, this is the same for test_dhry.

> Another point is that it would be nice if there was a way to present
> the result in a form that a script can retrieve from the directory,
> maybe the last measurement or something like this. I know that scripts
> are commonly used to check for a machine's correct behavior, and I try
> to encourage users to verify that it's working well, so anything we can
> do that makes it easier to use would be welcome.

I'll give that a try...

> But overall, I like it! You've got my ack.

Thanks!

> Hmmm I don't know if this is intended, the SPDX tag says MIT but the
> MODULE_LICENSE at the top says MIT/GPL. I can't say I care that much but
> I preferred to report it in case it's an accident ;-)

That must be an oversight. I'll change the SPDX-License-Identifier to
"GPL-2.0 OR MIT".

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-02-11 11:17:55

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH/RFC] lib: add CPU MHz benchmark test

Hi Geert!

On Thu, Feb 01, 2024 at 09:49:33AM +0100, Geert Uytterhoeven wrote:
> > > Parallel runs (run on multiple CPU cores) are supported, just kick the
> > > "run" file multiple times.
> >
> > Hmmm does it mean it will run on the CPU that writes this "run" ?
> > Because this could allow one to start tests using e.g.:
> >
> > taskset -c $CPU tee /sys/.../run <<< y
>
> That does indeed work.

OK!

> > But we could also wonder if it wouldn't be easier to either send "y"
> > to /sys/.../cpu0/run or may just send the CPU number to "run" instead
> > of "y".
>
> That would complicate the code a lot.

OK I trust you, I was merely asking just in case.

> > In my experience with this tool, you most always want to easily
> > control the CPU number because SoCs these days are not symmetrical at
> > all.
>
> That's why it prints the CPU number ;-)
>
> On multi-core systems, you can also do e.g.
>
> for i in $(seq $(nproc)); do echo yes >
> /sys/module/test_mhz/parameters/run & done
>
> and collect the results for all CPU cores.

OK!

> BTW, this is the same for test_dhry.

I didn't know, that's an even better reason for not changing any of this!

> > Another point is that it would be nice if there was a way to present
> > the result in a form that a script can retrieve from the directory,
> > maybe the last measurement or something like this. I know that scripts
> > are commonly used to check for a machine's correct behavior, and I try
> > to encourage users to verify that it's working well, so anything we can
> > do that makes it easier to use would be welcome.
>
> I'll give that a try...

Thanks.

> > Hmmm I don't know if this is intended, the SPDX tag says MIT but the
> > MODULE_LICENSE at the top says MIT/GPL. I can't say I care that much but
> > I preferred to report it in case it's an accident ;-)
>
> That must be an oversight. I'll change the SPDX-License-Identifier to
> "GPL-2.0 OR MIT".

OK no problem!

Thanks,
Willy