Re: [PATCH v6 00/14] Series short description

From: "NeilBrown" <neilb@suse.de>
To: "Chuck Lever" <chuck.lever@oracle.com>
Cc: linux-nfs@vger.kernel.org, jlayton@redhat.com
Subject: Re: [PATCH v6 00/14] Series short description
Date: Fri, 28 Oct 2022 10:29:07 +1100	[thread overview]
Message-ID: <166691334702.13915.10147957275207365905@noble.neil.brown.name> (raw)
In-Reply-To: <166689625728.90991.15067635142973595248.stgit@klimt.1015granger.net>

On Fri, 28 Oct 2022, Chuck Lever wrote:
> I'm proposing this series for v6.2 (for-next).
> 
> For quite some time, we've been encouraged to disable filecache
> garbage collection for NFSv4 files, and I think I found a surgical
> way to do just that. That is presented in "NFSD: Add an NFSD_FILE_GC
> flag to enable nfsd_file garbage collection".
> 
> The other major change in this set is reworking the file_hashtbl to
> resize itself dynamically. This reduces the average size of its
> bucket chains, greatly speeding up hash insertion, which holds the
> state_lock.
> 
> This version seems to pass thread-intensive testing so far.
> 
> Comments and opinions are welcome.

All looks good to me - with the understanding that refcount fixes will
follow.
I cannot comment on the tracepoint changes as I'm not particularly
familiar with that code, but for everything else
  Reviewed-by: NeilBrown <neilb@suse.de>

Thanks,
NeilBrown

> 
> Changes since v5:
> - Wrap hash insertion with inode->i_lock
> - Replace hashfn and friends with in-built rhashtable functions
> - Add a tracepoint to report delegation return
> 
> Changes since v4:
> - Addressed some comments in the GC patch; more to come
> - Split clean-ups out of the rhashtable patch, reordered the series
> - Removed state_lock from the rhashtable helpers
> 
> Changes since v3:
> - the new filehandle alias check was still not right
> 
> Changes since v2:
> - Converted nfs4_file_rhashtbl to nfs4_file_rhltable
> - Addressed most or all other review comments
> 
> Changes since RFC:
> - checking nfs4_files for inode aliases is now done only on hash
>   insertion
> - the nfs4_file reference count is now bumped only while the RCU
>   read lock is held
> - comments and function names have been revised and clarified
> 
> ---
> 
> Chuck Lever (14):
>       NFSD: Pass the target nfsd_file to nfsd_commit()
>       NFSD: Revert "NFSD: NFSv4 CLOSE should release an nfsd_file immediately"
>       NFSD: Add an NFSD_FILE_GC flag to enable nfsd_file garbage collection
>       NFSD: Clean up nfs4_preprocess_stateid_op() call sites
>       NFSD: Trace stateids returned via DELEGRETURN
>       NFSD: Trace delegation revocations
>       NFSD: Use const pointers as parameters to fh_ helpers
>       NFSD: Update file_hashtbl() helpers
>       NFSD: Clean up nfsd4_init_file()
>       NFSD: Add a remove_nfs4_file() helper
>       NFSD: Clean up find_or_add_file()
>       NFSD: Refactor find_file()
>       NFSD: Allocate an rhashtable for nfs4_file objects
>       NFSD: Use rhashtable for managing nfs4_file objects
> 
> 
>  fs/nfsd/filecache.c |  81 +++++++++++++++-------
>  fs/nfsd/filecache.h |   4 +-
>  fs/nfsd/nfs3proc.c  |  10 ++-
>  fs/nfsd/nfs4proc.c  |  42 +++++------
>  fs/nfsd/nfs4state.c | 165 ++++++++++++++++++++++++--------------------
>  fs/nfsd/nfsfh.h     |  10 +--
>  fs/nfsd/state.h     |   5 +-
>  fs/nfsd/trace.h     |  59 +++++++++++++++-
>  fs/nfsd/vfs.c       |  19 ++---
>  fs/nfsd/vfs.h       |   3 +-
>  10 files changed, 249 insertions(+), 149 deletions(-)
> 
> --
> Chuck Lever
> 
> 