On Thu, 09 Jan 2025, Jeff Layton wrote: > On Wed, 2025-01-08 at 10:03 +1100, NeilBrown wrote: > > The nfsd filecache currently uses list_lru for tracking files recently > > used in NFSv3 requests which need to be "garbage collected" when they > > have becoming idle - unused for 2-4 seconds. > > > > I do not believe list_lru is a good tool for this. It does no allow the > > timeout which filecache requires so we have to add a timeout mechanism > > which holds the list_lru for while the whole list is scanned looking for > > entries that haven't been recently accessed. When the list is largish > > (even a few hundred) this can block new requests which need the lock to > > remove a file to access it. > > > > Wait, what? The whole point of an LRU list is that the least-recently- > used entries are at the head of the list. We should only need to walk > down to the first entry that can't be reaped and then stop there. Why > is it walking the whole list? A "list_lru" is not just one list, it is one list per numa node. The "normal" use is that each numa node removes a few things from the head in the shrinker. Apart from nfsd, users only walk the whole list when shutting down the list. We need to visit every node in the list so we can clear the REFERENCED bit on every entry we decide to leave in the list. > > > > We discard the nf_gc linkage in an nfsd_file and only use nf_lru. > > We discard NFSD_FILE_REFERENCED. > > nfsd_file_close_inode_sync() included a copy of > > nfsd_file_dispose_list(). This has been change to call that function > > instead. > > > > Possibly this patch could be broken into a few smaller patches. > > > > Yeah, that would help. > I'm currently on leave until 20th Jan. I'll do that when I get back - if I don't find time before. Thanks, NeilBrown