[PATCH] pstore/zone: Emit registration message as a single pr_info()

[PATCH] pstore/zone: Emit registration message as a single pr_info()

[Kartik Rajput]

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;
 
 	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) {
-- 
2.43.0

Re: [PATCH] pstore/zone: Emit registration message as a single pr_info()

[Kees Cook]

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

Re: [PATCH] pstore/zone: Emit registration message as a single pr_info()

[Kartik Rajput]

Hi Kees,

Thanks for reviewing the patch!

On 24/04/26 21:03, Kees Cook wrote:
> External email: Use caution opening links or attachments
> 
> 
> 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
> 

ACK. I will push v2 to use seq_buf instead of scnprintf calls.

Thanks,
Kartik