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.
> +
> + __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>
