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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 29, 2024 at 10:33:18AM -0400, Jeff Layton wrote:
> On Wed, 2024-08-28 at 21:04 -0400, Mike Snitzer wrote:
> > From: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> > ---
> >  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.

No, it doesn't. We don't have a solution for how to trace
LOCALIO activity here yet.

-- 
Chuck Lever




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux