Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set

[Sergey Fedorov]

On 18/07/16 20:11, Paolo Bonzini wrote:
>
> On 18/07/2016 19:07, Sergey Fedorov wrote:
>> On 18/07/16 20:00, Paolo Bonzini wrote:
>>> On 18/07/2016 18:57, Sergey Fedorov wrote:
>>>> On 18/07/16 19:53, Paolo Bonzini wrote:
>>>>> On 18/07/2016 18:52, Sergey Fedorov wrote:
>>>>>> So how are we going to use them?
>>>>> Instead of atomic_read/atomic_set when marking invalid TBs.
>>>> But shouldn't they be atomic to avoid reading torn writes?
>>> A torn write would probably fail to match anyway, but even if it doesn't
>>> it is indistinguishable from a race, isn't it?
>> I'm afraid, torn write can happen to be a false match against a wrong
>> TB. In case of a race with atomic access we either get the right TB or
>> an invalid one which couldn't match any valid CPU state. Probably, we
>> have to make sure (and document this) that TB invalidation process
>> cannot make a partially invalidated TB which can match any meaningful
>> CPU state.
> x86 is atomic (because flags are 32-bit); those that have cs_base==0 are
> safe against torn writes too.  Only SPARC perhaps could use
> "tb->cs_base|=1" instead in case 0xffffffff........ matches another TB.

That could really work but needs some comment, of course. BTW, what is
the main point of such change? A bit more performance on some 32-bit hosts?

Thanks,
Sergey

>
> Paolo
>
>>> By the way, tb_cmp should also use volatile_read.
>> You are right, we must user the save type of access in tb_cmp().
>>
>> Thanks,
>> Sergey
>>