[PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[Chuck Lever]

These can be shared with all kernel ULPs, and more can easily be
added as needed.

Signed-off-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 include/trace/events/rdma.h |  128 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100644 include/trace/events/rdma.h

Hi-

I'm putting together a series of patches for v4.16 (or later) that
add static ftrace trace points to the RPC-over-RDMA client imple-
mentation. As part of that effort, I've constructed some trace point
helpers that might be useful for other kernel ULPs.

So far this patch adds just helpers that xprtrdma needs. It is not
complete, but additional helpers can be introduced as they are
needed.

I'd like to hear comments about these, or please let me know if
such helpers already exist and where xprtrdma can find them.

Thanks!


diff --git a/include/trace/events/rdma.h b/include/trace/events/rdma.h
new file mode 100644
index 0000000..9d02fbe
--- /dev/null
+++ b/include/trace/events/rdma.h
@@ -0,0 +1,128 @@
+/*
+ * Copyright (c) 2017 Oracle.  All rights reserved.
+ */
+
+/*
+ * enum ib_event_type, from include/rdma/ib_verbs.h
+ */
+
+#define IB_EVENT_LIST				\
+	ib_event(CQ_ERR)			\
+	ib_event(QP_FATAL)			\
+	ib_event(QP_REQ_ERR)			\
+	ib_event(QP_ACCESS_ERR)			\
+	ib_event(COMM_EST)			\
+	ib_event(SQ_DRAINED)			\
+	ib_event(PATH_MIG)			\
+	ib_event(PATH_MIG_ERR)			\
+	ib_event(DEVICE_FATAL)			\
+	ib_event(PORT_ACTIVE)			\
+	ib_event(PORT_ERR)			\
+	ib_event(LID_CHANGE)			\
+	ib_event(PKEY_CHANGE)			\
+	ib_event(SM_CHANGE)			\
+	ib_event(SRQ_ERR)			\
+	ib_event(SRQ_LIMIT_REACHED)		\
+	ib_event(QP_LAST_WQE_REACHED)		\
+	ib_event(CLIENT_REREGISTER)		\
+	ib_event(GID_CHANGE)			\
+	ib_event_end(WQ_FATAL)
+
+#undef ib_event
+#undef ib_event_end
+
+#define ib_event(x)		TRACE_DEFINE_ENUM(IB_EVENT_##x);
+#define ib_event_end(x)		TRACE_DEFINE_ENUM(IB_EVENT_##x);
+
+IB_EVENT_LIST
+
+#undef ib_event
+#undef ib_event_end
+
+#define ib_event(x)		{ IB_EVENT_##x, #x },
+#define ib_event_end(x)		{ IB_EVENT_##x, #x }
+
+#define rdma_show_ib_event(x) \
+		__print_symbolic(x, IB_EVENT_LIST)
+
+/*
+ * enum ib_wc_status type, from include/rdma/ib_verbs.h
+ */
+#define IB_WC_STATUS_LIST			\
+	ib_wc_status(SUCCESS)			\
+	ib_wc_status(LOC_LEN_ERR)		\
+	ib_wc_status(LOC_QP_OP_ERR)		\
+	ib_wc_status(LOC_EEC_OP_ERR)		\
+	ib_wc_status(LOC_PROT_ERR)		\
+	ib_wc_status(WR_FLUSH_ERR)		\
+	ib_wc_status(MW_BIND_ERR)		\
+	ib_wc_status(BAD_RESP_ERR)		\
+	ib_wc_status(LOC_ACCESS_ERR)		\
+	ib_wc_status(REM_INV_REQ_ERR)		\
+	ib_wc_status(REM_ACCESS_ERR)		\
+	ib_wc_status(REM_OP_ERR)		\
+	ib_wc_status(RETRY_EXC_ERR)		\
+	ib_wc_status(RNR_RETRY_EXC_ERR)		\
+	ib_wc_status(LOC_RDD_VIOL_ERR)		\
+	ib_wc_status(REM_INV_RD_REQ_ERR)	\
+	ib_wc_status(REM_ABORT_ERR)		\
+	ib_wc_status(INV_EECN_ERR)		\
+	ib_wc_status(INV_EEC_STATE_ERR)		\
+	ib_wc_status(FATAL_ERR)			\
+	ib_wc_status(RESP_TIMEOUT_ERR)		\
+	ib_wc_status_end(GENERAL_ERR)
+
+#undef ib_wc_status
+#undef ib_wc_status_end
+
+#define ib_wc_status(x)		TRACE_DEFINE_ENUM(IB_WC_##x);
+#define ib_wc_status_end(x)	TRACE_DEFINE_ENUM(IB_WC_##x);
+
+IB_WC_STATUS_LIST
+
+#undef ib_wc_status
+#undef ib_wc_status_end
+
+#define ib_wc_status(x)		{ IB_WC_##x, #x },
+#define ib_wc_status_end(x)	{ IB_WC_##x, #x }
+
+#define rdma_show_wc_status(x) \
+		__print_symbolic(x, IB_WC_STATUS_LIST)
+
+/*
+ * enum rdma_cm_event_type, from include/rdma/rdma_cm.h
+ */
+#define RDMA_CM_EVENT_LIST			\
+	rdma_cm_event(ADDR_RESOLVED)		\
+	rdma_cm_event(ADDR_ERROR)		\
+	rdma_cm_event(ROUTE_RESOLVED)		\
+	rdma_cm_event(ROUTE_ERROR)		\
+	rdma_cm_event(CONNECT_REQUEST)		\
+	rdma_cm_event(CONNECT_RESPONSE)		\
+	rdma_cm_event(CONNECT_ERROR)		\
+	rdma_cm_event(UNREACHABLE)		\
+	rdma_cm_event(REJECTED)			\
+	rdma_cm_event(ESTABLISHED)		\
+	rdma_cm_event(DISCONNECTED)		\
+	rdma_cm_event(DEVICE_REMOVAL)		\
+	rdma_cm_event(MULTICAST_JOIN)		\
+	rdma_cm_event(MULTICAST_ERROR)		\
+	rdma_cm_event(ADDR_CHANGE)		\
+	rdma_cm_event_end(TIMEWAIT_EXIT)
+
+#undef rdma_cm_event
+#undef rdma_cm_event_end
+
+#define rdma_cm_event(x)	TRACE_DEFINE_ENUM(RDMA_CM_EVENT_##x);
+#define rdma_cm_event_end(x)	TRACE_DEFINE_ENUM(RDMA_CM_EVENT_##x);
+
+RDMA_CM_EVENT_LIST
+
+#undef rdma_cm_event
+#undef rdma_cm_event_end
+
+#define rdma_cm_event(x)	{ RDMA_CM_EVENT_##x, #x },
+#define rdma_cm_event_end(x)	{ RDMA_CM_EVENT_##x, #x }
+
+#define rdma_show_cm_event(x) \
+		__print_symbolic(x, RDMA_CM_EVENT_LIST)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 5823 bytes --]

