Re: [Qemu-devel] [PATCH 3/3] tcg: signal-free qemu_cpu_kick

[Paolo Bonzini <pbonzini@redhat.com>]

On 17/08/2015 11:31, Richard Henderson wrote:
> On 08/14/2015 06:15 AM, Paolo Bonzini wrote:
>> +    atomic_mb_set(&current_cpu, cpu);
> ...
>> +        cpu_exit(atomic_rcu_read(&current_cpu));
> 
> Mixing java and rcu style sync to the same data structure?

Well, I usually read rcu_read as CONSUME, rcu_set as RELEASE, mb_read as
either ACQUIRE or "SEQ_CST without IRIW" and mb_set as "SEQ_CST without
IRIW".  But you're right that the patch is unreadable.

>> +                         * ensure tcg_exit_req is read before exit_request
>> +                         * or interrupt_request.
>>                           */
>> +                        smp_rmb();
>>                          next_tb = 0;
> 
> This I don't understand, since we've just read exit_request above, and you're
> putting the barrier here?

If we see cpu->exit_request == 1, we exit.  In that case,
cpu->tcg_exit_req doesn't matter.

Here we saw cpu->exit_request == 0 and then got TB_EXIT_REQUESTED.
Because of TB_EXIT_REQUESTED we know cpu->tcg_exit_req is 1; the
smp_rmb() ensures that cpu->exit_request will be read as 1 on the next
iteration.

Paolo

>> +        /* Ensure whatever caused the exit has reached the CPU threads before
>> +         * writing exit_request.
>> +         */
>> +        smp_wmb();
>> +        exit_request = 1;
>> +        /* Ignore the CPU argument since all CPUs run in the same thread;
>> +         * preempt the currently running one.  The memory barriers ensures
>> +         * that other CPUs will see the request if the current CPU is
>> +         * preempted.
>> +         */
>> +        smp_wmb();
>> +        cpu_exit(atomic_rcu_read(&current_cpu));
> 
> ...
> 
>> +    /* Pairs with smp_wmb in qemu_cpu_kick.  */
>> +    atomic_mb_set(&exit_request, 0);
>>  }
> 
> Bare barriers and java style sync to the same data structure?
> 
>>      cpu->exit_request = 1;
>> +    /* Ensure cpu_exec will see the exit request after TCG has exited.  */
>> +    smp_wmb();
>>      cpu->tcg_exit_req = 1;
>>  }
> 
> Likewise.
> 
> I find this mixing highly confusing.  I see no way to prove that it's going to
> be right for non-x86.
> 
> 
> r~
> 
>