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.

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

Reply via email to