diff mbox series

[1/3] nfsd: add new tracepoint in nfsd_open_break_lease

Message ID 20220729164715.75702-1-jlayton@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/3] nfsd: add new tracepoint in nfsd_open_break_lease | expand

Commit Message

Jeff Layton July 29, 2022, 4:47 p.m. UTC 
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/trace.h | 16 ++++++++++++++++
 fs/nfsd/vfs.c   |  2 ++
 2 files changed, 18 insertions(+)

Comments

Chuck Lever July 29, 2022, 5:20 p.m. UTC | #1 
> On Jul 29, 2022, at 12:47 PM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> 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
diff mbox series

Patch

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)
+	),
+	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);
+
 	if (access & NFSD_MAY_NOT_BREAK_LEASE)
 		return 0;
 	mode = (access & NFSD_MAY_WRITE) ? O_WRONLY : O_RDONLY;