On Mon, Oct 30, 2017 at 06:01:23PM -0400, Chuck Lever wrote:
> These can be shared with all kernel ULPs, and more can easily be
> added as needed.
>
> Signed-off-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
>  include/trace/events/rdma.h |  128 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 128 insertions(+)
>  create mode 100644 include/trace/events/rdma.h
>
> Hi-
>
> I'm putting together a series of patches for v4.16 (or later) that
> add static ftrace trace points to the RPC-over-RDMA client imple-
> mentation. As part of that effort, I've constructed some trace point
> helpers that might be useful for other kernel ULPs.
>
> So far this patch adds just helpers that xprtrdma needs. It is not
> complete, but additional helpers can be introduced as they are
> needed.
>
> I'd like to hear comments about these, or please let me know if
> such helpers already exist and where xprtrdma can find them.

Bart, Steve

You attended the MS track, and for the rest of us, the quote below
sounds a little bit cryptic. Does the quote below mean that Chuck's
proposal is no-go?

From LWN.net "Another attempt to address the tracepoint ABI problem" [1]

"The solution that was arrived at for now, as related by Torvalds,
 is to hold off on adding explicit tracepoints to the kernel. Instead,
 support will be added to make it easy for an application to attach a
 BPF script to any function in the kernel, with access to that function's
 arguments."

