2024-03-28 16:17:34

by Chen Hanxiao

[permalink] [raw]
Subject: [PATCH v2] NFSD: trace export root filehandle event

Add a tracepoint for obtaining root filehandle event

Signed-off-by: Chen Hanxiao <[email protected]>
---
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



2024-04-05 18:32:53

by Chuck Lever

[permalink] [raw]
Subject: 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().


> Signed-off-by: Chen Hanxiao <[email protected]>
> ---
> 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

2024-04-06 15:29:55

by Chen Hanxiao

[permalink] [raw]
Subject: Re: [PATCH v2] NFSD: trace export root filehandle event



> -----?ʼ?ԭ??-----
> ??????: Chuck Lever <[email protected]>
> ????ʱ??: 2024??4??6?? 2:32
> ?ռ???: Chen, Hanxiao <[email protected]>
> ????: Jeff Layton <[email protected]>; Neil Brown <[email protected]>; Olga
> Kornievskaia <[email protected]>; Dai Ngo <[email protected]>; Tom
> Talpey <[email protected]>; [email protected]
> ????: 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

2024-04-06 15:37:09

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v2] NFSD: trace export root filehandle event


> On Apr 6, 2024, at 11:29 AM, Hanxiao Chen (Fujitsu) <[email protected]> wrote:
>
>> -----邮件原件-----
>> 发件人: Chuck Lever <[email protected]>
>> 发送时间: 2024年4月6日 2:32
>> 收件人: Chen, Hanxiao <[email protected]>
>> 抄送: Jeff Layton <[email protected]>; Neil Brown <[email protected]>; Olga
>> Kornievskaia <[email protected]>; Dai Ngo <[email protected]>; Tom
>> Talpey <[email protected]>; [email protected]
>> 主题: 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


2024-04-06 16:03:11

by Chen Hanxiao

[permalink] [raw]
Subject: Re: [PATCH v2] NFSD: trace export root filehandle event



> -----邮件原件-----
> 发件人: Chuck Lever III <[email protected]>
> 发送时间: 2024年4月6日 23:36
> 收件人: Chen, Hanxiao/ <[email protected]>
> 抄送: Jeff Layton <[email protected]>; Neil Brown <[email protected]>; Olga
> Kornievskaia <[email protected]>; Dai Ngo <[email protected]>; Tom
> Talpey <[email protected]>; Linux NFS Mailing List <[email protected]>
> >> 主题: 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