lockless file descriptor lookup
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).
More information about the freebsd-arch