[1] https://lwn.net/Articles/737530/

Thanks

>
> Thanks!
>
>
> diff --git a/include/trace/events/rdma.h b/include/trace/events/rdma.h
> new file mode 100644
> index 0000000..9d02fbe
> --- /dev/null
> +++ b/include/trace/events/rdma.h
> @@ -0,0 +1,128 @@
> +/*
> + * Copyright (c) 2017 Oracle.  All rights reserved.
> + */
> +
> +/*
> + * enum ib_event_type, from include/rdma/ib_verbs.h
> + */
> +
> +#define IB_EVENT_LIST				\
> +	ib_event(CQ_ERR)			\
> +	ib_event(QP_FATAL)			\
> +	ib_event(QP_REQ_ERR)			\
> +	ib_event(QP_ACCESS_ERR)			\
> +	ib_event(COMM_EST)			\
> +	ib_event(SQ_DRAINED)			\
> +	ib_event(PATH_MIG)			\
> +	ib_event(PATH_MIG_ERR)			\
> +	ib_event(DEVICE_FATAL)			\
> +	ib_event(PORT_ACTIVE)			\
> +	ib_event(PORT_ERR)			\
> +	ib_event(LID_CHANGE)			\
> +	ib_event(PKEY_CHANGE)			\
> +	ib_event(SM_CHANGE)			\
> +	ib_event(SRQ_ERR)			\
> +	ib_event(SRQ_LIMIT_REACHED)		\
> +	ib_event(QP_LAST_WQE_REACHED)		\
> +	ib_event(CLIENT_REREGISTER)		\
> +	ib_event(GID_CHANGE)			\
> +	ib_event_end(WQ_FATAL)
> +
> +#undef ib_event
> +#undef ib_event_end
> +
> +#define ib_event(x)		TRACE_DEFINE_ENUM(IB_EVENT_##x);
> +#define ib_event_end(x)		TRACE_DEFINE_ENUM(IB_EVENT_##x);
> +
> +IB_EVENT_LIST
> +
> +#undef ib_event
> +#undef ib_event_end
> +
> +#define ib_event(x)		{ IB_EVENT_##x, #x },
> +#define ib_event_end(x)		{ IB_EVENT_##x, #x }
> +
> +#define rdma_show_ib_event(x) \
> +		__print_symbolic(x, IB_EVENT_LIST)
> +
> +/*
> + * enum ib_wc_status type, from include/rdma/ib_verbs.h
> + */
> +#define IB_WC_STATUS_LIST			\
> +	ib_wc_status(SUCCESS)			\
> +	ib_wc_status(LOC_LEN_ERR)		\
> +	ib_wc_status(LOC_QP_OP_ERR)		\
> +	ib_wc_status(LOC_EEC_OP_ERR)		\
> +	ib_wc_status(LOC_PROT_ERR)		\
> +	ib_wc_status(WR_FLUSH_ERR)		\
> +	ib_wc_status(MW_BIND_ERR)		\
> +	ib_wc_status(BAD_RESP_ERR)		\
> +	ib_wc_status(LOC_ACCESS_ERR)		\
> +	ib_wc_status(REM_INV_REQ_ERR)		\
> +	ib_wc_status(REM_ACCESS_ERR)		\
> +	ib_wc_status(REM_OP_ERR)		\
> +	ib_wc_status(RETRY_EXC_ERR)		\
> +	ib_wc_status(RNR_RETRY_EXC_ERR)		\
> +	ib_wc_status(LOC_RDD_VIOL_ERR)		\
> +	ib_wc_status(REM_INV_RD_REQ_ERR)	\
> +	ib_wc_status(REM_ABORT_ERR)		\
> +	ib_wc_status(INV_EECN_ERR)		\
> +	ib_wc_status(INV_EEC_STATE_ERR)		\
> +	ib_wc_status(FATAL_ERR)			\
> +	ib_wc_status(RESP_TIMEOUT_ERR)		\
> +	ib_wc_status_end(GENERAL_ERR)
> +
> +#undef ib_wc_status
> +#undef ib_wc_status_end
> +
> +#define ib_wc_status(x)		TRACE_DEFINE_ENUM(IB_WC_##x);
> +#define ib_wc_status_end(x)	TRACE_DEFINE_ENUM(IB_WC_##x);
> +
> +IB_WC_STATUS_LIST
> +
> +#undef ib_wc_status
> +#undef ib_wc_status_end
> +
> +#define ib_wc_status(x)		{ IB_WC_##x, #x },
> +#define ib_wc_status_end(x)	{ IB_WC_##x, #x }
> +
> +#define rdma_show_wc_status(x) \
> +		__print_symbolic(x, IB_WC_STATUS_LIST)
> +
> +/*
> + * enum rdma_cm_event_type, from include/rdma/rdma_cm.h
> + */
> +#define RDMA_CM_EVENT_LIST			\
> +	rdma_cm_event(ADDR_RESOLVED)		\
> +	rdma_cm_event(ADDR_ERROR)		\
> +	rdma_cm_event(ROUTE_RESOLVED)		\
> +	rdma_cm_event(ROUTE_ERROR)		\
> +	rdma_cm_event(CONNECT_REQUEST)		\
> +	rdma_cm_event(CONNECT_RESPONSE)		\
> +	rdma_cm_event(CONNECT_ERROR)		\
> +	rdma_cm_event(UNREACHABLE)		\
> +	rdma_cm_event(REJECTED)			\
> +	rdma_cm_event(ESTABLISHED)		\
> +	rdma_cm_event(DISCONNECTED)		\
> +	rdma_cm_event(DEVICE_REMOVAL)		\
> +	rdma_cm_event(MULTICAST_JOIN)		\
> +	rdma_cm_event(MULTICAST_ERROR)		\
> +	rdma_cm_event(ADDR_CHANGE)		\
> +	rdma_cm_event_end(TIMEWAIT_EXIT)
> +
> +#undef rdma_cm_event
> +#undef rdma_cm_event_end
> +
> +#define rdma_cm_event(x)	TRACE_DEFINE_ENUM(RDMA_CM_EVENT_##x);
> +#define rdma_cm_event_end(x)	TRACE_DEFINE_ENUM(RDMA_CM_EVENT_##x);
> +
> +RDMA_CM_EVENT_LIST
> +
> +#undef rdma_cm_event
> +#undef rdma_cm_event_end
> +
> +#define rdma_cm_event(x)	{ RDMA_CM_EVENT_##x, #x },
> +#define rdma_cm_event_end(x)	{ RDMA_CM_EVENT_##x, #x }
> +
> +#define rdma_show_cm_event(x) \
> +		__print_symbolic(x, RDMA_CM_EVENT_LIST)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[Steven Rostedt]

