Re: [PATCH 4/4] tracing: Allow boot instances to have snapshot buffers

On Fri, 13 Jan 2023 17:08:09 -0700
Ross Zwisler [off-list ref] wrote:

quoted

+++ 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?

They are both empty (BSS) and initdata. So they get allocated at boot up
and freed at the end of boot.

quoted

+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.

Yeah, I'm willing to do this (as mentioned before).

Just two nits, other than that you can add:

Reviewed-by: Ross Zwisler <redacted>

Thanks!

-- Steve