Re: [PATCH v2] dynamic debug: allow printing to trace event

From: Steven Rostedt
Date: Fri Aug 14 2020 - 13:15:37 EST


On Fri, 14 Aug 2020 15:31:51 +0200
Vincent Whitchurch <vincent.whitchurch@xxxxxxxx> wrote:
> index aa9ff9e1c0b3..f599ed21ecc5 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -27,13 +27,16 @@ struct _ddebug {
> * writes commands to <debugfs>/dynamic_debug/control
> */
> #define _DPRINTK_FLAGS_NONE 0
> -#define _DPRINTK_FLAGS_PRINT (1<<0) /* printk() a message using the format */
> +#define _DPRINTK_FLAGS_PRINTK (1<<0) /* printk() a message using the format */

The above looks like a cleanup unrelated to this patch, and probably
should be on its own.

> #define _DPRINTK_FLAGS_INCL_MODNAME (1<<1)
> #define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
> #define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
> #define _DPRINTK_FLAGS_INCL_TID (1<<4)
> +#define _DPRINTK_FLAGS_TRACE (1<<5)
> +#define _DPRINTK_FLAGS_PRINT (_DPRINTK_FLAGS_PRINTK | \
> + _DPRINTK_FLAGS_TRACE)
> #if defined DEBUG
> -#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
> +#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINTK
> #else
> #define _DPRINTK_FLAGS_DEFAULT 0
> #endif
> diff --git a/include/trace/events/printk.h b/include/trace/events/printk.h
> index 13d405b2fd8b..6c89121a1669 100644
> --- a/include/trace/events/printk.h
> +++ b/include/trace/events/printk.h
> @@ -7,7 +7,7 @@
>
> #include <linux/tracepoint.h>
>
> -TRACE_EVENT(console,
> +DECLARE_EVENT_CLASS(printk,
> TP_PROTO(const char *text, size_t len),
>
> TP_ARGS(text, len),
> @@ -31,6 +31,16 @@ TRACE_EVENT(console,
>
> TP_printk("%s", __get_str(msg))
> );
> +
> +DEFINE_EVENT(printk, console,
> + TP_PROTO(const char *text, size_t len),
> + TP_ARGS(text, len)
> +);
> +
> +DEFINE_EVENT(printk, dynamic,

Can we call this "dynamic_printk" or "printk_dynamic", as
trace_dynamic() is too generic.

> + TP_PROTO(const char *text, size_t len),
> + TP_ARGS(text, len)
> +);
> #endif /* _TRACE_PRINTK_H */
>
> /* This part must be outside protection */
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 1d012e597cc3..76fc3e33fe41 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -36,6 +36,7 @@
> #include <linux/sched.h>
> #include <linux/device.h>
> #include <linux/netdevice.h>
> +#include <trace/events/printk.h>
>
> #include <rdma/ib_verbs.h>
>
> @@ -84,11 +85,12 @@ static inline const char *trim_prefix(const char *path)
> }
>
> static struct { unsigned flag:8; char opt_char; } opt_array[] = {
> - { _DPRINTK_FLAGS_PRINT, 'p' },
> + { _DPRINTK_FLAGS_PRINTK, 'p' },

Again, this looks unrelated, and shouldn't be part of this patch.

> { _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
> { _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
> { _DPRINTK_FLAGS_INCL_LINENO, 'l' },
> { _DPRINTK_FLAGS_INCL_TID, 't' },
> + { _DPRINTK_FLAGS_TRACE, 'x' },
> { _DPRINTK_FLAGS_NONE, '_' },
> };
>

Other then the two comments above, I'm fine with this change.

Reviewed-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>

-- Steve