On Wed, 1 Nov 2017 08:27:43 +0200
Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> On Mon, Oct 30, 2017 at 06:01:23PM -0400, Chuck Lever wrote:
> > These can be shared with all kernel ULPs, and more can easily be
> > added as needed.
> >
> > Signed-off-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> >  include/trace/events/rdma.h |  128 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 128 insertions(+)
> >  create mode 100644 include/trace/events/rdma.h
> >
> > Hi-
> >
> > I'm putting together a series of patches for v4.16 (or later) that
> > add static ftrace trace points to the RPC-over-RDMA client imple-
> > mentation. As part of that effort, I've constructed some trace point
> > helpers that might be useful for other kernel ULPs.
> >
> > So far this patch adds just helpers that xprtrdma needs. It is not
> > complete, but additional helpers can be introduced as they are
> > needed.
> >
> > I'd like to hear comments about these, or please let me know if
> > such helpers already exist and where xprtrdma can find them.  
> 
> Bart, Steve
> 
> You attended the MS track, and for the rest of us, the quote below
> sounds a little bit cryptic. Does the quote below mean that Chuck's
> proposal is no-go?

Actually, I wasn't invited to the Maintainer's Summit. Perhaps I should
have been.

> 
> From LWN.net "Another attempt to address the tracepoint ABI problem" [1]
> 
> "The solution that was arrived at for now, as related by Torvalds,
>  is to hold off on adding explicit tracepoints to the kernel. Instead,
>  support will be added to make it easy for an application to attach a
>  BPF script to any function in the kernel, with access to that function's
>  arguments."
> 
> [1] https://lwn.net/Articles/737530/

