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 Fri, 13 Jan 2023 17:08:09 -0700
Ross Zwisler <zwisler@google.com> wrote:

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

>
> > +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 <zwisler@google.com>

Thanks!

-- Steve

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