From: Vincent Donnefort <vdonnefort@google.com>
To: sashiko-reviews@lists.linux.dev
Cc: linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v5 06/18] tracing/remotes: Add dmesg tracefs file
Date: Thu, 13 Aug 2026 15:02:20 +0100 [thread overview]
Message-ID: <an3ObE0FFGHK07EO@google.com> (raw)
In-Reply-To: <20260812164257.AD3A31F000E9@smtp.kernel.org>
On Wed, Aug 12, 2026 at 04:42:57PM +0000, sashiko-bot@kernel.org 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 <vdonnefort@google.com>
>
> 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/20260812161340.2417322-1-vdonnefort@google.com?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
next prev parent reply other threads:[~2026-08-13 14:03 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-12 16:13 [PATCH v5 00/18] tracing/remotes: Add printk, dump_on_panic and boot parameters Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 01/18] tracing/remotes: Gate tracefs files opening on trace remote registration Vincent Donnefort
2026-08-12 16:31 ` sashiko-bot
2026-08-13 13:16 ` Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 02/18] tracing/remotes: Release tracefs,eventfs on registration failure Vincent Donnefort
2026-08-12 16:25 ` sashiko-bot
2026-08-13 13:25 ` Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 03/18] tracing/remotes: Use kstrtobool for boolean tracefs files Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 04/18] tracing/remotes: Use a single per-remote polling work Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 05/18] tracing/simple_ring_buffer: Add support for compressed length Vincent Donnefort
2026-08-12 16:42 ` sashiko-bot
2026-08-13 14:00 ` Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 06/18] tracing/remotes: Add dmesg tracefs file Vincent Donnefort
2026-08-12 16:42 ` sashiko-bot
2026-08-13 14:02 ` Vincent Donnefort [this message]
2026-08-12 16:13 ` [PATCH v5 07/18] tracing/remotes: selftests: Add a test for the " Vincent Donnefort
2026-08-12 16:40 ` sashiko-bot
2026-08-13 14:06 ` Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 08/18] tracing/remotes: selftests: Prefix hypervisor folder Vincent Donnefort
2026-08-12 16:52 ` sashiko-bot
2026-08-12 16:13 ` [PATCH v5 09/18] ring-buffer: Use irqsave for the reader lock in ring_buffer_poll_remote Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 10/18] ring-buffer: Use panic-friendly locking in ring_buffer_iter interface Vincent Donnefort
2026-08-12 16:54 ` sashiko-bot
[not found] ` <20260812161340.2417322-15-vdonnefort@google.com>
2026-08-12 17:02 ` [PATCH v5 14/18] tracing/remotes: selftests: Add a test for the dump_on_panic tracefs file sashiko-bot
[not found] ` <20260812161340.2417322-14-vdonnefort@google.com>
2026-08-12 17:08 ` [PATCH v5 13/18] tracing/remotes: Add " sashiko-bot
[not found] ` <20260812161340.2417322-17-vdonnefort@google.com>
2026-08-12 17:11 ` [PATCH v5 16/18] tracing/remotes: Add trace_remote cmdline options sashiko-bot
2026-08-13 14:21 ` Vincent Donnefort
[not found] ` <20260812161340.2417322-16-vdonnefort@google.com>
2026-08-12 17:13 ` [PATCH v5 15/18] tracing/remotes: Add poll_ms tracefs file sashiko-bot
2026-08-13 14:20 ` Vincent Donnefort
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=an3ObE0FFGHK07EO@google.com \
--to=vdonnefort@google.com \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox