All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Vincent Donnefort" <vdonnefort@google.com>
Cc: linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v5 06/18] tracing/remotes: Add dmesg tracefs file
Date: Wed, 12 Aug 2026 16:42:57 +0000	[thread overview]
Message-ID: <20260812164257.AD3A31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260812161340.2417322-7-vdonnefort@google.com>

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

  reply	other threads:[~2026-08-12 16:42 UTC|newest]

Thread overview: 30+ 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-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-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-12 16:13 ` [PATCH v5 06/18] tracing/remotes: Add dmesg tracefs file Vincent Donnefort
2026-08-12 16:42   ` sashiko-bot [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-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
2026-08-12 16:13 ` [PATCH v5 11/18] ring-buffer: Add ring_buffer_read_remote_meta_page() Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 12/18] ring-buffer: Add kerneldoc for ring_buffer_poll_remote Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 13/18] tracing/remotes: Add dump_on_panic tracefs file Vincent Donnefort
2026-08-12 17:08   ` sashiko-bot
2026-08-12 16:13 ` [PATCH v5 14/18] tracing/remotes: selftests: Add a test for the " Vincent Donnefort
2026-08-12 17:02   ` sashiko-bot
2026-08-12 16:13 ` [PATCH v5 15/18] tracing/remotes: Add poll_ms " Vincent Donnefort
2026-08-12 17:13   ` sashiko-bot
2026-08-12 16:13 ` [PATCH v5 16/18] tracing/remotes: Add trace_remote cmdline options Vincent Donnefort
2026-08-12 17:11   ` sashiko-bot
2026-08-12 16:13 ` [PATCH v5 17/18] Documentation: tracing/remotes: Add detailed tracefs layout Vincent Donnefort
2026-08-12 16:13 ` [PATCH v5 18/18] Documentation/kernel-parameters: Add trace_remote 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=20260812164257.AD3A31F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vdonnefort@google.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.