| Message ID | 20240328161654.1432-1-chenhx.fnst@fujitsu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] NFSD: trace export root filehandle event | expand |
On Fri, Mar 29, 2024 at 12:16:54AM +0800, Chen Hanxiao wrote: > Add a tracepoint for obtaining root filehandle event I've had a chance to look at this more closely. It turns out we've already got trace_nfsd_ctl_filehandle(). So let's only remove the dprintk call site in exp_rootfh(). > Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com> > --- > v2: > remove dentry address record > add netns inum entry > trace ex_flags > > fs/nfsd/export.c | 4 +--- > fs/nfsd/trace.h | 41 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+), 3 deletions(-) > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c > index 7b641095a665..63acd1564eab 100644 > --- a/fs/nfsd/export.c > +++ b/fs/nfsd/export.c > @@ -1027,15 +1027,13 @@ exp_rootfh(struct net *net, struct auth_domain *clp, char *name, > } > inode = d_inode(path.dentry); > > - dprintk("nfsd: exp_rootfh(%s [%p] %s:%s/%ld)\n", > - name, path.dentry, clp->name, > - inode->i_sb->s_id, inode->i_ino); > exp = exp_parent(cd, clp, &path); > if (IS_ERR(exp)) { > err = PTR_ERR(exp); > goto out; > } > > + trace_nfsd_exp_rootfh(net, name, clp->name, inode, exp); > /* > * fh must be initialized before calling fh_compose > */ > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h > index 1cd2076210b1..adac651e398d 100644 > --- a/fs/nfsd/trace.h > +++ b/fs/nfsd/trace.h > @@ -396,6 +396,47 @@ TRACE_EVENT(nfsd_export_update, > ) > ); > > +TRACE_EVENT(nfsd_exp_rootfh, > + TP_PROTO( > + const struct net *net, > + const char *name, > + const char *clp_name, > + const struct inode *inode, > + const struct svc_export *exp > + ), > + TP_ARGS(net, name, clp_name, inode, exp), > + TP_STRUCT__entry( > + __string(name, name) > + __field(unsigned int, netns_ino) > + __string(clp_name, clp_name) > + __string(s_id, inode->i_sb->s_id) > + __field(unsigned long, i_ino) > + __array(unsigned char, uuid, EX_UUID_LEN) > + __field(int, ex_flags) > + __field(const void *, ex_uuid) > + ), > + TP_fast_assign( > + __assign_str(name, name); > + __entry->netns_ino = net->ns.inum; > + __assign_str(clp_name, clp_name); > + __assign_str(s_id, inode->i_sb->s_id); > + __entry->i_ino = inode->i_ino; > + __entry->ex_flags = exp->ex_flags; > + __entry->ex_uuid = exp->ex_uuid; > + if (exp->ex_uuid) > + memcpy(__entry->uuid, exp->ex_uuid, EX_UUID_LEN); > + ), > + TP_printk( > + "path=%s domain=%s sid=%s/inode=%ld ex_flags=%d ex_uuid=%s", > + __get_str(name), > + __get_str(clp_name), > + __get_str(s_id), > + __entry->i_ino, > + __entry->ex_flags, > + __entry->ex_uuid ? __print_hex_str(__entry->uuid, EX_UUID_LEN) : "NULL" > + ) > +); > + > DECLARE_EVENT_CLASS(nfsd_io_class, > TP_PROTO(struct svc_rqst *rqstp, > struct svc_fh *fhp, > -- > 2.39.1 >
> -----邮件原件----- > 发件人: Chuck Lever <chuck.lever@oracle.com> > 发送时间: 2024年4月6日 2:32 > 收件人: Chen, Hanxiao <chenhx.fnst@fujitsu.com> > 抄送: Jeff Layton <jlayton@kernel.org>; Neil Brown <neilb@suse.de>; Olga > Kornievskaia <kolga@netapp.com>; Dai Ngo <Dai.Ngo@oracle.com>; Tom > Talpey <tom@talpey.com>; linux-nfs@vger.kernel.org > 主题: Re: [PATCH v2] NFSD: trace export root filehandle event > > On Fri, Mar 29, 2024 at 12:16:54AM +0800, Chen Hanxiao wrote: > > Add a tracepoint for obtaining root filehandle event > > I've had a chance to look at this more closely. It turns out we've > already got trace_nfsd_ctl_filehandle(). > > So let's only remove the dprintk call site in exp_rootfh(). > > Hi, struct svc_export should be a useful info to trace, such as ex_uuid. Can we move trace_nfsd_ctl_filehandle into exp_rootfh ,then add an entry to it to trace ex_uuid? Regards, - Chen
> On Apr 6, 2024, at 11:29 AM, Hanxiao Chen (Fujitsu) <chenhx.fnst@fujitsu.com> wrote: > >> -----邮件原件----- >> 发件人: Chuck Lever <chuck.lever@oracle.com> >> 发送时间: 2024年4月6日 2:32 >> 收件人: Chen, Hanxiao <chenhx.fnst@fujitsu.com> >> 抄送: Jeff Layton <jlayton@kernel.org>; Neil Brown <neilb@suse.de>; Olga >> Kornievskaia <kolga@netapp.com>; Dai Ngo <Dai.Ngo@oracle.com>; Tom >> Talpey <tom@talpey.com>; linux-nfs@vger.kernel.org >> 主题: Re: [PATCH v2] NFSD: trace export root filehandle event >> >> On Fri, Mar 29, 2024 at 12:16:54AM +0800, Chen Hanxiao wrote: >>> Add a tracepoint for obtaining root filehandle event >> >> I've had a chance to look at this more closely. It turns out we've >> already got trace_nfsd_ctl_filehandle(). >> >> So let's only remove the dprintk call site in exp_rootfh(). >> >> > Hi, > > struct svc_export should be a useful info to trace, such as ex_uuid. Then the patch description needs to explain why the existing tracepoint is not adequate. Why does a trouble-shooter need the UUID and export flags for, for example? > Can we move trace_nfsd_ctl_filehandle into exp_rootfh ,then add an entry to it to trace ex_uuid? No, trace_nfsd_ctl_* all report information from user space, so they belong where they are now. If we need more information, then yes, another tracepoint can be added. But I haven't seen a detailed rationale for this yet. <more-info-needed> ;-) -- Chuck Lever
> -----邮件原件----- > 发件人: Chuck Lever III <chuck.lever@oracle.com> > 发送时间: 2024年4月6日 23:36 > 收件人: Chen, Hanxiao/ <chenhx.fnst@fujitsu.com> > 抄送: Jeff Layton <jlayton@kernel.org>; Neil Brown <neilb@suse.de>; Olga > Kornievskaia <kolga@netapp.com>; Dai Ngo <dai.ngo@oracle.com>; Tom > Talpey <tom@talpey.com>; Linux NFS Mailing List <linux-nfs@vger.kernel.org> > >> 主题: Re: [PATCH v2] NFSD: trace export root filehandle event > >> > >> On Fri, Mar 29, 2024 at 12:16:54AM +0800, Chen Hanxiao wrote: > >>> Add a tracepoint for obtaining root filehandle event > >> > >> I've had a chance to look at this more closely. It turns out we've > >> already got trace_nfsd_ctl_filehandle(). > >> > >> So let's only remove the dprintk call site in exp_rootfh(). > >> > >> > > Hi, > > > > struct svc_export should be a useful info to trace, such as ex_uuid. > > Then the patch description needs to explain why the existing > tracepoint is not adequate. Why does a trouble-shooter need > the UUID and export flags for, for example? > > > > Can we move trace_nfsd_ctl_filehandle into exp_rootfh ,then add an entry > to it to trace ex_uuid? > > No, trace_nfsd_ctl_* all report information from user space, so > they belong where they are now. > > If we need more information, then yes, another tracepoint can > be added. But I haven't seen a detailed rationale for this yet. > > <more-info-needed> ;-) Thanks for your detailed explanation. I'm trying to write patches to obtain root fh in user land by nfs-utils. The ex_uuid is useful info for userland tools to identify whether we got the right one. If the user land root fh patches is merged, maybe we could give the new tracepoint a chance. I'll send a patch to just remove the dprintk part in exp_rootfh. Regards, - Chen
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c index 7b641095a665..63acd1564eab 100644 --- a/fs/nfsd/export.c +++ b/fs/nfsd/export.c @@ -1027,15 +1027,13 @@ exp_rootfh(struct net *net, struct auth_domain *clp, char *name, } inode = d_inode(path.dentry); - dprintk("nfsd: exp_rootfh(%s [%p] %s:%s/%ld)\n", - name, path.dentry, clp->name, - inode->i_sb->s_id, inode->i_ino); exp = exp_parent(cd, clp, &path); if (IS_ERR(exp)) { err = PTR_ERR(exp); goto out; } + trace_nfsd_exp_rootfh(net, name, clp->name, inode, exp); /* * fh must be initialized before calling fh_compose */ diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h index 1cd2076210b1..adac651e398d 100644 --- a/fs/nfsd/trace.h +++ b/fs/nfsd/trace.h @@ -396,6 +396,47 @@ TRACE_EVENT(nfsd_export_update, ) ); +TRACE_EVENT(nfsd_exp_rootfh, + TP_PROTO( + const struct net *net, + const char *name, + const char *clp_name, + const struct inode *inode, + const struct svc_export *exp + ), + TP_ARGS(net, name, clp_name, inode, exp), + TP_STRUCT__entry( + __string(name, name) + __field(unsigned int, netns_ino) + __string(clp_name, clp_name) + __string(s_id, inode->i_sb->s_id) + __field(unsigned long, i_ino) + __array(unsigned char, uuid, EX_UUID_LEN) + __field(int, ex_flags) + __field(const void *, ex_uuid) + ), + TP_fast_assign( + __assign_str(name, name); + __entry->netns_ino = net->ns.inum; + __assign_str(clp_name, clp_name); + __assign_str(s_id, inode->i_sb->s_id); + __entry->i_ino = inode->i_ino; + __entry->ex_flags = exp->ex_flags; + __entry->ex_uuid = exp->ex_uuid; + if (exp->ex_uuid) + memcpy(__entry->uuid, exp->ex_uuid, EX_UUID_LEN); + ), + TP_printk( + "path=%s domain=%s sid=%s/inode=%ld ex_flags=%d ex_uuid=%s", + __get_str(name), + __get_str(clp_name), + __get_str(s_id), + __entry->i_ino, + __entry->ex_flags, + __entry->ex_uuid ? __print_hex_str(__entry->uuid, EX_UUID_LEN) : "NULL" + ) +); + DECLARE_EVENT_CLASS(nfsd_io_class, TP_PROTO(struct svc_rqst *rqstp, struct svc_fh *fhp,
Add a tracepoint for obtaining root filehandle event Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com> --- v2: remove dentry address record add netns inum entry trace ex_flags fs/nfsd/export.c | 4 +--- fs/nfsd/trace.h | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-)