Re: [PATCH 2/4] nfsd: NFSD_FILE_KEY_INODE only needs to find GC'ed entries
From: Chuck Lever III <chuck.lever@oracle.com>
Date: 2023-01-05 14:19:02
Hi Jeff-
On Jan 5, 2023, at 7:15 AM, Jeff Layton [off-list ref] 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.
quoted hunk ↗ jump to hunk
Signed-off-by: Jeff Layton <jlayton@kernel.org> --- 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 voidnfsd_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 @@ boolnfsd_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