> On Nov 4, 2022, at 8:44 AM, Jeff Layton <jlayton@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, 2022-11-03 at 16:22 -0400, Chuck Lever wrote:
>> Now that we have trace_nfsd_file_closing, all we really need to
>> capture is when an external caller has requested a close/sync.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> fs/nfsd/filecache.c | 17 ++++++-----------
>> fs/nfsd/trace.h | 45 ++++++++++++++++-----------------------------
>> 2 files changed, 22 insertions(+), 40 deletions(-)
>>
>> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
>> index cf1a8f1d1349..7be62af4bfb7 100644
>> --- a/fs/nfsd/filecache.c
>> +++ b/fs/nfsd/filecache.c
>> @@ -706,14 +706,13 @@ static struct shrinker nfsd_file_shrinker = {
>> * The nfsd_file objects on the list will be unhashed, and each will have a
>> * reference taken.
>> */
>> -static unsigned int
>> +static void
>> __nfsd_file_close_inode(struct inode *inode, struct list_head *dispose)
>> {
>> struct nfsd_file_lookup_key key = {
>> .type = NFSD_FILE_KEY_INODE,
>> .inode = inode,
>> };
>> - unsigned int count = 0;
>> struct nfsd_file *nf;
>>
>> rcu_read_lock();
>> @@ -723,11 +722,9 @@ __nfsd_file_close_inode(struct inode *inode, struct list_head *dispose)
>> if (!nf)
>> break;
>>
>> - if (nfsd_file_unhash_and_queue(nf, dispose))
>> - count++;
>> + nfsd_file_unhash_and_queue(nf, dispose);
>> } while (1);
>> rcu_read_unlock();
>> - return count;
>> }
>>
>> /**
>> @@ -742,11 +739,9 @@ static void
>> nfsd_file_close_inode(struct inode *inode)
>> {
>> struct nfsd_file *nf, *tmp;
>> - unsigned int count;
>> LIST_HEAD(dispose);
>>
>> - count = __nfsd_file_close_inode(inode, &dispose);
>> - trace_nfsd_file_close_inode(inode, count);
>> + __nfsd_file_close_inode(inode, &dispose);
>> list_for_each_entry_safe(nf, tmp, &dispose, nf_lru) {
>> trace_nfsd_file_closing(nf);
>> if (!refcount_dec_and_test(&nf->nf_ref))
>> @@ -765,11 +760,11 @@ void
>> nfsd_file_close_inode_sync(struct inode *inode)
>> {
>> struct nfsd_file *nf;
>> - unsigned int count;
>> LIST_HEAD(dispose);
>>
>> - count = __nfsd_file_close_inode(inode, &dispose);
>> - trace_nfsd_file_close_inode(inode, count);
>> + trace_nfsd_file_close(inode);
>
> With this change we lose the count of entries on the list. That info is
> not particularly helpful, but maybe we ought to consider a tracepoint in
> unhash_and_queue that records whether a file we found in the hash got
> queued? It might be nice to have a way to detect cases where we close a
> nfsd_file but the refcount was >1 or 0, so we don't end up queueing it
> to the list.
Would that be an exception case, or an error? I'm assuming the former.
No objection if you'd like to add that as part of a follow-on series.
>> +
>> + __nfsd_file_close_inode(inode, &dispose);
>> while (!list_empty(&dispose)) {
>> nf = list_first_entry(&dispose, struct nfsd_file, nf_lru);
>> list_del_init(&nf->nf_lru);
>> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
>> index e41007807b7e..ef01ecd3eec6 100644
>> --- a/fs/nfsd/trace.h
>> +++ b/fs/nfsd/trace.h
>> @@ -1099,35 +1099,6 @@ TRACE_EVENT(nfsd_file_open,
>> __entry->nf_file)
>> )
>>
>> -DECLARE_EVENT_CLASS(nfsd_file_search_class,
>> - TP_PROTO(
>> - const struct inode *inode,
>> - unsigned int count
>> - ),
>> - TP_ARGS(inode, count),
>> - TP_STRUCT__entry(
>> - __field(const struct inode *, inode)
>> - __field(unsigned int, count)
>> - ),
>> - TP_fast_assign(
>> - __entry->inode = inode;
>> - __entry->count = count;
>> - ),
>> - TP_printk("inode=%p count=%u",
>> - __entry->inode, __entry->count)
>> -);
>> -
>> -#define DEFINE_NFSD_FILE_SEARCH_EVENT(name) \
>> -DEFINE_EVENT(nfsd_file_search_class, name, \
>> - TP_PROTO( \
>> - const struct inode *inode, \
>> - unsigned int count \
>> - ), \
>> - TP_ARGS(inode, count))
>> -
>> -DEFINE_NFSD_FILE_SEARCH_EVENT(nfsd_file_close_inode_sync);
>> -DEFINE_NFSD_FILE_SEARCH_EVENT(nfsd_file_close_inode);
>> -
>> TRACE_EVENT(nfsd_file_is_cached,
>> TP_PROTO(
>> const struct inode *inode,
>> @@ -1238,6 +1209,22 @@ DEFINE_EVENT(nfsd_file_lruwalk_class, name, \
>> DEFINE_NFSD_FILE_LRUWALK_EVENT(nfsd_file_gc_removed);
>> DEFINE_NFSD_FILE_LRUWALK_EVENT(nfsd_file_shrinker_removed);
>>
>> +TRACE_EVENT(nfsd_file_close,
>> + TP_PROTO(
>> + const struct inode *inode
>> + ),
>> + TP_ARGS(inode),
>> + TP_STRUCT__entry(
>> + __field(const void *, inode)
>> + ),
>> + TP_fast_assign(
>> + __entry->inode = inode;
>> + ),
>> + TP_printk("inode=%p",
>> + __entry->inode
>> + )
>> +);
>> +
>> TRACE_EVENT(nfsd_file_fsync,
>> TP_PROTO(
>> const struct nfsd_file *nf,
>>
>>
>
> --
> Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
Chuck Lever
