On 4/22/2023 12:42 AM, Alex Bennée wrote: > > Fei Wu <[email protected]> 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? > I sent out v12 series which fixes some CI failures. qatomic is not touched yet, the current code with CONFIG_PROFILER should have the same issue, what's the policy of 32 bit guests support on qemu?
Besides time, there are some other counters with uint64_t using qatomic such as TCGProfile.table_op_count, we might switch to size_t instead? > - 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. > added one in docs/tb-stats.txt. Some extra bits could be added to explain the fields of the output. > - 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) > Is it reproducible on your system? I didn't see it on my system, is it possible the system events cause this? > - code motion in 9/14 should be folded into the first patch > done. btw, I also added a few comments on v12 series, could you please check if they make sense? Thanks, Fei. > 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? >
