Re: [PATCH v14 07/25] NFSD: Short-circuit fh_verify tracepoints for LOCALIO

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2024-08-28 at 21:04 -0400, Mike Snitzer wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> LOCALIO will be able to call fh_verify() with a NULL rqstp. In this
> case, the existing trace points need to be skipped because they
> want to dereference the address fields in the passed-in rqstp.
> 
> Temporarily make these trace points conditional to avoid a seg
> fault in this case. Putting the "rqstp != NULL" check in the trace
> points themselves makes the check more efficient.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> ---
>  fs/nfsd/trace.h | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 77bbd23aa150..d22027e23761 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -193,7 +193,7 @@ TRACE_EVENT(nfsd_compound_encode_err,
>  		{ S_IFIFO,		"FIFO" }, \
>  		{ S_IFSOCK,		"SOCK" })
>  
> -TRACE_EVENT(nfsd_fh_verify,
> +TRACE_EVENT_CONDITION(nfsd_fh_verify,
>  	TP_PROTO(
>  		const struct svc_rqst *rqstp,
>  		const struct svc_fh *fhp,
> @@ -201,6 +201,7 @@ TRACE_EVENT(nfsd_fh_verify,
>  		int access
>  	),
>  	TP_ARGS(rqstp, fhp, type, access),
> +	TP_CONDITION(rqstp != NULL),
>  	TP_STRUCT__entry(
>  		__field(unsigned int, netns_ino)
>  		__sockaddr(server, rqstp->rq_xprt->xpt_remotelen)
> @@ -239,7 +240,7 @@ TRACE_EVENT_CONDITION(nfsd_fh_verify_err,
>  		__be32 error
>  	),
>  	TP_ARGS(rqstp, fhp, type, access, error),
> -	TP_CONDITION(error),
> +	TP_CONDITION(rqstp != NULL && error),
>  	TP_STRUCT__entry(
>  		__field(unsigned int, netns_ino)
>  		__sockaddr(server, rqstp->rq_xprt->xpt_remotelen)
> @@ -295,12 +296,13 @@ DECLARE_EVENT_CLASS(nfsd_fh_err_class,
>  		  __entry->status)
>  )
>  
> -#define DEFINE_NFSD_FH_ERR_EVENT(name)		\
> -DEFINE_EVENT(nfsd_fh_err_class, nfsd_##name,	\
> -	TP_PROTO(struct svc_rqst *rqstp,	\
> -		 struct svc_fh	*fhp,		\
> -		 int		status),	\
> -	TP_ARGS(rqstp, fhp, status))
> +#define DEFINE_NFSD_FH_ERR_EVENT(name)			\
> +DEFINE_EVENT_CONDITION(nfsd_fh_err_class, nfsd_##name,	\
> +	TP_PROTO(struct svc_rqst *rqstp,		\
> +		 struct svc_fh	*fhp,			\
> +		 int		status),		\
> +	TP_ARGS(rqstp, fhp, status),			\
> +	TP_CONDITION(rqstp != NULL))
>  
>  DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badexport);
>  DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badhandle);

A bit ugly. We really only want the rqstp here to get at the socket
structures. I'm still looking at the rest of the set, so I'll assume
that this gets cleaned up later.

Acked-by: Jeff Layton <jlayton@kernel.org>