What I can tell you is what I talked to Linus about when I cornered him
in the hallway. ;-)

Basically, nothing has changed. If a maintainer is fine with
trace events, then they can add new trace events. The issue is with the
maintainers that don't want the possibility of having to maintain stale
trace events because some tool depends on them and it becomes a burden
in the future. This is mostly an issues with the scheduler and VFS.

What Linus asked me to do is to build an infrastructure on top of
ftrace, and make it easier to extract data from these "dynamic function
trace events". I need to add easier use cases for eBPF because the
usage of eBPF is still a large learning curve compared to the ease of
accessing the current trace event infrastructure.

I'll be working on this in the very near future, but I don't expect
anything to be useful for within a year's time. Thus, continue doing
what you have been doing, and hopefully in a year we will be able to
extract more data from the kernel from anywhere a function is traced.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]

On Wed, Nov 01, 2017 at 11:29:32AM -0400, Steven Rostedt wrote:
> On Wed, 1 Nov 2017 08:27:43 +0200
> Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> > On Mon, Oct 30, 2017 at 06:01:23PM -0400, Chuck Lever wrote:
> > > These can be shared with all kernel ULPs, and more can easily be
> > > added as needed.
> > >
> > > Signed-off-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  include/trace/events/rdma.h |  128 +++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 128 insertions(+)
> > >  create mode 100644 include/trace/events/rdma.h
> > >
> > > Hi-
> > >
> > > I'm putting together a series of patches for v4.16 (or later) that
> > > add static ftrace trace points to the RPC-over-RDMA client imple-
> > > mentation. As part of that effort, I've constructed some trace point
> > > helpers that might be useful for other kernel ULPs.
> > >
> > > So far this patch adds just helpers that xprtrdma needs. It is not
> > > complete, but additional helpers can be introduced as they are
> > > needed.
> > >
> > > I'd like to hear comments about these, or please let me know if
> > > such helpers already exist and where xprtrdma can find them.
> >
> > Bart, Steve
> >
> > You attended the MS track, and for the rest of us, the quote below
> > sounds a little bit cryptic. Does the quote below mean that Chuck's
> > proposal is no-go?
>
> Actually, I wasn't invited to the Maintainer's Summit. Perhaps I should
> have been.
>
> >
> > From LWN.net "Another attempt to address the tracepoint ABI problem" [1]
> >
> > "The solution that was arrived at for now, as related by Torvalds,
> >  is to hold off on adding explicit tracepoints to the kernel. Instead,
> >  support will be added to make it easy for an application to attach a
> >  BPF script to any function in the kernel, with access to that function's
> >  arguments."
> >
> > [1] https://lwn.net/Articles/737530/
>
> What I can tell you is what I talked to Linus about when I cornered him
> in the hallway. ;-)
>
> Basically, nothing has changed. If a maintainer is fine with
> trace events, then they can add new trace events. The issue is with the
> maintainers that don't want the possibility of having to maintain stale
> trace events because some tool depends on them and it becomes a burden
> in the future. This is mostly an issues with the scheduler and VFS.
>
> What Linus asked me to do is to build an infrastructure on top of
> ftrace, and make it easier to extract data from these "dynamic function
> trace events". I need to add easier use cases for eBPF because the
> usage of eBPF is still a large learning curve compared to the ease of
> accessing the current trace event infrastructure.
>
> I'll be working on this in the very near future, but I don't expect
> anything to be useful for within a year's time. Thus, continue doing
> what you have been doing, and hopefully in a year we will be able to
> extract more data from the kernel from anywhere a function is traced.

