Re: [Qemu-devel] [PATCH v6 3/7] trace: [tcg] Delay changes to dynamic state when translating

[Lluís Vilanova]

Paolo Bonzini writes:

> On 12/01/2017 20:37, Lluís Vilanova wrote:
>> Stefan Hajnoczi writes:
>> 
>>> On Tue, Jan 10, 2017 at 05:31:37PM +0100, Paolo Bonzini wrote:
>>>> On 09/01/2017 18:01, Stefan Hajnoczi wrote:
>>>>> Or use a simpler scheme:
>>>>> 
>>>>> struct CPUState {
>>>>> ...
>>>>> uint32_t dstate_update_count;
>>>>> };
>>>>> 
>>>>> In trace_event_set_vcpu_state_dynamic():
>>>>> 
>>>>> if (state) {
>>>>> trace_events_enabled_count++;
>>>>> set_bit(vcpu_id, vcpu->trace_dstate_delayed);
>>>>> atomic_inc(&vcpu->dstate_update_count, 1);
>>>>> (*ev->dstate)++;
>>>>> } ...
>>>>> 
>>>>> In cpu_exec() and friends:
>>>>> 
>>>>> last_dstate_update_count = atomic_read(&vcpu->dstate_update_count);
>>>>> 
>>>>> tb = tb_find(cpu, last_tb, tb_exit);
>>>>> cpu_loop_exec_tb(cpu, tb, &last_tb, &tb_exit, &sc);
>>>>> 
>>>>> /* apply and disable delayed dstate changes */
>>>>> if (unlikely(atomic_read(&cpu->dstate_update_count) != 
>>>>> last_dstate_update_count)) {
>>>>> bitmap_copy(cpu->trace_dstate, cpu->trace_dstate_delayed,
>>>>> trace_get_vcpu_event_count());
>>>>> }
>>>>> 
>>>>> (You'll need to adjust the details but the update counter approach
>>>>> should be workable.)
>>>> 
>>>> Would it work to use async_run_on_cpu?
>> 
>>> I think so.
>> 
>> AFAIU we cannot use async_run_on_cpu(), since we need to reset the local
>> variable "last_tb" to avoid chaining TBs with different dstates, and we 
>> cannot
>> use cpu_loop_exit() inside the callback.

> async_run_on_cpu would run as soon as the currently executing TB
> finishes, and would leave cpu_exec completely, so there would be no
> chaining.

Aha, I've re-read the internals used by async and that'll be sufficient.


Thanks,
  Lluis