Re: [PATCH v3 1/3] tracing: Show real address for trace event arguments

From: Jon Hunter
Date: Tue Apr 20 2021 - 07:16:57 EST



On 19/04/2021 19:22, Steven Rostedt wrote:
> On Mon, 19 Apr 2021 14:08:14 +0100
> Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>
>> I have encountered the following crash on a couple of our ARM64 Jetson
>> platforms and bisect is pointing to this change. The crash I am seeing
>> is on boot when I am directing the trace prints to the console by adding
>> 'tp_printk trace_event="cpu_frequency,cpu_frequency_limits"' to the
>> kernel command line and enabling CONFIG_BOOTTIME_TRACING. Reverting this
>> change does fix the problem. Let me know if you have any thoughts.
>
> Thanks for the report. I was able to reproduce this on x86 as well.
>
> It's the tp_printk that's the problem. Does this fix it for you?
>
> -- Steve
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 66a4ad93b5e9..f1ce4be7a499 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3580,7 +3580,11 @@ static char *trace_iter_expand_format(struct trace_iterator *iter)
> {
> char *tmp;
>
> - if (iter->fmt == static_fmt_buf)
> + /*
> + * iter->tr is NULL when used with tp_printk, which makes
> + * this get called where it is not safe to call krealloc().
> + */
> + if (!iter->tr || iter->fmt == static_fmt_buf)
> return NULL;
>
> tmp = krealloc(iter->fmt, iter->fmt_size + STATIC_FMT_BUF_SIZE,
> @@ -3799,7 +3803,7 @@ const char *trace_event_format(struct trace_iterator *iter, const char *fmt)
> if (WARN_ON_ONCE(!fmt))
> return fmt;
>
> - if (iter->tr->trace_flags & TRACE_ITER_HASH_PTR)
> + if (!iter->tr || iter->tr->trace_flags & TRACE_ITER_HASH_PTR)
> return fmt;
>
> p = fmt;
> @@ -9931,7 +9935,7 @@ void __init early_trace_init(void)
> {
> if (tracepoint_printk) {
> tracepoint_print_iter =
> - kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
> + kzalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
> if (MEM_FAIL(!tracepoint_print_iter,
> "Failed to allocate trace iterator\n"))
> tracepoint_printk = 0;
>


Yes that works for me thanks!

Tested-by: Jon Hunter <jonathanh@xxxxxxxxxx>

Cheers!
Jon

--
nvpublic