Thanks

>
> -- Steve
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[Bart Van Assche]

On Wed, 2017-11-01 at 08:27 +0200, Leon Romanovsky wrote:
> Bart, Steve
> 
> You attended the MS track, and for the rest of us, the quote below
> sounds a little bit cryptic. Does the quote below mean that Chuck's
> proposal is no-go?
> 
> From LWN.net "Another attempt to address the tracepoint ABI problem" [1]
> 
> "The solution that was arrived at for now, as related by Torvalds,
>  is to hold off on adding explicit tracepoints to the kernel. Instead,
>  support will be added to make it easy for an application to attach a
>  BPF script to any function in the kernel, with access to that function's
>  arguments."
> 
> [1] https://lwn.net/Articles/737530/

What I remember is that Linus does not require us to avoid breaking user
space applications that use tracepoints. Powertop however is an exception
to this rule. Although it uses tracepoints, we must not break it. I also
remember that Linus noticed that the purpose of many tracepoints is to
allow users to trace a function and its arguments. Linus wants a better
approach for tracing kernel functions than adding an explicit tracepoint
to each kernel function. Maybe I wasn't listening carefully enough but I
haven't heard Linus saying that we would not be allowed to add new
tracepoints.

Bart.

Re: [PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]

On Wed, Nov 01, 2017 at 03:59:47PM +0000, Bart Van Assche wrote:
> On Wed, 2017-11-01 at 08:27 +0200, Leon Romanovsky wrote:
> > Bart, Steve
> >
> > You attended the MS track, and for the rest of us, the quote below
> > sounds a little bit cryptic. Does the quote below mean that Chuck's
> > proposal is no-go?
> >
> > From LWN.net "Another attempt to address the tracepoint ABI problem" [1]
> >
> > "The solution that was arrived at for now, as related by Torvalds,
> >  is to hold off on adding explicit tracepoints to the kernel. Instead,
> >  support will be added to make it easy for an application to attach a
> >  BPF script to any function in the kernel, with access to that function's
> >  arguments."
> >
> > [1] https://lwn.net/Articles/737530/
>
> What I remember is that Linus does not require us to avoid breaking user
> space applications that use tracepoints. Powertop however is an exception
> to this rule. Although it uses tracepoints, we must not break it. I also
> remember that Linus noticed that the purpose of many tracepoints is to
> allow users to trace a function and its arguments. Linus wants a better
> approach for tracing kernel functions than adding an explicit tracepoint
> to each kernel function. Maybe I wasn't listening carefully enough but I
> haven't heard Linus saying that we would not be allowed to add new
> tracepoints.

Like Steven, I wasn't in MS either and the sentence "is to hold off on adding explicit
tracepoints to the kernel" made me worry.

Thanks.

>
> Bart.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[Chuck Lever]

> On Oct 30, 2017, at 6:01 PM, Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> 
> These can be shared with all kernel ULPs, and more can easily be
> added as needed.
> 
> Signed-off-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> include/trace/events/rdma.h |  128 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 128 insertions(+)
> create mode 100644 include/trace/events/rdma.h
> 
> Hi-
> 
> I'm putting together a series of patches for v4.16 (or later) that
> add static ftrace trace points to the RPC-over-RDMA client imple-
> mentation. As part of that effort, I've constructed some trace point
> helpers that might be useful for other kernel ULPs.
> 
> So far this patch adds just helpers that xprtrdma needs. It is not
> complete, but additional helpers can be introduced as they are
> needed.
> 
> I'd like to hear comments about these, or please let me know if
> such helpers already exist and where xprtrdma can find them.

Now that the question of whether new trace points will be allowed
has been answered...

The helpers that are used for displaying errors in the kernel
log display the spelled out text, not the symbolic name of the
error code, as is done here.

Would the community prefer long-form text or symbolic error names
in trace reports?


