Re: [PATCH RFC 1/3] nfsd: don't try to reuse an expired DRC entry off the list

[Jeff Layton <jlayton@redhat.com>]

On Thu, 5 Dec 2013 10:50:24 -0500
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Thu, Dec 05, 2013 at 08:41:56AM -0500, Jeff Layton wrote:
> > On Thu, 5 Dec 2013 05:29:19 -0800
> > Christoph Hellwig <hch@infradead.org> wrote:
> > 
> > > On Thu, Dec 05, 2013 at 06:00:51AM -0500, Jeff Layton wrote:
> > > > Currently when we are processing a request, we try to scrape an expired
> > > > or over-limit entry off the list in preference to allocating a new one
> > > > from the slab.
> > > > 
> > > > This is unnecessarily complicated. Just use the slab layer.
> > > 
> > > I really don't think pruning caches from lookup functions is a good
> > > idea. To many chances for locking inversions, unbounded runtime and
> > > other issues.  So I'd prefer my earlier version of the patch to
> > > remove it entirely if we want to change anything beyond your minimal
> > > fix.
> > > 
> > 
> > It's not likely to hit a locking inversion here since we don't have a
> > lot of different locks, but point taken...
> > 
> > If we take that out, then that does mean that the cache may grow larger
> > than the max...possibly much larger since the pruner workqueue job
> > only runs every 120s and you can put a lot more entries into the cache
> > in that period.
> 
> Yeah, this scares me.
> 
> I looked through my old mail but couldn't find your previous results:
> how long did you find the hash buckets needed to be before the
> difference was noticeable on your setup?
> 
> We typically do a failed-lookup-and-insert on every cached operation so
> I wouldn't be surprised if a workload of small random writes, for
> example, could produce an unreasonably large cache in that time.
> 

I don't think I ever measured that. I did some some measurement of how
long it took to generate checksums, but that's a different phase of
this stuff. I just sort of went with "too long a chain == bad, so keep
them as short as possible".


> 
> > 
> > That said, I don't feel too strongly about it, so if you think leaving
> > it to the workqueue job and the shrinker is the right thing to do, then
> > so be it.
> > 
> > -- 
> > Jeff Layton <jlayton@redhat.com>


-- 
Jeff Layton <jlayton@redhat.com>