Hi Jeff-
> On Jan 5, 2023, at 7:15 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> Since v4 files are expected to be long-lived, there's little value in
> closing them out of the cache when there is conflicting access.
Seems like targeting long-lived nfsd_file items could actually
be a hazardous behavior. Are you sure it's safe to leave it in
stable kernels?
> Rename NFSD_FILE_KEY_INODE to NFSD_FILE_KEY_INODE_GC,
I'd prefer to keep the name as it is. It's for searching for
inodes, and adding the ".gc = true" to the search criteria is
enough to show what you are looking for.
> and change the
> comparator to also match the gc value in the key. Change both of the
> current users of that key to set the gc value in the key to "true".
>
> Also, test_bit returns bool, AFAICT, so I think we're ok to drop the
> !! from the condition later in the comparator.
And I'd prefer that you leave this clean up for another patch.
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/nfsd/filecache.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index 9fff1fa09d08..a67b22579c6e 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -78,7 +78,7 @@ static struct rhashtable nfsd_file_rhash_tbl
> ____cacheline_aligned_in_smp;
>
> enum nfsd_file_lookup_type {
> - NFSD_FILE_KEY_INODE,
> + NFSD_FILE_KEY_INODE_GC,
> NFSD_FILE_KEY_FULL,
> };
>
> @@ -174,7 +174,9 @@ static int nfsd_file_obj_cmpfn(struct rhashtable_compare_arg *arg,
> const struct nfsd_file *nf = ptr;
>
> switch (key->type) {
> - case NFSD_FILE_KEY_INODE:
> + case NFSD_FILE_KEY_INODE_GC:
> + if (test_bit(NFSD_FILE_GC, &nf->nf_flags) != key->gc)
> + return 1;
> if (nf->nf_inode != key->inode)
> return 1;
> break;
> @@ -187,7 +189,7 @@ static int nfsd_file_obj_cmpfn(struct rhashtable_compare_arg *arg,
> return 1;
> if (!nfsd_match_cred(nf->nf_cred, key->cred))
> return 1;
> - if (!!test_bit(NFSD_FILE_GC, &nf->nf_flags) != key->gc)
> + if (test_bit(NFSD_FILE_GC, &nf->nf_flags) != key->gc)
> return 1;
> if (test_bit(NFSD_FILE_HASHED, &nf->nf_flags) == 0)
> return 1;
> @@ -681,8 +683,9 @@ static void
> nfsd_file_queue_for_close(struct inode *inode, struct list_head *dispose)
> {
> struct nfsd_file_lookup_key key = {
> - .type = NFSD_FILE_KEY_INODE,
> + .type = NFSD_FILE_KEY_INODE_GC,
> .inode = inode,
> + .gc = true,
> };
> struct nfsd_file *nf;
>
> @@ -1057,8 +1060,9 @@ bool
> nfsd_file_is_cached(struct inode *inode)
> {
> struct nfsd_file_lookup_key key = {
> - .type = NFSD_FILE_KEY_INODE,
> + .type = NFSD_FILE_KEY_INODE_GC,
> .inode = inode,
> + .gc = true,
> };
> bool ret = false;
>
> --
> 2.39.0
>
--
Chuck Lever
