Avi Kivity wrote: > On 09/13/2012 09:54 AM, liu ping fan wrote: > > >>> +typedef struct Atomic { > >>> + int counter; > >>> +} Atomic; > >> > >> Best to mark counter 'volatile'. > >> > >>> + > >>> +static inline void atomic_set(Atomic *v, int i) > >>> +{ > >>> + v->counter = i; > >>> +} > >>> + > >>> +static inline int atomic_read(Atomic *v) > >>> +{ > >>> + return v->counter; > >>> +} > >>> > >> > >> So these two operations don't get mangled by the optimizer. > >> > > Browsing linux code and reading lkml, find some similar material. But > > they have moved volatile from ->counter to function - atomic_read(). > > As to atomic_read(), I think it need to prevent optimizer from > > refetching issue, but as to atomic_set(), do we need ? > > I think so, to prevent reordering.
Hi, I don't think volatile makes any difference to reordering here. The compiler is not going to move the atomic_set() store before or after another instruction on the same atomic variable anyway, just like it wouldn't do that for an ordinary assignment. If you're concerned about ordering with respect to other memory, then volatile wouldn't make much difference. barrier() before and after would. If you're copying Linux's semantics, Linux's atomic_set() doesn't include any barriers, nor imply any. atomic_read() uses volatile to ensure that each call re-reads the value, for example in a loop. (Same as ACCESS_ONCE()). If there was a call to atomic_set() in a loop, it doesn't guarantee that will be written each time. -- Jamie