On Thu, 5 Dec 2013 10:50:24 -0500 "J. Bruce Fields" <bfields@xxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxx> -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
