qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: Daniel Henrique Barboza <danielhb413@gmail.com>, qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org, clg@kaod.org, david@gibson.dropbear.id.au
Subject: Re: [PATCH 0/8] Re-write PPC64 PMU instruction count using TCG Ops
Date: Wed, 22 Dec 2021 18:43:34 -0800	[thread overview]
Message-ID: <f4185811-f0dc-8ee9-c254-ebb04d883cd6@linaro.org> (raw)
In-Reply-To: <20211222134520.587877-1-danielhb413@gmail.com>

On 12/22/21 5:45 AM, Daniel Henrique Barboza wrote:
> Hi,
> 
> Two days ago Richard Henderson reported test failures with Avocado and
> powernv8/9 due to timeouts [1]. The culprit ended up to be commit , a
> commit where I introduced PMU instruction counting for TCG PPC64.
> 
> For a reason that is still unclear to me these Avocado powernv tests are
> suffering a huge performance impact after that patch, something that I
> didn't verify in any other scenario I've tested. So one alternative to
> fix the situation is to understand this difference and try to solve it,
> which can take some time.
>   
> Another alternative is to optimize the code introduced by that commit.
> Today the instruction count is done by a TCG helper that is called after
> each TB exit. I was aware that calling a helper frequently isn't
> optimal, but that got the job done and didn't  hindered the use of
> pSeries and powernv machines.  Well, until [1] at least.
> 
> This series rewrites the PMU instruction counting using TCG Ops instead
> of a TCG helper. To do that we needed to write in TCG Ops not only the
> logic for increment the counters but also the logic to detect counter
> overflows.
> 
> A lot of code was added but the performance improvement is noticeable.
> Using my local machine I did some test runs with the 2 Avocado powernv
> tests that are timing out at this moment:

You generate a *lot* of inline code here.  Way too much, actually.

If you can get this performance improvement with this reorg, it merely means that your 
original C algorithm was poor.  The compiler should have been able to do better.

I've tested this theory here and...

> - failing Avocado powernv tests with current master:
> 
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (70.17 s)
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (70.90 s)
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (70.81 s)
>   
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (75.62 s)
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (69.79 s)
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (72.33 s)

boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (75.73 s)
boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (80.20 s)

> - after this series:
> 
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (39.90 s)
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (38.25 s)
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (37.99 s)
> 
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (43.17 s)
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (43.64 s)
>   (1/1) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (44.21 s)

boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (39.66 s)
boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (43.02 s)

BTW, pre-power8-pmu, 29c4a3363b:

boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8: PASS (36.62 s)
boot_linux_console.py:BootLinuxConsole.test_ppc_powernv9: PASS (39.69 s)

I'll post my series shortly.


r~


      parent reply	other threads:[~2021-12-23  2:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-22 13:45 [PATCH 0/8] Re-write PPC64 PMU instruction count using TCG Ops Daniel Henrique Barboza
2021-12-22 13:45 ` [PATCH 1/8] target/ppc: introduce power8-pmu-insn-cnt.c.inc Daniel Henrique Barboza
2021-12-22 18:00   ` Cédric Le Goater
2021-12-22 18:10     ` Daniel Henrique Barboza
2021-12-22 13:45 ` [PATCH 2/8] target/ppc/power8-pmu-insn-cnt: add pmu_inc_pmc5() Daniel Henrique Barboza
2021-12-22 13:45 ` [PATCH 3/8] target/ppc/power8-pmu-insn-cnt: add pmu_inc_pmc1() Daniel Henrique Barboza
2021-12-22 13:45 ` [PATCH 4/8] target/ppc/power8-pmu-insn-cnt: add pmu_inc_pmc2() Daniel Henrique Barboza
2021-12-22 13:45 ` [PATCH 5/8] target/ppc/power8-pmu-insn-cnt: add pmu_inc_pmc3() Daniel Henrique Barboza
2021-12-22 13:45 ` [PATCH 6/8] target/ppc/power8-pmu-insn-cnt.c: add pmu_inc_pmc4() Daniel Henrique Barboza
2021-12-22 13:45 ` [PATCH 7/8] target/ppc/power8-pmu-insn-cnt: add pmu_check_overflow() Daniel Henrique Barboza
2021-12-22 13:45 ` [PATCH 8/8] target/ppc/power8-pmu.c: remove helper_insns_inc() Daniel Henrique Barboza
2021-12-23  2:43 ` Richard Henderson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f4185811-f0dc-8ee9-c254-ebb04d883cd6@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=clg@kaod.org \
    --cc=danielhb413@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).