lockless file descriptor lookup

Bruce Evans brde at optusnet.com.au
Thu May 21 08:03:27 UTC 2009


On Wed, 20 May 2009, Jeff Roberson wrote:

> On Thu, 14 May 2009, Bruce Evans wrote:
>
>> On Tue, 12 May 2009, Jeff Roberson wrote:
>> 
>>> On Tue, 12 May 2009, Dag-Erling Sm?rgrav wrote:
>>> 
>>>> Jeff Roberson <jroberson at 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.,
>
> This is actually intended to catch cases where the descriptor array has 
> expanded and the pointer to fd_ofiles has changed, or the file has been 
> closed and the pointer at the fd'th element has changed.  I'm attempting to 
> force the compiler to reload the fd_ofiles array pointer from the fdp 
> structure.  If it has done that, it can not have the fd'th element cached and 
> so that must be a fresh memory reference.

So you want to refresh both (the array element implicitly from the pointer).
The above cast is clearly no use for refreshing fdp->fd_ofiles, since its
type is that of fdp_ofiles (modulo a '*' or two), while to affect
fdp->fd_ofiles it would need to make (at least the fd_ofile part of) (*fdp)
volatile, and for that it would need to have the type of fdp
(modulo a '*' or two), which is quite different (struct filedesc instead
of struct file).  It is simplest to make all of (*fdp) volatile.  The cast
for that is (I think) (volatile struct filedesc *)fdp (normal spelling)
or (struct filedesc volatile *)fdp (better spelling).

Continued in my reply to jhb's reply (on use of atomic instructions/barriers
-- we should be able to drop the volatile cast instead of fixing it as above,
but should be more careful about the barriers).

Bruce


More information about the freebsd-arch mailing list