On Wed, Aug 12, 2026 at 04:42:57PM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] When CONFIG_PRINTK is disabled, dmesg redirection stalls the event > iterator perpetually because `pr_info` returns 0. > -- > > --- Patch [6]: [PATCH v5 06/18] tracing/remotes: Add dmesg tracefs file --- > commit b41c280df86084ff7600dad9b61a6fa3dbbdf1dd > Author: Vincent Donnefort <[email protected]> > > tracing/remotes: Add dmesg tracefs file > > When enabled, the dmesg tracefs file enables the redirection of all > events to dmesg. This is similar to tp_printk. > > > diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c > > index 1fbe2f3f20e99..3a5e0d84c1722 100644 > > --- a/kernel/trace/trace_remote.c > > +++ b/kernel/trace/trace_remote.c > > [ ... ] > > > @@ -882,6 +908,87 @@ static const struct file_operations trace_fops = { > > .release = trace_release, > > }; > > > > +static bool trace_remote_do_dmesg(struct trace_remote *remote) > > +{ > > + struct trace_remote_iterator *iter = smp_load_acquire(&remote->dmesg); > > + unsigned int max_events = 1000; > > + > > + if (!iter) > > + return false; > > + > > + trace_remote_iter_read_start(iter); > > + > > + while (trace_remote_iter_read_event(iter)) { > > + trace_seq_init(&iter->seq); > > + > > + trace_remote_iter_print_event(iter); > > + trace_seq_putc(&iter->seq, 0); > > + if (!pr_info("%s", seq_buf_str(&iter->seq.seq))) > > + break; > > [Severity: Medium] > When CONFIG_PRINTK is disabled, pr_info() is defined as no_printk, which > unconditionally returns 0. Does this cause the loop to always break early > and skip the call to trace_remote_iter_move() below? > > If so, it appears the background polling work would continually fail to drain > the trace remote buffer and run periodically without doing any work, as the > iterator never advances. > > > + > > + trace_remote_iter_move(iter); > > + > > + if (!(--max_events)) > > + break; > > + } > > + > > + trace_remote_iter_read_finished(iter); > > + > > + return !max_events; > > +} > > -- > Sashiko AI review ยท > https://sashiko.dev/#/patchset/[email protected]?part=6
Do we really want to support !CONFIG_PRINTK? In that case dmesg/dump_on_panic should just fail to enable... but is it really worth it? -- Vincent
