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.
Paolo
> 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
>
