[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 1/2] add a header file for atomic operations

From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH v2 1/2] add a header file for atomic operations
Date: Tue, 18 Jun 2013 17:04:13 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6

Il 18/06/2013 16:38, Richard Henderson ha scritto:
>>>> +#ifndef atomic_read
>>>> +#define atomic_read(ptr)       (*(__typeof__(*ptr) *volatile) (ptr))
>>>>  #endif
>>>> +#ifndef atomic_set
>>>> +#define atomic_set(ptr, i)     ((*(__typeof__(*ptr) *volatile) (ptr)) = 
>>>> (i))
>>>> +#endif
>>> Use
>>> __atomic_load(..., __ATOMIC_RELAXED)
>>> __atomic_store(..., __ATOMIC_RELAXED)
>>> ?
>> Same here, I didn't want proliferation of #ifdefs beyond what is actually 
>> required.
> Not knowing exactly where these might be used within the code base, I'd be
> worried about someone applying them to a uint64_t, somewhere a 32-bit host
> might see it.  At which point the above is going to be silently wrong, loaded
> with two 32-bit pieces.
> Given that we're not requiring gcc 4.8, and cannot guarantee use of
> __atomic_load, perhaps we ought to do something like
> #define atomic_read(ptr) \
>   ({ if (sizeof(*(ptr)) > sizeof(ptr)) invalid_atomic_read(); \
>      *(__typeof__(*ptr) *volatile) (ptr)); })
> which should generate a link error when reading a size we can't guarantee will
> Just Work.

Good idea.

>> The FAQ also has an "important note", however:
>>     Important Note: Note that it is important for both threads to access
>>     the same volatile variable in order to properly set up the happens-before
>>     relationship. It is not the case that everything visible to thread A
>>     when it writes volatile field f becomes visible to thread B after it
>>     reads volatile field g. The release and acquire have to "match" (i.e.,
>>     be performed on the same volatile field) to have the right semantics. 
>> Is this final "important note" the difference between ACQ_REL and SEQ_CST?
> Yes, exactly.

I still have some confusion, so I sent another follow-up with the exact
differences in the generated code.  But in any case, I will augment the
documentation with the text from the FAQ, seems safe.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]