Date: Thu, 14 May 2009 14:47:41 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Jeff Roberson <jroberson@jroberson.net> Cc: =?ISO-8859-15?Q?Dag-Erling_Sm=F8rgrav?= <des@des.no>, arch@freebsd.org Subject: Re: lockless file descriptor lookup Message-ID: <20090514131613.T1224@besplex.bde.org> In-Reply-To: <alpine.BSF.2.00.0905121411070.981@desktop> References: <alpine.BSF.2.00.0905111720280.981@desktop> <86bppy60ti.fsf@ds4.des.no> <alpine.BSF.2.00.0905121411070.981@desktop>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 12 May 2009, Jeff Roberson wrote: > On Tue, 12 May 2009, Dag-Erling Sm?rgrav wrote: > >> Jeff Roberson <jroberson@jroberson.net> writes: >>> I'd also appreciate it if someone could look at my volatile cast and >>> make sure I'm actually forcing the compiler to refresh the fd_ofiles >>> array here: >>> >>> + if (fp == ((struct file *volatile*)fdp->fd_ofiles)[fd]) This has 2 style bugs (missing space after first '*' and missing space before second '*'. It isn't clear whether you want to refresh the fd_ofiles pointer to the (first element of) the array, or the fd'th element. It is clear that you don't want to refresh the whole array. The above refreshes the fd'th element. Strangely, in my tests gcc refreshes the fd'th element even without the cast. E.g., test(fdp->fd_ofiles[fd], fdp->fd_ofiles[fd]); results in 1 memory access for each of the [fd]'s. >> The problem is that since it is not declared as volatile, some other >> piece of code may have modified it but not yet flushed it to RAM. > > That is an acceptable race due to other guarantees. If it hasn't been > committed to memory yet, the old table still contains valid data. I only > need to be certain that the compiler doesn't cache the original ofiles value. > It can't anyway because atomics use inline assembly on all platforms but I'd > like it to be explicit anyway. It shouldn't matter that atomics use inline asm. Non-broken inline asm declares all its inputs and outputs, so compilers can see what it changes just as easily as for C code (and more easily than for non- inline asm or C). Anyway, you probably need atomics that have suitable memory barriers. Memory barriers must affect the compiler and make it perform refreshes for them to work, so you shouldn't need any volatile casts. E.g., all atomic store operations (including cmpset) have release semantics even if they aren't spelled with "_rel" or implemented using inline asm. On amd64 and i386, they happen to be implemented using inline asm with "memory" clobbers. The "memory" clobbers force refreshes of all non-local variables. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090514131613.T1224>