public inbox for linux-tegra@vger.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Kartik Rajput <kkartik@nvidia.com>,
	Tony Luck <tony.luck@intel.com>,
	"Guilherme G . Piccoli" <gpiccoli@igalia.com>,
	Jon Hunter <jonathanh@nvidia.com>,
	Thierry Reding <thierry.reding@kernel.org>,
	linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org
Subject: Re: [PATCH] pstore/zone: Emit registration message as a single pr_info()
Date: Fri, 24 Apr 2026 08:33:41 -0700	[thread overview]
Message-ID: <273C4CEB-8FC5-4420-BC74-691A58E254F2@kernel.org> (raw)
In-Reply-To: <20260424095820.1433700-1-kkartik@nvidia.com>



On April 24, 2026 2:58:20 AM PDT, Kartik Rajput <kkartik@nvidia.com> wrote:
>register_pstore_zone() prints its "registered ... as backend for ..."
>summary as a pr_info() followed by several pr_cont() calls.
>
>pr_cont() is not atomic and has no log level of its own. It appends
>to whichever line was most recently opened by a printk(). If a
>pr_err() or pr_warn() from another CPU or an interrupt handler
>preempts the pr_info() / pr_cont() sequence, it closes the
>continuation between the fragments. This can cause parts of the
>pstore registration message to appear at the wrong log level and be
>interleaved with other messages.
>
>Furthermore, this causes the detection of new warning and error
>messages in the kernel log to be unreliable.
>
>Format the registration line in a small local buffer using
>scnprintf() and emit it with a single pr_info() call, making the
>line atomic with respect to concurrent printk() callers.  No
>functional change to registration.
>
>Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
>---
> fs/pstore/zone.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
>diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c
>index a3b003f9a3a0..fbe0a252dd2a 100644
>--- a/fs/pstore/zone.c
>+++ b/fs/pstore/zone.c
>@@ -1301,6 +1301,8 @@ int register_pstore_zone(struct pstore_zone_info *info)
> {
> 	int err = -EINVAL;
> 	struct psz_context *cxt = &pstore_zone_cxt;
>+	char buf[256] = "";
>+	size_t len = 0;

Please use a seq_buf backed by a stack array instead of the scnprintf calls.

Otherwise, yes, seems good to keep this all on one line.

Thanks!

-Kees

> 
> 	if (info->total_size < 4096) {
> 		pr_warn("total_size must be >= 4096\n");
>@@ -1383,30 +1385,28 @@ int register_pstore_zone(struct pstore_zone_info *info)
> 	}
> 	cxt->pstore.data = cxt;
> 
>-	pr_info("registered %s as backend for", info->name);
> 	cxt->pstore.max_reason = info->max_reason;
> 	cxt->pstore.name = info->name;
> 	if (info->kmsg_size) {
> 		cxt->pstore.flags |= PSTORE_FLAGS_DMESG;
>-		pr_cont(" kmsg(%s",
>-			kmsg_dump_reason_str(cxt->pstore.max_reason));
>-		if (cxt->pstore_zone_info->panic_write)
>-			pr_cont(",panic_write");
>-		pr_cont(")");
>+		len += scnprintf(buf + len, sizeof(buf) - len, " kmsg(%s%s)",
>+			       kmsg_dump_reason_str(cxt->pstore.max_reason),
>+			       cxt->pstore_zone_info->panic_write ? ",panic_write" : "");
> 	}
> 	if (info->pmsg_size) {
> 		cxt->pstore.flags |= PSTORE_FLAGS_PMSG;
>-		pr_cont(" pmsg");
>+		len += scnprintf(buf + len, sizeof(buf) - len, " pmsg");
> 	}
> 	if (info->console_size) {
> 		cxt->pstore.flags |= PSTORE_FLAGS_CONSOLE;
>-		pr_cont(" console");
>+		len += scnprintf(buf + len, sizeof(buf) - len, " console");
> 	}
> 	if (info->ftrace_size) {
> 		cxt->pstore.flags |= PSTORE_FLAGS_FTRACE;
>-		pr_cont(" ftrace");
>+		len += scnprintf(buf + len, sizeof(buf) - len, " ftrace");
> 	}
>-	pr_cont("\n");
>+
>+	pr_info("registered %s as backend for%s\n", info->name, buf);
> 
> 	err = pstore_register(&cxt->pstore);
> 	if (err) {

-- 
Kees Cook

  reply	other threads:[~2026-04-24 15:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-24  9:58 [PATCH] pstore/zone: Emit registration message as a single pr_info() Kartik Rajput
2026-04-24 15:33 ` Kees Cook [this message]
2026-04-26  8:59   ` Kartik Rajput

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=273C4CEB-8FC5-4420-BC74-691A58E254F2@kernel.org \
    --to=kees@kernel.org \
    --cc=gpiccoli@igalia.com \
    --cc=jonathanh@nvidia.com \
    --cc=kkartik@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=thierry.reding@kernel.org \
    --cc=tony.luck@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox