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

[Richard Henderson <richard.henderson@linaro.org>]

On 10/2/25 05:27, Salil Mehta wrote:
> 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.

I think you misunderstand "and everything after that happens per usual".

When allocating a tb, if a cpu finds that it's current region is full, then it tries to 
allocate another region.  If that is not successful, then we flush the entire 
code_gen_buffer and try again.

Thus tbflush_pend is exactly equivalent to setting

     s->code_gen_ptr > s->code_gen_highwater.

As far as lazy_realized...  The utility of the assert under these conditions may be called 
into question; we could just remove it.


r~