Re: [PATCH v3 00/32] Overhaul NFSD filecache

From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever <chuck.lever@oracle.com>,
	linux-nfs@vger.kernel.org, netdev@vger.kernel.org
Cc: david@fromorbit.com, tgraf@suug.ch
Subject: Re: [PATCH v3 00/32] Overhaul NFSD filecache
Date: Fri, 08 Jul 2022 16:27:55 -0400	[thread overview]
Message-ID: <6927644b409f92ed0622bb0e8a677b8d8007756d.camel@kernel.org> (raw)
In-Reply-To: <165730437087.28142.6731645688073512500.stgit@klimt.1015granger.net>

On Fri, 2022-07-08 at 14:23 -0400, Chuck Lever wrote:
> This series overhauls the NFSD filecache, a cache of server-side
> "struct file" objects recently used by NFS clients. The purposes of
> this overhaul are an immediate improvement in cache scalability in
> the number of open files, and preparation for further improvements.
> 
> There are three categories of patches in this series:
> 
> 1. Add observability of cache operation so we can see what we're
> doing as changes are made to the code.
> 
> 2. Improve the scalability of filecache garbage collection,
> addressing several bugs along the way.
> 
> 3. Improve the scalability of the filecache hash table by converting
> it to use rhashtable.
> 
> These patches are available in the for-next branch of:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git 
> 
> Changes since v2:
> - Fix a cred use-after-free crasher
> - Fix a Smatch error reported by Dan Carpenter
> - Replace dereferences of nfsd_file::nf_inode
> - Further clean-ups and white-space adjustments
> 
> Changes since RFC:
> - Fixed several crashers
> - Adjusted some of the new observability
> - Tests with generic/531 now pass
> - Fixed bugzilla 387 too, maybe
> - Plenty of clean-ups
> 
> ---
> 
> Chuck Lever (32):
>       NFSD: Demote a WARN to a pr_warn()
>       NFSD: Report filecache LRU size
>       NFSD: Report count of calls to nfsd_file_acquire()
>       NFSD: Report count of freed filecache items
>       NFSD: Report average age of filecache items
>       NFSD: Add nfsd_file_lru_dispose_list() helper
>       NFSD: Refactor nfsd_file_gc()
>       NFSD: Refactor nfsd_file_lru_scan()
>       NFSD: Report the number of items evicted by the LRU walk
>       NFSD: Record number of flush calls
>       NFSD: Zero counters when the filecache is re-initialized
>       NFSD: Hook up the filecache stat file
>       NFSD: WARN when freeing an item still linked via nf_lru
>       NFSD: Trace filecache LRU activity
>       NFSD: Leave open files out of the filecache LRU
>       NFSD: Fix the filecache LRU shrinker
>       NFSD: Never call nfsd_file_gc() in foreground paths
>       NFSD: No longer record nf_hashval in the trace log
>       NFSD: Remove lockdep assertion from unhash_and_release_locked()
>       NFSD: nfsd_file_unhash can compute hashval from nf->nf_inode
>       NFSD: Refactor __nfsd_file_close_inode()
>       NFSD: nfsd_file_hash_remove can compute hashval
>       NFSD: Remove nfsd_file::nf_hashval
>       NFSD: Replace the "init once" mechanism
>       NFSD: Set up an rhashtable for the filecache
>       NFSD: Convert the filecache to use rhashtable
>       NFSD: Clean up unused code after rhashtable conversion
>       NFSD: Separate tracepoints for acquire and create
>       NFSD: Move nfsd_file_trace_alloc() tracepoint
>       NFSD: Update the nfsd_file_fsnotify_handle_event() tracepoint
>       NFSD: NFSv4 CLOSE should release an nfsd_file immediately
>       NFSD: Ensure nf_inode is never dereferenced
> 
> 
>  fs/nfsd/filecache.c       | 727 ++++++++++++++++++++++++--------------
>  fs/nfsd/filecache.h       |   7 +-
>  fs/nfsd/nfs4proc.c        |   6 +-
>  fs/nfsd/nfs4state.c       |   7 +-
>  fs/nfsd/nfsctl.c          |  10 +
>  fs/nfsd/trace.h           | 300 +++++++++++++---
>  include/trace/events/fs.h |  37 ++
>  7 files changed, 774 insertions(+), 320 deletions(-)
> 
> --
> Chuck Lever
> 

Nice work, Chuck!

You can add this to all but #15 (where I still have a question about
whether adding it to the LRU on every put is the right thing to do).

Reviewed-by: Jeff Layton <jlayton@kernel.org>