On Mon, Aug 17, 2026 at 02:12:51PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
> to consider:
> - [Medium] The `dump_on_panic` command-line option sets `remote->panic_on = 
> true` but fails to dynamically register the panic handler if the trace buffer 
> is already loaded.
> --
> 
> --- Patch [16]: [PATCH v6 16/18] tracing/remotes: Add trace_remote cmdline 
> options ---
> commit 16551de261e3bdeffe51c1a5227c6a115ddbe725
> Author: Vincent Donnefort <[email protected]>
> 
> tracing/remotes: Add trace_remote cmdline options
>     
> This commit introduces command-line options for trace remotes, allowing
> configuration of features like dump_on_panic, dmesg, buf_size, and poll,
> as well as enabling specific trace events automatically on boot.
> 
> > diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> > index 204d5bef7a8c4..34847463d9e43 100644
> > --- a/kernel/trace/trace_remote.c
> > +++ b/kernel/trace/trace_remote.c
> 
> [ ... ]
> 
> > @@ -1170,6 +1172,126 @@ static int poll_ms_show(struct seq_file *s, void 
> > *unused)
> > +static void trace_remote_apply_cmdline_opts(struct trace_remote *remote, 
> > char *cmdline)
> > +{
> > +   bool dmesg_on = false;
> > +   char *opt;
> > +   int ret;
> > +
> > +   while ((opt = strsep(&cmdline, "^"))) {
> > +           if (!*opt)
> > +                   continue;
> > +
> > +           if (!strcmp(opt, "dump_on_panic")) {
> > +                   remote->panic_on = true;
> 
> [Severity: Medium]
> If the trace remote was configured across multiple command line arguments, and
> an earlier argument already enabled events (which loads the trace buffer via
> trace_remote_enable_tracing()), will setting remote->panic_on = true here
> fail to register the panic notifier?
> 
> Since this doesn't call trace_remote_panic_load() if the buffer is already
> loaded, it appears the panic handler might never be registered under this
> configuration pattern.

Interesting. I will fix that

> 
> > +           } else if (!strcmp(opt, "dmesg")) {
> > +                   dmesg_on = true;
> > +           } else if (!strncmp(opt, "buf_size=", 9)) {
> > +                   /* buf_size can only be applied if the buffer is 
> > unloaded */
> > +                   if (trace_remote_loaded(remote))
> > +                           pr_warn("Cannot apply buf_size after events are 
> > enabled\n");
> > +                   else
> > +                           remote->trace_buffer_size = memparse(opt + 9, 
> > NULL);
> 
> [ ... ]
> 
> -- 
> Sashiko AI review ยท 
> https://sashiko.dev/#/patchset/[email protected]?part=16

-- 
Vincent

Reply via email to