Thanks for thinking this through. 
An alternative is using a trylock and assuming a cache miss (or not adding to the cache) if trylock fails. 

That would surely be simpler code. I will benchmark both variants vs the original and let you know my findings
- Daniel


On Sun, Apr 6, 2014 at 8:47 AM, Ken Thomases <ken@codeweavers.com> wrote:

On Apr 5, 2014, at 8:37 PM, Ken Thomases wrote:

> It might be possible to have a safe, lockless version of get_cached_fd() that sometimes produces a false-negative. Then, if it fails, you'd enter the uninterrupted section and try the reliable version before making the server call. Devising such a scheme and convincing us that it's safe and reliable is the hard part. It might be sufficient put a memory barrier before the lookup of the fd in get_cached_fd() and then something like this at the bottom of that block:
>
> if (interlocked_cmpxchg( &fd_cache[entry][idx].fd, fd + 1, fd + 1) != fd + 1)
> fd = -1;
>
> The idea is to verify that the fd has not been replaced or removed after we fetched it and its properties.

It occurred to me that, for this to work, add_fd_to_cache() has to be changed, too. I think the fd has to be temporarily cleared, then the properties have to be stored, and then the fd can be stored. A write memory barrier is required, but is supplied by interlocked_xchg() which is already used.

So, adding looks like:

clear fd
store properties
store fd

Getting looks like:

get fd
get properties
get fd again and compare

If a valid fd was read and it didn't change between the two points, then you can be sure the properties that you read go with the fd.

-Ken