|
From: | Philippe Mathieu-Daudé |
Subject: | Re: [RFC PATCH] softmmu: Fix async_run_on_cpu() use in tcg_commit_cpu() |
Date: | Fri, 8 Sep 2023 12:49:33 +0200 |
User-agent: | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 |
On 7/9/23 21:36, Philippe Mathieu-Daudé wrote:
On 7/9/23 18:28, Richard Henderson wrote:On 9/7/23 09:14, Philippe Mathieu-Daudé wrote:CPUState::halt_cond is an accelerator specific pointer, used in particular by TCG (which tcg_commit() is about). The pointer is set by the AccelOpsClass::create_vcpu_thread() handler. AccelOpsClass::create_vcpu_thread() is called by the generic qemu_init_vcpu(), which expect the accelerator handler to eventually call cpu_thread_signal_created() which is protected with a QemuCond. It is safer to check the vCPU is created with this field rather than the 'halt_cond' pointer set in create_vcpu_thread() before the vCPU thread is initialized. This avoids calling tcg_commit() until all CPUs are realized. Here we can see for a machine with N CPUs, tcg_commit() is called N times before the 'machine_creation_done' event:(lldb) settings set -- target.run-args "-M" "virt" "-smp" "512" "-display" "none" (lldb) breakpoint set --name qemu_machine_creation_done --one-shot true(lldb) breakpoint set --name tcg_commit_cpu --auto-continue true (lldb) run Process 84089 launched: 'qemu-system-aarch64' (arm64) Process 84089 stopped* thread #1, queue = 'com.apple.main-thread', stop reason = one-shot breakpoint 2(lldb) breakpoint list --brief Current breakpoints:2: name = 'tcg_commit_cpu', locations = 2, resolved = 2, hit count = 512 Options: enabled auto-continue^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^Of course the function is called 512 times: you asked for 512 cpus, and each has its own address space which needs initializing.
My commit description is too confusing. I'll split in tiny simple changes and try to better describe each.
The AS are still initialized at the same time, but we defer the listener callback until the vCPU is ready (what was expected first IIUC).If you skip the call before cpu->created, when exactly are you going to do it?With this patch tcg_commit_cpu() is only called by vCPU threads, in their processing loop. i.e: comparing backtraces, now the first hit is: (lldb) bt * thread #514, stop reason = breakpoint 4.2* frame #0: 0x1005d9d48 qemu-system-aarch64`tcg_commit_cpu(cpu=0x173358000, data=...) at physmem.c:2493:63 frame #1: 0x10000d684 qemu-system-aarch64`process_queued_cpu_work(cpu=0x173358000) at cpus-common.c:360:13 frame #2: 0x100297390 qemu-system-aarch64`qemu_wait_io_event [inlined] qemu_wait_io_event_common(cpu=<unavailable>) at cpus.c:412:5 [artificial] frame #3: 0x100623b98 qemu-system-aarch64`mttcg_cpu_thread_fn(arg=0x173358000) at tcg-accel-ops-mttcg.c:123:9 frame #4: 0x10079f15c qemu-system-aarch64`qemu_thread_start(args=<unavailable>) at qemu-thread-posix.c:541:9frame #5: 0x18880ffa8 libsystem_pthread.dylib`_pthread_start + 148
[Prev in Thread] | Current Thread | [Next in Thread] |