Re: [PATCH 6/9] sunrpc/cache: retry cache lookups that return -ETIMEDOUT

["J. Bruce Fields" <bfields@fieldses.org>]

On Tue, Dec 15, 2009 at 05:27:29PM +1100, Neil Brown wrote:
> On Fri, 4 Dec 2009 20:17:42 -0500
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > > 
> > > How about this as an alternate.  I have only compile tested it, nothing more.
> > > But if it looks good to you I'll make sure it really works.  
> > 
> > Well, without having really thinking about it:
> > 
> > 	- If this were two separate patches, I'd have an easier time
> > 	  sorting out the interesting stuff from the trivial (though
> > 	  nevertheless good) hash-function reshuffling.
> 
> I'll see what I can come up with...

Have you had a chance to get back to this?

> 
> > 	- Adding code to the common lookup_and_check() instead of to
> > 	  every caller certainly seems better, but too bad about the
> > 	  special cases that remain.
> 
> yeah.... I could possibly add a pass-by-reference to lookup_and_check
> which points to a possible cached value, but that would have
> only one user, so the special case would be moved elsewhere...
> ??

Yeah, that doesn't sound so great.

> > 	- Something still seems odd here: we shouldn't ever have
> > 	  duplicate cache entries with the same key, because during
> > 	  their lifetimes cache entries are always kept in the hash.  So
> > 	  why do we need extra code to check for that case?  I may just
> > 	  be forgetting what we're doing here.  Should I go reread the
> > 	  rest of the series?
> 
> When sunrpc_update_cache is called to update and item that is
> already valid, it unhashes that item and creates a new one.
> (The unhashed item disappears once all the refcounts go).
> So if we wait for user-space to update an entry for us, we
> might find out that it has been unhashed, so we need to find
> the new one.

But nobody ever waits on a valid entry, right?  So isn't the only case
we care about the invalid case?  I'll admit I haven't thought this
through.

--b.