On 03/10/2016 11:48, Alex Bennée wrote:
>
> Paolo Bonzini <pbonz...@redhat.com> writes:
>
>> On 30/09/2016 23:31, Alex Bennée wrote:
>>> tb = atomic_rcu_read(&cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)]);
>>> - if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
>>> - tb->flags != flags)) {
>>> + if (unlikely(!tb || atomic_read(&tb->pc) != pc ||
>>> atomic_read(&tb->cs_base) != cs_base ||
>>> + atomic_read(&tb->flags) != flags)) {
>>
>> This should not be necessary (and is responsible for the 64-on-32
>> compilation failure). The load of tb from the cache is an acquire
>> operation, and synchronizes with the corresponding store in
>> cpu->tb_jmp_cache.
>
> Is the C11 spec happy with "plain" accesses after the acquire operation?
Only if there are no concurrent atomic writes---but there shouldn't be,
since those three fields are immutable after tb_alloc, and in turn
tb_alloc's assignments are not visible until qht publishes them.
It's actually a bit more complex, and if I understand the
ramifications correctly the "barrier" in
#define smp_wmb() ({ barrier(); __atomic_thread_fence(__ATOMIC_RELEASE); })
might not be required at all. I need to run this by the memory model
heavyweights. So it's possible that this is a sanitizer bug.
Paolo
> Unfortunately the sanitizer isn't able to see the indirect acquires
> effect on the other accesses.
>
>>
>> Paolo
>
>
> --
> Alex Bennée
>
