> On Jul 29, 2022, at 12:47 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/nfsd/trace.h | 16 ++++++++++++++++
> fs/nfsd/vfs.c | 2 ++
> 2 files changed, 18 insertions(+)
>
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 96bb6629541e..38fb1ca31010 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -531,6 +531,22 @@ DEFINE_STATEID_EVENT(open);
> DEFINE_STATEID_EVENT(deleg_read);
> DEFINE_STATEID_EVENT(deleg_recall);
>
> +TRACE_EVENT(nfsd_open_break_lease,
> + TP_PROTO(struct inode *inode,
> + int access),
> + TP_ARGS(inode, access),
> + TP_STRUCT__entry(
> + __field(void *, inode)
> + __field(int, access)
trace_print_symbols_seq() takes an unsigned long,
so I prefer to store values that are passed to it in
an unsigned long field to make the type conversion
more obvious to readers.
> + ),
> + TP_fast_assign(
> + __entry->inode = inode;
> + __entry->access = access;
> + ),
> + TP_printk("inode=%p access=%s", __entry->inode,
> + show_nfsd_may_flags(__entry->access))
> +)
> +
> DECLARE_EVENT_CLASS(nfsd_stateseqid_class,
> TP_PROTO(u32 seqid, const stateid_t *stp),
> TP_ARGS(seqid, stp),
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index d79db56475d4..0edfe6ff7d22 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -729,6 +729,8 @@ int nfsd_open_break_lease(struct inode *inode, int access)
> {
> unsigned int mode;
>
> + trace_nfsd_open_break_lease(inode, access);
> +
IMO this tracepoint should include the incoming XID as well.
I checked: each caller of nfsd_open_break_lease() has an
svc_rqst pointer to pass in here, and that can be passed
along to the tracepoint.
Thus if we're going to change the synopsis of
nfsd_open_break_lease() perhaps it would be better to return
a __be32 nfserr status, since every one of its call sites
appears to convert the returned hosterr value to an nfserr.
> if (access & NFSD_MAY_NOT_BREAK_LEASE)
> return 0;
> mode = (access & NFSD_MAY_WRITE) ? O_WRONLY : O_RDONLY;
> --
> 2.37.1
>
--
Chuck Lever
