On Wed, 4 Dec 2013 00:33:36 -0800 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > On Tue, Dec 03, 2013 at 01:21:12PM -0500, Jeff Layton wrote: > > Most of this code is protected by a single spinlock (cache_lock). The > > main benefit to switching to list_lru would be that we could move to a > > per-node lock. But, to make that worthwhile would mean we'd need to > > redesign the locking and break the cache_lock into multiple locks. > > No need to redo locks just yet, the biggest benefit is to use a well > debug library for lru list handling, with the second biggest one being > that you get out of the box shrinker support. > > > Also, the existing code does take pains to reuse an expired entry off > > the LRU list in preference to allocating a new one. The list_lru code > > doesn't have a mechanism to scrape the first entry off the LRU list, > > though I suppose we could add one or abuse the cb_arg in the walk > > callback as a return pointer. > > That's just because it is an old-school cache with a fixed upper bound > of entries and no shrinker support. With proper shrinker integration > this nasty code isn't needed. Well, the old code just had a fixed number of entries (1024 or so?), which was ridiculously small on modern server. A busy server could blow out the cache in less than the retransmit interval, which made it effectively worthless. The new code has a much larger upper bound, but it is still bounded by the amount of low memory on the host. We have shrinker integration (though its propriety might be in question). Are you suggesting that we shouldn't try to bound the cache at all and should instead just let it grow and rely on the shrinker to keep it in check? -- 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
