Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
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
In-reply-to: <166689625728.90991.15067635142973595248.stgit@klimt.1015granger.net>
References: <166689625728.90991.15067635142973595248.stgit@klimt.1015granger.net>
Date:   Fri, 28 Oct 2022 10:29:07 +1100
Message-id: <166691334702.13915.10147957275207365905@noble.neil.brown.name>
Precedence: bulk

On Fri, 28 Oct 2022, Chuck Lever wrote:
> I'm proposing this series for v6.2 (for-next).
>=20
> 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".
>=20
> 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.
>=20
> This version seems to pass thread-intensive testing so far.
>=20
> 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


>=20
> 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
>=20
> 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
>=20
> Changes since v3:
> - the new filehandle alias check was still not right
>=20
> Changes since v2:
> - Converted nfs4_file_rhashtbl to nfs4_file_rhltable
> - Addressed most or all other review comments
>=20
> 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
>=20
> ---
>=20
> Chuck Lever (14):
>       NFSD: Pass the target nfsd_file to nfsd_commit()
>       NFSD: Revert "NFSD: NFSv4 CLOSE should release an nfsd_file immediate=
ly"
>       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
>=20
>=20
>  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(-)
>=20
> --
> Chuck Lever
>=20
>=20