From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH 8/8] evtchn: add FIFO-based event channel hypercalls and port ops Date: Wed, 20 Mar 2013 15:54:10 +0000 Message-ID: <5149DBA2.8030701@citrix.com> References: <1363726818-25409-1-git-send-email-david.vrabel@citrix.com> <1363726818-25409-9-git-send-email-david.vrabel@citrix.com> <5149A1E602000078000C71C1@nat28.tlf.novell.com> <5149BCBA.4060106@citrix.com> <5149CDC802000078000C72EB@nat28.tlf.novell.com> <20130320142358.GA95617@ocelot.phlegethon.org> <5149C9DC.3050806@citrix.com> <20130320153420.GA95746@ocelot.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130320153420.GA95746@ocelot.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: Wei Liu , "xen-devel@lists.xen.org" , "Keir (Xen.org)" , Jan Beulich , Konrad Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org 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 wrote: >>>>> On 20/03/13 10:47, Jan Beulich wrote: >>>>>>>>> On 19.03.13 at 22:00, David Vrabel 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