Re: [Qemu-devel] [PATCH v6 1/8] atomic: introduce atomic operations

[liu ping fan]

On Wed, Nov 14, 2012 at 5:47 PM, Paolo Bonzini <address@hidden> wrote:
> Il 14/11/2012 10:38, liu ping fan ha scritto:
>> On Tue, Nov 13, 2012 at 6:11 PM, Paolo Bonzini <address@hidden> wrote:
>>>>> Il 05/11/2012 06:38, Liu Ping Fan ha scritto:
>>>>>> From: Liu Ping Fan <address@hidden>
>>>>>>
>>>>>> If out of global lock, we will be challenged by SMP in low level,
>>>>>> so need atomic ops.
>>>>>>
>>>>>> This file is a wrapper of GCC atomic builtin.
>>>>>
>>>>> I still object to this.
>>>>>
>>>>> I know it enforces type-safety, but it is incomplete.  It doesn't
>>>>
>>>> Although it is incomplete, but the rest cases are rarely used.  Linux
>>>> faces such issue, and the "int" version is enough, so I think we can
>>>> borrow experience from there.
>>>
>>> One of the two places that use __sync_* require 64-bit accesses.  My
>> Yes, these two places are not easy to fix.
>
> Which shows that Linux's atomic_t is not suited for QEMU, in my opinion.
>
>>> RCU prototype required pointer-sized access, which you cannot make type-
>> But I think that your RCU prototype should rely on atomic of CPU, not
>> gcc‘s atomic.
>
> What's the difference?  gcc's atomic produces the same instructions as
> hand-written assembly (or should).
>
Probably I made a mistake here, in vhost,  log =
__sync_fetch_and_and(from, 0)  is used to fetch 64bits atomically in
the case  32bits qemu running on 64bits linux.  Right?   But how can
we read 32bits twice in atomic?  Seem that no instruction like "_lock
xchg" for this ops.  So I guess _sync_fetch_and_and() based on
something like spinlock.

And I think the broken issue is caused by vhost thread updates log,
while qemu read out it not atomicly, Right?

>> Otherwise, it could be slow (I guess something like spinlock there).
>
> Not sure what you mean.  I'm using two things: 1) write barriers; 2)
> atomic_xchg of a pointer for fast, wait-free enqueuing of call_rcu
> callbacks.
>
Got your main idea. Will go through your prototype. Just one more
question, why here atomic_xchg needed?  Could not the sequence "read
old pointer, set new pointer" satisfy your requirement?

Regards,
Pingfan

> Paolo