> Thanks!
> 
> 
> diff --git a/include/trace/events/rdma.h b/include/trace/events/rdma.h
> new file mode 100644
> index 0000000..9d02fbe
> --- /dev/null
> +++ b/include/trace/events/rdma.h
> @@ -0,0 +1,128 @@
> +/*
> + * Copyright (c) 2017 Oracle.  All rights reserved.
> + */
> +
> +/*
> + * enum ib_event_type, from include/rdma/ib_verbs.h
> + */
> +
> +#define IB_EVENT_LIST				\
> +	ib_event(CQ_ERR)			\
> +	ib_event(QP_FATAL)			\
> +	ib_event(QP_REQ_ERR)			\
> +	ib_event(QP_ACCESS_ERR)			\
> +	ib_event(COMM_EST)			\
> +	ib_event(SQ_DRAINED)			\
> +	ib_event(PATH_MIG)			\
> +	ib_event(PATH_MIG_ERR)			\
> +	ib_event(DEVICE_FATAL)			\
> +	ib_event(PORT_ACTIVE)			\
> +	ib_event(PORT_ERR)			\
> +	ib_event(LID_CHANGE)			\
> +	ib_event(PKEY_CHANGE)			\
> +	ib_event(SM_CHANGE)			\
> +	ib_event(SRQ_ERR)			\
> +	ib_event(SRQ_LIMIT_REACHED)		\
> +	ib_event(QP_LAST_WQE_REACHED)		\
> +	ib_event(CLIENT_REREGISTER)		\
> +	ib_event(GID_CHANGE)			\
> +	ib_event_end(WQ_FATAL)
> +
> +#undef ib_event
> +#undef ib_event_end
> +
> +#define ib_event(x)		TRACE_DEFINE_ENUM(IB_EVENT_##x);
> +#define ib_event_end(x)		TRACE_DEFINE_ENUM(IB_EVENT_##x);
> +
> +IB_EVENT_LIST
> +
> +#undef ib_event
> +#undef ib_event_end
> +
> +#define ib_event(x)		{ IB_EVENT_##x, #x },
> +#define ib_event_end(x)		{ IB_EVENT_##x, #x }
> +
> +#define rdma_show_ib_event(x) \
> +		__print_symbolic(x, IB_EVENT_LIST)
> +
> +/*
> + * enum ib_wc_status type, from include/rdma/ib_verbs.h
> + */
> +#define IB_WC_STATUS_LIST			\
> +	ib_wc_status(SUCCESS)			\
> +	ib_wc_status(LOC_LEN_ERR)		\
> +	ib_wc_status(LOC_QP_OP_ERR)		\
> +	ib_wc_status(LOC_EEC_OP_ERR)		\
> +	ib_wc_status(LOC_PROT_ERR)		\
> +	ib_wc_status(WR_FLUSH_ERR)		\
> +	ib_wc_status(MW_BIND_ERR)		\
> +	ib_wc_status(BAD_RESP_ERR)		\
> +	ib_wc_status(LOC_ACCESS_ERR)		\
> +	ib_wc_status(REM_INV_REQ_ERR)		\
> +	ib_wc_status(REM_ACCESS_ERR)		\
> +	ib_wc_status(REM_OP_ERR)		\
> +	ib_wc_status(RETRY_EXC_ERR)		\
> +	ib_wc_status(RNR_RETRY_EXC_ERR)		\
> +	ib_wc_status(LOC_RDD_VIOL_ERR)		\
> +	ib_wc_status(REM_INV_RD_REQ_ERR)	\
> +	ib_wc_status(REM_ABORT_ERR)		\
> +	ib_wc_status(INV_EECN_ERR)		\
> +	ib_wc_status(INV_EEC_STATE_ERR)		\
> +	ib_wc_status(FATAL_ERR)			\
> +	ib_wc_status(RESP_TIMEOUT_ERR)		\
> +	ib_wc_status_end(GENERAL_ERR)
> +
> +#undef ib_wc_status
> +#undef ib_wc_status_end
> +
> +#define ib_wc_status(x)		TRACE_DEFINE_ENUM(IB_WC_##x);
> +#define ib_wc_status_end(x)	TRACE_DEFINE_ENUM(IB_WC_##x);
> +
> +IB_WC_STATUS_LIST
> +
> +#undef ib_wc_status
> +#undef ib_wc_status_end
> +
> +#define ib_wc_status(x)		{ IB_WC_##x, #x },
> +#define ib_wc_status_end(x)	{ IB_WC_##x, #x }
> +
> +#define rdma_show_wc_status(x) \
> +		__print_symbolic(x, IB_WC_STATUS_LIST)
> +
> +/*
> + * enum rdma_cm_event_type, from include/rdma/rdma_cm.h
> + */
> +#define RDMA_CM_EVENT_LIST			\
> +	rdma_cm_event(ADDR_RESOLVED)		\
> +	rdma_cm_event(ADDR_ERROR)		\
> +	rdma_cm_event(ROUTE_RESOLVED)		\
> +	rdma_cm_event(ROUTE_ERROR)		\
> +	rdma_cm_event(CONNECT_REQUEST)		\
> +	rdma_cm_event(CONNECT_RESPONSE)		\
> +	rdma_cm_event(CONNECT_ERROR)		\
> +	rdma_cm_event(UNREACHABLE)		\
> +	rdma_cm_event(REJECTED)			\
> +	rdma_cm_event(ESTABLISHED)		\
> +	rdma_cm_event(DISCONNECTED)		\
> +	rdma_cm_event(DEVICE_REMOVAL)		\
> +	rdma_cm_event(MULTICAST_JOIN)		\
> +	rdma_cm_event(MULTICAST_ERROR)		\
> +	rdma_cm_event(ADDR_CHANGE)		\
> +	rdma_cm_event_end(TIMEWAIT_EXIT)
> +
> +#undef rdma_cm_event
> +#undef rdma_cm_event_end
> +
> +#define rdma_cm_event(x)	TRACE_DEFINE_ENUM(RDMA_CM_EVENT_##x);
> +#define rdma_cm_event_end(x)	TRACE_DEFINE_ENUM(RDMA_CM_EVENT_##x);
> +
> +RDMA_CM_EVENT_LIST
> +
> +#undef rdma_cm_event
> +#undef rdma_cm_event_end
> +
> +#define rdma_cm_event(x)	{ RDMA_CM_EVENT_##x, #x },
> +#define rdma_cm_event_end(x)	{ RDMA_CM_EVENT_##x, #x }
> +
> +#define rdma_show_cm_event(x) \
> +		__print_symbolic(x, RDMA_CM_EVENT_LIST)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] rdma/ib: Add trace point macros to display human-readable values

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

