Re: [PATCH v11 03/19] dyndbg: add write-to-tracefs code

Vincent Whitchurch
2022-01-14T11:46:56Z

On Fri, Jan 07, 2022 at 06:29:26AM +0100, Jim Cromie wrote:
> adds: dynamic_trace()
> uses trace_console() temporarily to issue printk:console event
> uses internal-ish __ftrace_trace_stack code:
> 4-context buffer stack, barriers per Steve
>
> call it from new funcs:
> dynamic_printk() - print to both syslog/tracefs
> dynamic_dev_printk() - dev-print to both syslog/tracefs
>
> These handle both _DPRINTK_FLAGS_PRINTK and _DPRINTK_FLAGS_TRACE
> cases, allowing to vsnprintf the message once and use it for both,
> skipping past the KERN_DEBUG character for tracing.
>
> Finally, adjust the callers: __dynamic_{pr_debug,{,net,ib}dev_dbg},
> replacing printk and dev_printk with the new funcs above.
>
> The _DPRINTK_FLAGS_TRACE flag character s 'T', so the following finds
> all callsites enabled for tracing:
>
> grep -P =p?T /proc/dynamic_debug/control
>
> Enabling debug-to-tracefs is 2 steps:
>
> # event enable
> echo 1 > /sys/kernel/tracing/events/dyndbg/enable
> # callsite enable
> echo module foo +T > /proc/dynamic_debug/control
>
> This patch,~1,~2 are based upon:
> https://lore.kernel.org/lkml/20200825153338.17061-1-vincent.whitchurch@axis.com/
>
> .. with simplification of temporarily reusing trace_console() rather
> than adding a new printk:dyndbg event. Soon, add 2 new events
> capturing the pr_debug & dev_dbg() args.

The example above does not match the code in this patch since the
dyndbg:* events are only added in a later patch. Perhaps you could
reorder this patch stack so that you don't use trace_console() in this
patch just to replace it with the new events in the next patch?

>
> CC: vincent.whitchurch@axis.com
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
> .../admin-guide/dynamic-debug-howto.rst | 1 +
> lib/dynamic_debug.c | 155 +++++++++++++++---
> 2 files changed, 130 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
[...]
> @@ -723,29 +822,33 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
> {
> struct va_format vaf;
> va_list args;
> + unsigned int flags;
>
> va_start(args, fmt);
>
> vaf.fmt = fmt;
> vaf.va = &args;
> + flags = descriptor->flags;
>
> if (ibdev && ibdev->dev.parent) {
> char buf[PREFIX_SIZE] = "";
>
> - dev_printk_emit(LOGLEVEL_DEBUG, ibdev->dev.parent,
> - "%s%s %s %s: %pV",
> - dynamic_emit_prefix(descriptor, buf),
> - dev_driver_string(ibdev->dev.parent),
> - dev_name(ibdev->dev.parent),
> - dev_name(&ibdev->dev),
> - &vaf);
> + dynamic_dev_printk(flags, ibdev->dev.parent,
> + "%s%s %s %s: %pV",
> + dynamic_emit_prefix(descriptor, buf),
> + dev_driver_string(ibdev->dev.parent),
> + dev_name(ibdev->dev.parent),
> + dev_name(&ibdev->dev),
> + &vaf);
> } else if (ibdev) {
> - printk(KERN_DEBUG "%s: %pV", dev_name(&ibdev->dev), &vaf);
> + dynamic_printk(flags, KERN_DEBUG "%s%s: %pV",
> + dev_name(&ibdev->dev), &vaf);
> } else {
> - printk(KERN_DEBUG "(NULL ib_device): %pV", &vaf);
> + dynamic_printk(flags, KERN_DEBUG "(NULL ip_device): %pV",
> + &vaf);
> }
>
> - va_end(args);
> +va_end(args);

This looks like an unintentional whitespace change?