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

[Lluís Vilanova]

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.

To make it work, we'd need to add some new boolean flag on the vCPU to control
when to reset "last_tb", and then we're just as good as implementing the async
work "protocol" manually for this specific case.

What I'll do is fix the race condition by simplifying that code (haven't looked
at the problem yet).


Thanks,
  Lluis