2020-11-12 15:01:44

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 0/4] NFSD tracepoint clean-ups for v5.11

Hi-

I'm planning to merge these minor clean-ups with v5.11. Thanks in
advance for any review comments!

---

Chuck Lever (4):
SUNRPC: Move the svc_xdr_recvfrom() tracepoint
NFSD: Clean up the show_nf_may macro
NFSD: Remove extra "0x" in tracepoint format specifier
NFSD: Add SPDX header for fs/nfsd/trace.c


fs/nfsd/trace.c | 1 +
fs/nfsd/trace.h | 48 ++++++++++++++++++++++++++++++------------------
2 files changed, 31 insertions(+), 18 deletions(-)

--
Chuck Lever


2020-11-12 15:02:05

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 2/4] NFSD: Clean up the show_nf_may macro

Display all currently possible NFSD_MAY permission flags.

Move and rename show_nf_may with a more generic name because the
NFSD_MAY permission flags are used in other places besides the file
cache.

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/trace.h | 40 ++++++++++++++++++++++++++--------------
1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 99bf07800cd0..532b66a4b7f1 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -12,6 +12,22 @@
#include "export.h"
#include "nfsfh.h"

+#define show_nfsd_may_flags(x) \
+ __print_flags(x, "|", \
+ { NFSD_MAY_EXEC, "EXEC" }, \
+ { NFSD_MAY_WRITE, "WRITE" }, \
+ { NFSD_MAY_READ, "READ" }, \
+ { NFSD_MAY_SATTR, "SATTR" }, \
+ { NFSD_MAY_TRUNC, "TRUNC" }, \
+ { NFSD_MAY_LOCK, "LOCK" }, \
+ { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
+ { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
+ { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
+ { NFSD_MAY_NOT_BREAK_LEASE, "NOT_BREAD_LEASE" }, \
+ { NFSD_MAY_BYPASS_GSS, "BYPASS_GSS" }, \
+ { NFSD_MAY_READ_IF_EXEC, "READ_IF_EXEC" }, \
+ { NFSD_MAY_64BIT_COOKIE, "64BIT_COOKIE" })
+
TRACE_EVENT(nfsd_compound,
TP_PROTO(const struct svc_rqst *rqst,
u32 args_opcnt),
@@ -421,6 +437,9 @@ TRACE_EVENT(nfsd_clid_inuse_err,
__entry->cl_boot, __entry->cl_id)
)

+/*
+ * from fs/nfsd/filecache.h
+ */
TRACE_DEFINE_ENUM(NFSD_FILE_HASHED);
TRACE_DEFINE_ENUM(NFSD_FILE_PENDING);
TRACE_DEFINE_ENUM(NFSD_FILE_BREAK_READ);
@@ -435,13 +454,6 @@ TRACE_DEFINE_ENUM(NFSD_FILE_REFERENCED);
{ 1 << NFSD_FILE_BREAK_WRITE, "BREAK_WRITE" }, \
{ 1 << NFSD_FILE_REFERENCED, "REFERENCED"})

-/* FIXME: This should probably be fleshed out in the future. */
-#define show_nf_may(val) \
- __print_flags(val, "|", \
- { NFSD_MAY_READ, "READ" }, \
- { NFSD_MAY_WRITE, "WRITE" }, \
- { NFSD_MAY_NOT_BREAK_LEASE, "NOT_BREAK_LEASE" })
-
DECLARE_EVENT_CLASS(nfsd_file_class,
TP_PROTO(struct nfsd_file *nf),
TP_ARGS(nf),
@@ -466,7 +478,7 @@ DECLARE_EVENT_CLASS(nfsd_file_class,
__entry->nf_inode,
__entry->nf_ref,
show_nf_flags(__entry->nf_flags),
- show_nf_may(__entry->nf_may),
+ show_nfsd_may_flags(__entry->nf_may),
__entry->nf_file)
)

@@ -492,10 +504,10 @@ TRACE_EVENT(nfsd_file_acquire,
__field(u32, xid)
__field(unsigned int, hash)
__field(void *, inode)
- __field(unsigned int, may_flags)
+ __field(unsigned long, may_flags)
__field(int, nf_ref)
__field(unsigned long, nf_flags)
- __field(unsigned char, nf_may)
+ __field(unsigned long, nf_may)
__field(struct file *, nf_file)
__field(u32, status)
),
@@ -514,10 +526,10 @@ TRACE_EVENT(nfsd_file_acquire,

TP_printk("xid=0x%x hash=0x%x inode=0x%p may_flags=%s ref=%d nf_flags=%s nf_may=%s nf_file=0x%p status=%u",
__entry->xid, __entry->hash, __entry->inode,
- show_nf_may(__entry->may_flags), __entry->nf_ref,
- show_nf_flags(__entry->nf_flags),
- show_nf_may(__entry->nf_may), __entry->nf_file,
- __entry->status)
+ show_nfsd_may_flags(__entry->may_flags),
+ __entry->nf_ref, show_nf_flags(__entry->nf_flags),
+ show_nfsd_may_flags(__entry->nf_may),
+ __entry->nf_file, __entry->status)
);

DECLARE_EVENT_CLASS(nfsd_file_search_class,


2020-11-12 15:02:19

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 3/4] NFSD: Remove extra "0x" in tracepoint format specifier

Clean up: %p adds its own 0x already.

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/trace.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 532b66a4b7f1..33facf9f518a 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -473,7 +473,7 @@ DECLARE_EVENT_CLASS(nfsd_file_class,
__entry->nf_may = nf->nf_may;
__entry->nf_file = nf->nf_file;
),
- TP_printk("hash=0x%x inode=0x%p ref=%d flags=%s may=%s file=%p",
+ TP_printk("hash=0x%x inode=%p ref=%d flags=%s may=%s file=%p",
__entry->nf_hashval,
__entry->nf_inode,
__entry->nf_ref,
@@ -524,7 +524,7 @@ TRACE_EVENT(nfsd_file_acquire,
__entry->status = be32_to_cpu(status);
),

- TP_printk("xid=0x%x hash=0x%x inode=0x%p may_flags=%s ref=%d nf_flags=%s nf_may=%s nf_file=0x%p status=%u",
+ TP_printk("xid=0x%x hash=0x%x inode=%p may_flags=%s ref=%d nf_flags=%s nf_may=%s nf_file=%p status=%u",
__entry->xid, __entry->hash, __entry->inode,
show_nfsd_may_flags(__entry->may_flags),
__entry->nf_ref, show_nf_flags(__entry->nf_flags),
@@ -545,7 +545,7 @@ DECLARE_EVENT_CLASS(nfsd_file_search_class,
__entry->hash = hash;
__entry->found = found;
),
- TP_printk("hash=0x%x inode=0x%p found=%d", __entry->hash,
+ TP_printk("hash=0x%x inode=%p found=%d", __entry->hash,
__entry->inode, __entry->found)
);

@@ -573,7 +573,7 @@ TRACE_EVENT(nfsd_file_fsnotify_handle_event,
__entry->mode = inode->i_mode;
__entry->mask = mask;
),
- TP_printk("inode=0x%p nlink=%u mode=0%ho mask=0x%x", __entry->inode,
+ TP_printk("inode=%p nlink=%u mode=0%ho mask=0x%x", __entry->inode,
__entry->nlink, __entry->mode, __entry->mask)
);



2020-11-12 15:02:22

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 4/4] NFSD: Add SPDX header for fs/nfsd/trace.c

Clean up.

The file was contributed in 2014 by Christoph Hellwig in commit
31ef83dc0538 ("nfsd: add trace events").

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/trace.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/nfsd/trace.c b/fs/nfsd/trace.c
index 90967466a1e5..f008b95ceec2 100644
--- a/fs/nfsd/trace.c
+++ b/fs/nfsd/trace.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0

#define CREATE_TRACE_POINTS
#include "trace.h"


2020-11-12 15:02:38

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 1/4] SUNRPC: Move the svc_xdr_recvfrom() tracepoint

Commit c509f15a5801 ("SUNRPC: Split the xdr_buf event class") added
display of the rqst's XID to the svc_xdr_buf_class. However, when
the recvfrom tracepoint fires, rq_xid has yet to be filled in with
the current XID. So it ends up recording the last XID that was
handled by that svc_rqst.

Signed-off-by: Chuck Lever <[email protected]>
---
include/trace/events/sunrpc.h | 24 ------------------------
net/sunrpc/svc_xprt.c | 4 +---
2 files changed, 1 insertion(+), 27 deletions(-)

diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index 2477014e3fa6..065f39056e87 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1499,30 +1499,6 @@ SVC_RQST_FLAG_LIST
#define show_rqstp_flags(flags) \
__print_flags(flags, "|", SVC_RQST_FLAG_LIST)

-TRACE_EVENT(svc_recv,
- TP_PROTO(struct svc_rqst *rqst, int len),
-
- TP_ARGS(rqst, len),
-
- TP_STRUCT__entry(
- __field(u32, xid)
- __field(int, len)
- __field(unsigned long, flags)
- __string(addr, rqst->rq_xprt->xpt_remotebuf)
- ),
-
- TP_fast_assign(
- __entry->xid = be32_to_cpu(rqst->rq_xid);
- __entry->len = len;
- __entry->flags = rqst->rq_flags;
- __assign_str(addr, rqst->rq_xprt->xpt_remotebuf);
- ),
-
- TP_printk("addr=%s xid=0x%08x len=%d flags=%s",
- __get_str(addr), __entry->xid, __entry->len,
- show_rqstp_flags(__entry->flags))
-);
-
TRACE_DEFINE_ENUM(SVC_GARBAGE);
TRACE_DEFINE_ENUM(SVC_SYSERR);
TRACE_DEFINE_ENUM(SVC_VALID);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 43cf8dbde898..5fb9164aa690 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -813,8 +813,6 @@ static int svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt)
len = svc_deferred_recv(rqstp);
else
len = xprt->xpt_ops->xpo_recvfrom(rqstp);
- if (len > 0)
- trace_svc_xdr_recvfrom(rqstp, &rqstp->rq_arg);
rqstp->rq_stime = ktime_get();
rqstp->rq_reserved = serv->sv_max_mesg;
atomic_add(rqstp->rq_reserved, &xprt->xpt_reserved);
@@ -868,7 +866,7 @@ int svc_recv(struct svc_rqst *rqstp, long timeout)

if (serv->sv_stats)
serv->sv_stats->netcnt++;
- trace_svc_recv(rqstp, len);
+ trace_svc_xdr_recvfrom(rqstp, &rqstp->rq_arg);
return len;
out_release:
rqstp->rq_res.len = 0;


2020-11-12 19:47:08

by Calum Mackay

[permalink] [raw]
Subject: Re: [PATCH v1 2/4] NFSD: Clean up the show_nf_may macro

On 12/11/2020 3:01 pm, Chuck Lever wrote:
> Display all currently possible NFSD_MAY permission flags.
>
> Move and rename show_nf_may with a more generic name because the
> NFSD_MAY permission flags are used in other places besides the file
> cache.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> fs/nfsd/trace.h | 40 ++++++++++++++++++++++++++--------------
> 1 file changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 99bf07800cd0..532b66a4b7f1 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -12,6 +12,22 @@
> #include "export.h"
> #include "nfsfh.h"
>
> +#define show_nfsd_may_flags(x) \
> + __print_flags(x, "|", \
> + { NFSD_MAY_EXEC, "EXEC" }, \
> + { NFSD_MAY_WRITE, "WRITE" }, \
> + { NFSD_MAY_READ, "READ" }, \
> + { NFSD_MAY_SATTR, "SATTR" }, \
> + { NFSD_MAY_TRUNC, "TRUNC" }, \
> + { NFSD_MAY_LOCK, "LOCK" }, \
> + { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> + { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> + { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> + { NFSD_MAY_NOT_BREAK_LEASE, "NOT_BREAD_LEASE" }, \

typo :)

> + { NFSD_MAY_BYPASS_GSS, "BYPASS_GSS" }, \
> + { NFSD_MAY_READ_IF_EXEC, "READ_IF_EXEC" }, \
> + { NFSD_MAY_64BIT_COOKIE, "64BIT_COOKIE" })
> +
> TRACE_EVENT(nfsd_compound,
> TP_PROTO(const struct svc_rqst *rqst,
> u32 args_opcnt),
> @@ -421,6 +437,9 @@ TRACE_EVENT(nfsd_clid_inuse_err,
> __entry->cl_boot, __entry->cl_id)
> )
>
> +/*
> + * from fs/nfsd/filecache.h
> + */
> TRACE_DEFINE_ENUM(NFSD_FILE_HASHED);
> TRACE_DEFINE_ENUM(NFSD_FILE_PENDING);
> TRACE_DEFINE_ENUM(NFSD_FILE_BREAK_READ);
> @@ -435,13 +454,6 @@ TRACE_DEFINE_ENUM(NFSD_FILE_REFERENCED);
> { 1 << NFSD_FILE_BREAK_WRITE, "BREAK_WRITE" }, \
> { 1 << NFSD_FILE_REFERENCED, "REFERENCED"})
>
> -/* FIXME: This should probably be fleshed out in the future. */
> -#define show_nf_may(val) \
> - __print_flags(val, "|", \
> - { NFSD_MAY_READ, "READ" }, \
> - { NFSD_MAY_WRITE, "WRITE" }, \
> - { NFSD_MAY_NOT_BREAK_LEASE, "NOT_BREAK_LEASE" })
> -
> DECLARE_EVENT_CLASS(nfsd_file_class,
> TP_PROTO(struct nfsd_file *nf),
> TP_ARGS(nf),
> @@ -466,7 +478,7 @@ DECLARE_EVENT_CLASS(nfsd_file_class,
> __entry->nf_inode,
> __entry->nf_ref,
> show_nf_flags(__entry->nf_flags),
> - show_nf_may(__entry->nf_may),
> + show_nfsd_may_flags(__entry->nf_may),
> __entry->nf_file)
> )
>
> @@ -492,10 +504,10 @@ TRACE_EVENT(nfsd_file_acquire,
> __field(u32, xid)
> __field(unsigned int, hash)
> __field(void *, inode)
> - __field(unsigned int, may_flags)
> + __field(unsigned long, may_flags)
> __field(int, nf_ref)
> __field(unsigned long, nf_flags)
> - __field(unsigned char, nf_may)
> + __field(unsigned long, nf_may)
> __field(struct file *, nf_file)
> __field(u32, status)
> ),
> @@ -514,10 +526,10 @@ TRACE_EVENT(nfsd_file_acquire,
>
> TP_printk("xid=0x%x hash=0x%x inode=0x%p may_flags=%s ref=%d nf_flags=%s nf_may=%s nf_file=0x%p status=%u",
> __entry->xid, __entry->hash, __entry->inode,
> - show_nf_may(__entry->may_flags), __entry->nf_ref,
> - show_nf_flags(__entry->nf_flags),
> - show_nf_may(__entry->nf_may), __entry->nf_file,
> - __entry->status)
> + show_nfsd_may_flags(__entry->may_flags),
> + __entry->nf_ref, show_nf_flags(__entry->nf_flags),
> + show_nfsd_may_flags(__entry->nf_may),
> + __entry->nf_file, __entry->status)
> );
>
> DECLARE_EVENT_CLASS(nfsd_file_search_class,
>
>

--
Calum Mackay
Linux Kernel Engineering
Oracle Linux and Virtualisation


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2020-11-12 19:47:57

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v1 2/4] NFSD: Clean up the show_nf_may macro



> On Nov 12, 2020, at 2:45 PM, Calum Mackay <[email protected]> wrote:
>
> On 12/11/2020 3:01 pm, Chuck Lever wrote:
>> Display all currently possible NFSD_MAY permission flags.
>> Move and rename show_nf_may with a more generic name because the
>> NFSD_MAY permission flags are used in other places besides the file
>> cache.
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> fs/nfsd/trace.h | 40 ++++++++++++++++++++++++++--------------
>> 1 file changed, 26 insertions(+), 14 deletions(-)
>> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
>> index 99bf07800cd0..532b66a4b7f1 100644
>> --- a/fs/nfsd/trace.h
>> +++ b/fs/nfsd/trace.h
>> @@ -12,6 +12,22 @@
>> #include "export.h"
>> #include "nfsfh.h"
>> +#define show_nfsd_may_flags(x) \
>> + __print_flags(x, "|", \
>> + { NFSD_MAY_EXEC, "EXEC" }, \
>> + { NFSD_MAY_WRITE, "WRITE" }, \
>> + { NFSD_MAY_READ, "READ" }, \
>> + { NFSD_MAY_SATTR, "SATTR" }, \
>> + { NFSD_MAY_TRUNC, "TRUNC" }, \
>> + { NFSD_MAY_LOCK, "LOCK" }, \
>> + { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
>> + { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
>> + { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
>> + { NFSD_MAY_NOT_BREAK_LEASE, "NOT_BREAD_LEASE" }, \
>
> typo :)

Amusing! Fixed.


>> + { NFSD_MAY_BYPASS_GSS, "BYPASS_GSS" }, \
>> + { NFSD_MAY_READ_IF_EXEC, "READ_IF_EXEC" }, \
>> + { NFSD_MAY_64BIT_COOKIE, "64BIT_COOKIE" })
>> +
>> TRACE_EVENT(nfsd_compound,
>> TP_PROTO(const struct svc_rqst *rqst,
>> u32 args_opcnt),
>> @@ -421,6 +437,9 @@ TRACE_EVENT(nfsd_clid_inuse_err,
>> __entry->cl_boot, __entry->cl_id)
>> )
>> +/*
>> + * from fs/nfsd/filecache.h
>> + */
>> TRACE_DEFINE_ENUM(NFSD_FILE_HASHED);
>> TRACE_DEFINE_ENUM(NFSD_FILE_PENDING);
>> TRACE_DEFINE_ENUM(NFSD_FILE_BREAK_READ);
>> @@ -435,13 +454,6 @@ TRACE_DEFINE_ENUM(NFSD_FILE_REFERENCED);
>> { 1 << NFSD_FILE_BREAK_WRITE, "BREAK_WRITE" }, \
>> { 1 << NFSD_FILE_REFERENCED, "REFERENCED"})
>> -/* FIXME: This should probably be fleshed out in the future. */
>> -#define show_nf_may(val) \
>> - __print_flags(val, "|", \
>> - { NFSD_MAY_READ, "READ" }, \
>> - { NFSD_MAY_WRITE, "WRITE" }, \
>> - { NFSD_MAY_NOT_BREAK_LEASE, "NOT_BREAK_LEASE" })
>> -
>> DECLARE_EVENT_CLASS(nfsd_file_class,
>> TP_PROTO(struct nfsd_file *nf),
>> TP_ARGS(nf),
>> @@ -466,7 +478,7 @@ DECLARE_EVENT_CLASS(nfsd_file_class,
>> __entry->nf_inode,
>> __entry->nf_ref,
>> show_nf_flags(__entry->nf_flags),
>> - show_nf_may(__entry->nf_may),
>> + show_nfsd_may_flags(__entry->nf_may),
>> __entry->nf_file)
>> )
>> @@ -492,10 +504,10 @@ TRACE_EVENT(nfsd_file_acquire,
>> __field(u32, xid)
>> __field(unsigned int, hash)
>> __field(void *, inode)
>> - __field(unsigned int, may_flags)
>> + __field(unsigned long, may_flags)
>> __field(int, nf_ref)
>> __field(unsigned long, nf_flags)
>> - __field(unsigned char, nf_may)
>> + __field(unsigned long, nf_may)
>> __field(struct file *, nf_file)
>> __field(u32, status)
>> ),
>> @@ -514,10 +526,10 @@ TRACE_EVENT(nfsd_file_acquire,
>> TP_printk("xid=0x%x hash=0x%x inode=0x%p may_flags=%s ref=%d nf_flags=%s nf_may=%s nf_file=0x%p status=%u",
>> __entry->xid, __entry->hash, __entry->inode,
>> - show_nf_may(__entry->may_flags), __entry->nf_ref,
>> - show_nf_flags(__entry->nf_flags),
>> - show_nf_may(__entry->nf_may), __entry->nf_file,
>> - __entry->status)
>> + show_nfsd_may_flags(__entry->may_flags),
>> + __entry->nf_ref, show_nf_flags(__entry->nf_flags),
>> + show_nfsd_may_flags(__entry->nf_may),
>> + __entry->nf_file, __entry->status)
>> );
>> DECLARE_EVENT_CLASS(nfsd_file_search_class,
>
> --
> Calum Mackay
> Linux Kernel Engineering
> Oracle Linux and Virtualisation

--
Chuck Lever