Fei Wu <fei2...@intel.com> writes:
> This patch series were done by Vanderson and Alex originally in 2019, I > (Fei Wu) rebased them on latest upstream from: > https://github.com/stsquad/qemu/tree/tcg/tbstats-and-perf-v10 > and send out this review per Alex's request, I will continue to address > any future review comments here. As it's been a very long time and there > are lots of conflicts during rebase, it's my fault if I introduce any > problems during the process. Hi Fei, Thanks for picking this up. I can confirm that this applies cleanly to master and I have kicked the tyres and things still seem to work. I'm not sure if I can provide much review on code I wrote but a few things to point out: - there are a number of CI failures, mainly qatomic on 32 bit guests see https://gitlab.com/stsquad/qemu/-/pipelines/844857279/failures maybe we just disable time accounting for 32 bit hosts? - we need a proper solution to the invalidation of TBs so we can exclude them from lists (or at least not do the attempt translation/fail dance). Alternatively we could page out a copy of the TB data to a disk file when we hit a certain hotness? How would this interact with the jitperf support already? - we should add some documentation to the manual so users don't have to figure it all out by trail and error at the HMP command line. - there may be some exit cases missed because I saw some weird TB's with very long IR generation times. TB id:5 | phys:0xb5f21d00 virt:0xffffcf2f17721d00 flags:0x00000051 1 inv/2 | exec:1889055/0 guest inst cov:1.05% | trans:2 ints: g:4 op:32 op_opt:26 spills:0 | h/g (host bytes / guest insts): 56.000000 | time to gen at 2.4GHz => code:6723.33(ns) IR:2378539.17(ns) - code motion in 9/14 should be folded into the first patch Even if we can't find a solution for safely dumping TBs I think the series without "tb-list" is still an improvement for getting rid of the --enable-profiler and making info JIT useful by default. Richard, What do you think? -- Alex Bennée Virtualisation Tech Lead @ Linaro
