From owner-freebsd-hackers@FreeBSD.ORG Tue Sep 29 21:39:46 2009 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 727DE106568D; Tue, 29 Sep 2009 21:39:46 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-bw0-f227.google.com (mail-bw0-f227.google.com [209.85.218.227]) by mx1.freebsd.org (Postfix) with ESMTP id 6704F8FC27; Tue, 29 Sep 2009 21:39:44 +0000 (UTC) Received: by bwz27 with SMTP id 27so4233101bwz.43 for ; Tue, 29 Sep 2009 14:39:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:received:in-reply-to :references:date:x-google-sender-auth:message-id:subject:from:to:cc :content-type; bh=Z9+Lo2TpUs+rG5Wl+UvI0jFs+8AzBuEjq93tt+kOjw0=; b=Dk7uRND3UAfSI1OshiWjgly/9NNZa7lrz660uzy6oGd4LKH2QCo806RCimQuQM5xLA RtyAhylEfpqlSFje/X27YCz2gtYY9R92LT/MhorxZTsskWIe5DqDJ08Srn7ZT2UdT5xQ xpS/rX8bp8wci6i4x5P9XaPUDsT9up8xDMM7M= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; b=OOtQamT5RVTzkeyZnohiGMsc4yNglPAQ4IRDlcqJmmH3wBqXgk1u3zmVombZeVUn0Q 0cHQ0Yg6sLKFUiBlinhw8oKghz7nvO5TR4vncjH4JM+ytoD/bZTpcXDllIe06sRoONoR P89rObxgVCIggfrTeAcrkelrvcrcj8uh/cuR0= MIME-Version: 1.0 Sender: asmrookie@gmail.com Received: by 10.223.60.68 with SMTP id o4mr1437433fah.102.1254260383410; Tue, 29 Sep 2009 14:39:43 -0700 (PDT) In-Reply-To: <200909291731.32394.jhb@freebsd.org> References: <20090924224935.GW473@gandalf.sssup.it> <200909291953.36373.max@love2party.net> <3bbf2fe10909291342o4d32e381ge23e446582bb2d18@mail.gmail.com> <200909291731.32394.jhb@freebsd.org> Date: Tue, 29 Sep 2009 23:39:43 +0200 X-Google-Sender-Auth: fb3e3246b4db98a6 Message-ID: <3bbf2fe10909291439x21f53e34n60d63554b1dea0de@mail.gmail.com> From: Attilio Rao To: John Baldwin Content-Type: text/plain; charset=UTF-8 Cc: Ed Schouten , Max Laier , Roman Divacky , Fabio Checconi , freebsd-hackers@freebsd.org Subject: Re: sx locks and memory barriers X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Sep 2009 21:39:46 -0000 2009/9/29 John Baldwin : > On Tuesday 29 September 2009 4:42:13 pm Attilio Rao wrote: >> 2009/9/29 Max Laier : >> > On Tuesday 29 September 2009 17:39:37 Attilio Rao wrote: >> >> 2009/9/25 Fabio Checconi : >> >> > Hi all, >> >> > looking at sys/sx.h I have some troubles understanding this comment: >> >> > >> >> > * A note about memory barriers. Exclusive locks need to use the same >> >> > * memory barriers as mutexes: _acq when acquiring an exclusive lock >> >> > * and _rel when releasing an exclusive lock. On the other side, >> >> > * shared lock needs to use an _acq barrier when acquiring the lock >> >> > * but, since they don't update any locked data, no memory barrier is >> >> > * needed when releasing a shared lock. >> >> > >> >> > In particular, I'm not understanding what prevents the following sequence >> >> > from happening: >> >> > >> >> > CPU A CPU B >> >> > >> >> > sx_slock(&data->lock); >> >> > >> >> > sx_sunlock(&data->lock); >> >> > >> >> > /* reordered after the unlock >> >> > by the cpu */ >> >> > if (data->buffer) >> >> > sx_xlock(&data->lock); >> >> > free(data->buffer); >> >> > data->buffer = NULL; >> >> > sx_xunlock(&data->lock); >> >> > >> >> > a = *data->buffer; >> >> > >> >> > IOW, even if readers do not modify the data protected by the lock, >> >> > without a release barrier a memory access may leak past the unlock (as >> >> > the cpu won't notice any dependency between the unlock and the fetch, >> >> > feeling free to reorder them), thus potentially racing with an exclusive >> >> > writer accessing the data. >> >> > >> >> > On architectures where atomic ops serialize memory accesses this would >> >> > never happen, otherwise the sequence above seems possible; am I missing >> >> > something? >> >> >> >> I think your concerns are right, possibly we need this patch: >> >> http://www.freebsd.org/~attilio/sxrw_unlockb.diff >> >> >> >> However speaking with John we agreed possibly there is a more serious >> >> breakage. Possibly, memory barriers would also require to ensure the >> >> compiler to not reorder the operation, while right now, in FreeBSD, they >> >> just take care of the reordering from the architecture perspective. >> >> The only way I'm aware of GCC offers that is to clobber memory. >> >> I will provide a patch that address this soon, hoping that GCC will be >> >> smart enough to not overhead too much the memory clobbering but just >> >> try to understand what's our purpose and servers it (I will try to >> >> compare code generated before and after the patch at least for tier-1 >> >> architectures). >> > >> > Does GCC really reorder accesses to volatile objects? The C Standard seems to >> > object: >> > >> > 5.1.2.3 - 2 >> > Accessing a volatile object, modifying an object, modifying a file, or calling >> > a function that does any of those operations are all side effects,11) which >> > are changes in the state of the execution environment. Evaluation of an >> > expression may produce side effects. At certain specified points in the >> > execution sequence called sequence points, all side effects of previous >> > evaluations shall be complete and no side effects of subsequent evaluations >> > shall have taken place. (A summary of the sequence points is given in annex >> > C.) >> >> Very interesting. >> I was thinking about the other operating systems which basically do >> 'memory clobbering' for ensuring a compiler barrier, but actually they >> often forsee such a barrier without the conjuction of a memory >> operand. >> >> I think I will need to speak a bit with a GCC engineer in order to see >> what do they implement in regard of volatile operands. > > GCC can be quite aggressive with reordering even in the face of volatile. I > was recently doing a hack to export some data from the kernel to userland > that used a spin loop to grab a snapshot of the contents of a structure > similar to the method used in the kernel with the timehands structures. It > used a volatile structure exposed from the kernel that looked something > like: > > struct foo { > volatile int gen; > /* other stuff */ > }; > > volatile struct foo *p; > > do { > x = p->gen; > /* read other stuff */ > y = p->gen; > } while (x != y && x != 0); > > GCC moved the 'y = ' up into the middle of the '/* read other stuff */'. > I eventually had to add explicit "memory" clobbers to force GCC to not > move the reads of 'gen' around but do them "around" all the other > operations, so that the working code is: > > do { > x = p->gen; > asm volatile("" ::: "memory"); > /* read other stuff */ > asm volatile("" ::: "memory"); > y = p->gen; > } while (x != y && x != 0); > I see. So probabilly clobbering memory is the only choice we have right now. I will try to make a patch which also keeps into account the possibility to skip it (or define by hand alternative approaches) for different compilers. I wonder, specifically, how llvm/clang relies with it. Attilio -- Peace can only be achieved by understanding - A. Einstein