Skip site navigation (1)Skip section navigation (2)
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>