RE: [PATCH RFC V6 24/24] tcg: Defer TB flush for 'lazy realized' vCPUs on first region alloc

[Salil Mehta via <qemu-devel@nongnu.org>]

Hi Richard,

Thanks for the reply. Please find my response inline.

Cheers.

> From: qemu-devel-bounces+salil.mehta=huawei.com@nongnu.org <qemu-
> devel-bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of Richard
> Henderson
> Sent: Wednesday, October 1, 2025 10:34 PM
> To: salil.mehta@opnsrc.net; qemu-devel@nongnu.org; qemu-
> arm@nongnu.org; mst@redhat.com
> Subject: Re: [PATCH RFC V6 24/24] tcg: Defer TB flush for 'lazy realized' vCPUs
> on first region alloc
> 
> On 9/30/25 18:01, salil.mehta@opnsrc.net wrote:
> > From: Salil Mehta <salil.mehta@huawei.com>
> >
> > The TCG code cache is split into regions shared by vCPUs under MTTCG.
> > For cold-boot (early realized) vCPUs, regions are sized/allocated during
> bring-up.
> > However, when a vCPU is *lazy_realized* (administratively "disabled"
> > at boot and realized later on demand), its TCGContext may fail the
> > very first code region allocation if the shared TB cache is saturated
> > by already-running vCPUs.
> >
> > Flushing the TB cache is the right remediation, but `tb_flush()` must
> > be performed from the safe execution context
> (cpu_exec_loop()/tb_gen_code()).
> > This patch wires a deferred flush:
> >
> >    * In `tcg_region_initial_alloc__locked()`, treat an initial allocation
> >      failure for a lazily realized vCPU as non-fatal: set `s->tbflush_pend`
> >      and return.
> >
> >    * In `tcg_tb_alloc()`, if `s->tbflush_pend` is observed, clear it and
> >      return NULL so the caller performs a synchronous `tb_flush()` and then
> >      retries allocation.
> >
> > This avoids hangs observed when a newly realized vCPU cannot obtain
> > its first region under TB-cache pressure, while keeping the flush at a safe
> point.
> >
> > No change for cold-boot vCPUs and when accel ops is KVM.
> >
> > In earlier series, this patch was with below named,
> > 'tcg: Update tcg_register_thread() leg to handle region alloc for hotplugged
> vCPU'
> 
> 
> I don't see why you need two different booleans for this.


I can see your point. Maybe I can move `s->tbflush_pend`  to 'CPUState' instead? 


> It seems to me that you could create the cpu in a state for which the first call
> to
> tcg_tb_alloc() sees highwater state, and everything after that happens per
> usual allocating a new region, and possibly flushing the full buffer.


Correct. but with a distinction that highwater state is relevant to a TCGContext
and the regions are allocated from a common pool 'Code Generation Buffer'.
'code_gen_highwater' is use to detect whether current context needs more
region allocation for the dynamic translation to continue. This is a different
condition than what we are encountering; which is the worst case condition
that the entire code generation buffer is saturated and cannot even allocate
a single free TCG region successfully. In such a case, we do not have any option
than to flush the entire buffer and reallocate the regions to all the threads.
A rebalancing act to accommodate a new vCPU - which is expensive but the
good thing is this does not happens every time and is a worst case condition
i.e. when a system is under tremendous stress and is running out of resources. 


We are avoiding this crash:

ERROR:../tcg/region.c:396:tcg_region_initial_alloc__locked: assertion failed: (!err)
Bail out! ERROR:../tcg/region.c:396:tcg_region_initial_alloc__locked: assertion failed: (!err)
./run-qemu.sh: line 8: 255346 Aborted                 
(core dumped) ./qemu/build/qemu-system-aarch64 -M virt,accel=tcg

Dump is here:

Thread 65 "qemu-system-aar" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fff48ff9640 (LWP 633577)]
0x00007ffff782f98c in __pthread_kill_implementation () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff782f98c in __pthread_kill_implementation () at /lib64/libc.so.6
#1  0x00007ffff77e2646 in raise () at /lib64/libc.so.6
#2  0x00007ffff77cc7f3 in abort () at /lib64/libc.so.6
#3  0x00007ffff7c21d6c in g_assertion_message_expr.cold () at /lib64/libglib-2.0.so.0
#4  0x00007ffff7c7ce2f in g_assertion_message_expr () at /lib64/libglib-2.0.so.0
#5  0x00005555561cf359 in tcg_region_initial_alloc__locked (s=0x7fff10000b60) at ../tcg/region.c:396
#6  0x00005555561cf3ab in tcg_region_initial_alloc (s=0x7fff10000b60) at ../tcg/region.c:402
#7  0x00005555561da83c in tcg_register_thread () at ../tcg/tcg.c:820
#8  0x00005555561a97bb in mttcg_cpu_thread_fn (arg=0x555557e0c2b0) at ../accel/tcg/tcg-accel-ops-mttcg.c:77
#9  0x00005555564f18ab in qemu_thread_start (args=0x5555582e2bc0) at ../util/qemu-thread-posix.c:541
#10 0x00007ffff782dc12 in start_thread () at /lib64/libc.so.6
#11 0x00007ffff78b2cc0 in clone3 () at /lib64/libc.so.6
(gdb)



> 
> What is the testcase for this?


As mentioned, tackling a worst case when 'code generation buffer' runs out
of space totally. We need a better mitigation plan that to simply assert().

Can be easily reproducible by decreasing the 'tb_size'  and increasing the 
number of vCPUs, and having larger programs running simultaneously.
I was able to reproduce it with only 6 vCPUs and with 'tb_size=10'.
Booting was dead slow but with a single vCPU hotplug action we can
 reproduce it.

RFC V6 has TCG broken for some other reason and I'm trying to fix it.
But if you wish you can try this on RFC 5 which has greater chances of
this happening as it actually uses vCPU hotplug approach i.e. threads
can be created and deleted.

https://github.com/salil-mehta/qemu/commits/virt-cpuhp-armv8/rfc-v5/

With RFC V6 this condition is likely to happen only once during delayed
spawning of the vCPU thread of a VCPU being lazily realized. We do not
delete the spawned thread.

Many thanks!

Best regards
Salil.

> 
> 
> r~