Re: [PATCH 8/8] evtchn: add FIFO-based event channel hypercalls and port ops

[David Vrabel <david.vrabel@citrix.com>]

On 20/03/13 15:34, Tim Deegan wrote:
> At 14:38 +0000 on 20 Mar (1363790300), David Vrabel wrote:
>> On 20/03/13 14:23, Tim Deegan wrote:
>>> At 13:55 +0000 on 20 Mar (1363787704), Jan Beulich wrote:
>>>>>>> On 20.03.13 at 14:42, David Vrabel <david.vrabel@citrix.com> wrote:
>>>>> On 20/03/13 10:47, Jan Beulich wrote:
>>>>>>>>> On 19.03.13 at 22:00, David Vrabel <david.vrabel@citrix.com> wrote:
>>>>>>> +struct evtchn_fifo_queue {
>>>>>>> +    volatile uint32_t *head; /* points into control block */
>>>>>>
>>>>>> I still think that explicit barriers are the way to go. Unless Linux'es
>>>>>> view on this has changed, you'll have issues getting the Linux folks
>>>>>> to accept this.
>>>>>
>>>>> This volatile can just be removed.  head is only written by Xen in one
>>>>> place and it is immediately followed by a spin_unlock() which is a barrier.
>>>>
>>>> A release type barrier only, but that presumably is sufficient for
>>>> the purposes here.
>>>
>>> You might have to use an atomic_t or similar if the consumer might be
>>> confused by partial updates.
>>
>> I have assumed that reads and writes to 32-bit words are atomic on all
>> interesting architectures.
> 
> True, but unless you explicitly tell it to, the compiler isn't required
> to update a 32-bit variable using 32-bit operations, or to avoid
> weird-looking intermediate values.  It seems unlikely that it would do
> anything other than straightforwardly write the new value, but we've seen
> compilers do some unlikely things. :)

Ok.  Looks like I need to use write_atomic() to update the head in Xen.

David