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

[Vincent Whitchurch]

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@xxxxxxxx/
> 
> .. 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@xxxxxxxx
> Signed-off-by: Jim Cromie <jim.cromie@xxxxxxxxx>
> ---
>  .../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?