On Thu, Nov 02, 2017 at 12:18:41PM -0400, Chuck Lever wrote:
>
> > On Oct 30, 2017, at 6:01 PM, Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > These can be shared with all kernel ULPs, and more can easily be
> > added as needed.
> >
> > Signed-off-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > include/trace/events/rdma.h |  128 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 128 insertions(+)
> > create mode 100644 include/trace/events/rdma.h
> >
> > Hi-
> >
> > I'm putting together a series of patches for v4.16 (or later) that
> > add static ftrace trace points to the RPC-over-RDMA client imple-
> > mentation. As part of that effort, I've constructed some trace point
> > helpers that might be useful for other kernel ULPs.
> >
> > So far this patch adds just helpers that xprtrdma needs. It is not
> > complete, but additional helpers can be introduced as they are
> > needed.
> >
> > I'd like to hear comments about these, or please let me know if
> > such helpers already exist and where xprtrdma can find them.
>
> Now that the question of whether new trace points will be allowed
> has been answered...
>
> The helpers that are used for displaying errors in the kernel
> log display the spelled out text, not the symbolic name of the
> error code, as is done here.
>
> Would the community prefer long-form text or symbolic error names
> in trace reports?

IMHO, short names are preferred. It is intended for the developers who
have access to the code, so anyway they will check the meaning if
necessary.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]