mbox series

[v3,00/32] Overhaul NFSD filecache

Message ID 165730437087.28142.6731645688073512500.stgit@klimt.1015granger.net (mailing list archive)
Headers show
Series Overhaul NFSD filecache | expand

Message

Chuck Lever July 8, 2022, 6:23 p.m. UTC
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

Comments

Jeffrey Layton July 8, 2022, 8:27 p.m. UTC | #1
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>
Chuck Lever July 8, 2022, 8:32 p.m. UTC | #2
> On Jul 8, 2022, at 4:27 PM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> 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).

Thanks for your review! #15 emulates what other list_lru consumers
appear to do, but I'd like to hear from Trond and/or Dave who
advocated for that approach.


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

--
Chuck Lever