Re: [PATCH v5 06/18] tracing/remotes: Add dmesg tracefs file

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