lkml.org 
[lkml]   [2023]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/4] tracing: Allow boot instances to have snapshot buffers
On Wed, Jan 11, 2023 at 09:56:40AM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> Add to ftrace_boot_snapshot, "=<instance>" name, where the instance will
> get a snapshot buffer, and will take a snapshot at the end of boot (which
> will save the boot traces).
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> .../admin-guide/kernel-parameters.txt | 9 +++
> kernel/trace/trace.c | 77 +++++++++++++++++--
> 2 files changed, 79 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 7a7f41652719..f4e87b17427f 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1532,6 +1532,15 @@
> boot up that is likely to be overridden by user space
> start up functionality.
>
> + Optionally, the snapshot can also be defined for a tracing
> + instance that was created by the trace_instance= command
> + line parameter.
> +
> + trace_instance=foo,sched_switch ftrace_boot_snapshot=foo
> +
> + The above will cause the "foo" tracing instance to trigger
> + a snapshot at the end of boot up.
> +
> ftrace_dump_on_oops[=orig_cpu]
> [FTRACE] will dump the trace buffers on oops.
> If no parameter is passed, ftrace will dump
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 3cb9bbc0f076..d445789dc247 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -192,6 +192,9 @@ static bool snapshot_at_boot;
> static char boot_instance_info[COMMAND_LINE_SIZE] __initdata;
> static int boot_instance_index;
>
> +static char boot_snapshot_info[COMMAND_LINE_SIZE] __initdata;

For x86 machines at least COMMAND_LINE_SIZE is pretty big (2048), so between
boot_instance_info and boot_snapshot_info we are using an entire 4k of memory.
It seems unlikely that any user would need a string this long for these
options. Should we trim this down to something smaller?

> +static int boot_snapshot_index;
> +
> static int __init set_cmdline_ftrace(char *str)
> {
> strlcpy(bootup_tracer_buf, str, MAX_TRACER_SIZE);
> @@ -228,9 +231,22 @@ __setup("traceoff_on_warning", stop_trace_on_warning);
>
> static int __init boot_alloc_snapshot(char *str)
> {
> - allocate_snapshot = true;
> - /* We also need the main ring buffer expanded */
> - ring_buffer_expanded = true;
> + char *slot = boot_snapshot_info + boot_snapshot_index;
> + int left = COMMAND_LINE_SIZE - boot_snapshot_index;

sizeof(boot_snapshot_info) is a bit safer than COMMAND_LINE_SIZE so they don't
get out of sync, plus we may also want to shrink it a bit as mentioned above.

Just two nits, other than that you can add:

Reviewed-by: Ross Zwisler <zwisler@google.com>

\
 
 \ /
  Last update: 2023-03-26 23:41    [W:0.101 / U:3.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site