From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH 8/8] evtchn: add FIFO-based event channel hypercalls and port ops Date: Wed, 20 Mar 2013 15:34:20 +0000 Message-ID: <20130320153420.GA95746@ocelot.phlegethon.org> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <5149C9DC.3050806@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Vrabel Cc: Wei Liu , "xen-devel@lists.xen.org" , "Keir (Xen.org)" , Jan Beulich , Konrad Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org 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. :) Tim.