* [PATCH] EDAC/mce_amd: Do not print a stray newline
@ 2026-07-12 3:16 Borislav Petkov
2026-07-13 18:44 ` Yazen Ghannam
0 siblings, 1 reply; 6+ messages in thread
From: Borislav Petkov @ 2026-07-12 3:16 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: linux-edac, LKML, Borislav Petkov (AMD)
From: "Borislav Petkov (AMD)" <bp@alien8.de>
With error records which don't have FRU text, an empty newline is
printed like this:
[28014.094610] mce: [Hardware Error]: Machine check events logged
[28014.094674] [Hardware Error]: Corrected error, no action required.
[28014.094687] [Hardware Error]: CPU:11 (19:44:1) MC0_STATUS[-|CE|MiscV|AddrV|-|-|SyndV|CECC|-|-|-]: 0x9c20400001010135
[28014.094724] [Hardware Error]: Error Addr: 0x000000019ba9a974
[28014.094732] [Hardware Error]: IPID: 0x001000b000000000, Syndrome: 0x000000081a1f2505
[28014.094744] [Hardware Error]: Load Store Unit Ext. Error Code: 1
[28014.094748] [Hardware Error]: cache level: L1, tx: DATA, mem-tx: DRD
Move the \n into the last printk() call where it belongs.
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
drivers/edac/mce_amd.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index bd252cb3c38e..ec7395cb7a70 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -870,12 +870,10 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
memcpy(&frutext[0], &err->vendor.amd.synd1, 8);
memcpy(&frutext[8], &err->vendor.amd.synd2, 8);
- pr_emerg(HW_ERR "FRU Text: %s", frutext);
+ pr_emerg(HW_ERR "FRU Text: %s\n", frutext);
}
}
- pr_cont("\n");
-
decode_smca_error(m);
goto err_code;
}
--
2.53.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] EDAC/mce_amd: Do not print a stray newline
2026-07-12 3:16 [PATCH] EDAC/mce_amd: Do not print a stray newline Borislav Petkov
@ 2026-07-13 18:44 ` Yazen Ghannam
2026-07-13 23:22 ` Borislav Petkov
0 siblings, 1 reply; 6+ messages in thread
From: Yazen Ghannam @ 2026-07-13 18:44 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-edac, LKML, Borislav Petkov (AMD)
On Sat, Jul 11, 2026 at 08:16:48PM -0700, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@alien8.de>
>
> With error records which don't have FRU text, an empty newline is
> printed like this:
>
> [28014.094610] mce: [Hardware Error]: Machine check events logged
> [28014.094674] [Hardware Error]: Corrected error, no action required.
> [28014.094687] [Hardware Error]: CPU:11 (19:44:1) MC0_STATUS[-|CE|MiscV|AddrV|-|-|SyndV|CECC|-|-|-]: 0x9c20400001010135
> [28014.094724] [Hardware Error]: Error Addr: 0x000000019ba9a974
> [28014.094732] [Hardware Error]: IPID: 0x001000b000000000, Syndrome: 0x000000081a1f2505
>
> [28014.094744] [Hardware Error]: Load Store Unit Ext. Error Code: 1
> [28014.094748] [Hardware Error]: cache level: L1, tx: DATA, mem-tx: DRD
>
> Move the \n into the last printk() call where it belongs.
>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Thanks,
Yazen
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] EDAC/mce_amd: Do not print a stray newline
2026-07-13 18:44 ` Yazen Ghannam
@ 2026-07-13 23:22 ` Borislav Petkov
2026-07-14 10:28 ` Srivatsa S. Bhat
0 siblings, 1 reply; 6+ messages in thread
From: Borislav Petkov @ 2026-07-13 23:22 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: Borislav Petkov, linux-edac, LKML
On Mon, Jul 13, 2026 at 02:44:02PM -0400, Yazen Ghannam wrote:
> On Sat, Jul 11, 2026 at 08:16:48PM -0700, Borislav Petkov wrote:
> > From: "Borislav Petkov (AMD)" <bp@alien8.de>
> >
> > With error records which don't have FRU text, an empty newline is
> > printed like this:
> >
> > [28014.094610] mce: [Hardware Error]: Machine check events logged
> > [28014.094674] [Hardware Error]: Corrected error, no action required.
> > [28014.094687] [Hardware Error]: CPU:11 (19:44:1) MC0_STATUS[-|CE|MiscV|AddrV|-|-|SyndV|CECC|-|-|-]: 0x9c20400001010135
> > [28014.094724] [Hardware Error]: Error Addr: 0x000000019ba9a974
> > [28014.094732] [Hardware Error]: IPID: 0x001000b000000000, Syndrome: 0x000000081a1f2505
> >
> > [28014.094744] [Hardware Error]: Load Store Unit Ext. Error Code: 1
> > [28014.094748] [Hardware Error]: cache level: L1, tx: DATA, mem-tx: DRD
> >
> > Move the \n into the last printk() call where it belongs.
> >
> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
>
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Yeah, Sashiko has a point:
https://sashiko.dev/#/patchset/20260712031648.486110-1-bp%40kernel.org
So we want the error records to be compact and fit in a block so that you can
recognize them easier and thus not print each thing with a newline.
But those things are not unconditional so you cannot always know.
I need to think of a slick scheme what to do here.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] EDAC/mce_amd: Do not print a stray newline
2026-07-13 23:22 ` Borislav Petkov
@ 2026-07-14 10:28 ` Srivatsa S. Bhat
2026-07-14 13:04 ` Yazen Ghannam
0 siblings, 1 reply; 6+ messages in thread
From: Srivatsa S. Bhat @ 2026-07-14 10:28 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Yazen Ghannam, Borislav Petkov, linux-edac, LKML
On Mon, Jul 13, 2026 at 04:22:19PM -0700, Borislav Petkov wrote:
> On Mon, Jul 13, 2026 at 02:44:02PM -0400, Yazen Ghannam wrote:
> > On Sat, Jul 11, 2026 at 08:16:48PM -0700, Borislav Petkov wrote:
> > > From: "Borislav Petkov (AMD)" <bp@alien8.de>
> > >
> > > With error records which don't have FRU text, an empty newline is
> > > printed like this:
> > >
> > > [28014.094610] mce: [Hardware Error]: Machine check events logged
> > > [28014.094674] [Hardware Error]: Corrected error, no action required.
> > > [28014.094687] [Hardware Error]: CPU:11 (19:44:1) MC0_STATUS[-|CE|MiscV|AddrV|-|-|SyndV|CECC|-|-|-]: 0x9c20400001010135
> > > [28014.094724] [Hardware Error]: Error Addr: 0x000000019ba9a974
> > > [28014.094732] [Hardware Error]: IPID: 0x001000b000000000, Syndrome: 0x000000081a1f2505
> > >
> > > [28014.094744] [Hardware Error]: Load Store Unit Ext. Error Code: 1
> > > [28014.094748] [Hardware Error]: cache level: L1, tx: DATA, mem-tx: DRD
> > >
> > > Move the \n into the last printk() call where it belongs.
> > >
> > > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> >
> > Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
>
> Yeah, Sashiko has a point:
>
> https://sashiko.dev/#/patchset/20260712031648.486110-1-bp%40kernel.org
>
> So we want the error records to be compact and fit in a block so that you can
> recognize them easier and thus not print each thing with a newline.
>
> But those things are not unconditional so you cannot always know.
>
> I need to think of a slick scheme what to do here.
>
Do you mean something other than moving the newline print into an else
block like this?
---
drivers/edac/mce_amd.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index bd252cb3c38e..60d1a62e1a95 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -870,12 +870,16 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
memcpy(&frutext[0], &err->vendor.amd.synd1, 8);
memcpy(&frutext[8], &err->vendor.amd.synd2, 8);
- pr_emerg(HW_ERR "FRU Text: %s", frutext);
+ pr_emerg(HW_ERR "FRU Text: %s\n", frutext);
}
+ } else {
+ /*
+ * Explicit line termination to handle nested
+ * conditional prints correctly.
+ */
+ pr_cont("\n");
}
- pr_cont("\n");
-
decode_smca_error(m);
goto err_code;
}
--
2.43.0
Regards,
Srivatsa
Microsoft Linux Systems Group
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] EDAC/mce_amd: Do not print a stray newline
2026-07-14 10:28 ` Srivatsa S. Bhat
@ 2026-07-14 13:04 ` Yazen Ghannam
2026-07-20 5:13 ` Srivatsa S. Bhat
0 siblings, 1 reply; 6+ messages in thread
From: Yazen Ghannam @ 2026-07-14 13:04 UTC (permalink / raw)
To: Srivatsa S. Bhat; +Cc: Borislav Petkov, Borislav Petkov, linux-edac, LKML
On Tue, Jul 14, 2026 at 03:58:52PM +0530, Srivatsa S. Bhat wrote:
> On Mon, Jul 13, 2026 at 04:22:19PM -0700, Borislav Petkov wrote:
> > On Mon, Jul 13, 2026 at 02:44:02PM -0400, Yazen Ghannam wrote:
> > > On Sat, Jul 11, 2026 at 08:16:48PM -0700, Borislav Petkov wrote:
> > > > From: "Borislav Petkov (AMD)" <bp@alien8.de>
> > > >
> > > > With error records which don't have FRU text, an empty newline is
> > > > printed like this:
> > > >
> > > > [28014.094610] mce: [Hardware Error]: Machine check events logged
> > > > [28014.094674] [Hardware Error]: Corrected error, no action required.
> > > > [28014.094687] [Hardware Error]: CPU:11 (19:44:1) MC0_STATUS[-|CE|MiscV|AddrV|-|-|SyndV|CECC|-|-|-]: 0x9c20400001010135
> > > > [28014.094724] [Hardware Error]: Error Addr: 0x000000019ba9a974
> > > > [28014.094732] [Hardware Error]: IPID: 0x001000b000000000, Syndrome: 0x000000081a1f2505
> > > >
> > > > [28014.094744] [Hardware Error]: Load Store Unit Ext. Error Code: 1
> > > > [28014.094748] [Hardware Error]: cache level: L1, tx: DATA, mem-tx: DRD
> > > >
> > > > Move the \n into the last printk() call where it belongs.
> > > >
> > > > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> > >
> > > Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> >
> > Yeah, Sashiko has a point:
> >
> > https://sashiko.dev/#/patchset/20260712031648.486110-1-bp%40kernel.org
> >
> > So we want the error records to be compact and fit in a block so that you can
> > recognize them easier and thus not print each thing with a newline.
> >
> > But those things are not unconditional so you cannot always know.
> >
> > I need to think of a slick scheme what to do here.
> >
>
> Do you mean something other than moving the newline print into an else
> block like this?
>
> ---
> drivers/edac/mce_amd.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
> index bd252cb3c38e..60d1a62e1a95 100644
> --- a/drivers/edac/mce_amd.c
> +++ b/drivers/edac/mce_amd.c
> @@ -870,12 +870,16 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
> memcpy(&frutext[0], &err->vendor.amd.synd1, 8);
> memcpy(&frutext[8], &err->vendor.amd.synd2, 8);
>
> - pr_emerg(HW_ERR "FRU Text: %s", frutext);
> + pr_emerg(HW_ERR "FRU Text: %s\n", frutext);
> }
> + } else {
> + /*
> + * Explicit line termination to handle nested
> + * conditional prints correctly.
> + */
> + pr_cont("\n");
> }
>
> - pr_cont("\n");
> -
> decode_smca_error(m);
> goto err_code;
> }
> --
Another option could be to build up each newline string and print it
once. The gist is to get rid of pr_cont() usage.
I've attached an old patch for that. Would this work?
Thanks,
Yazen
From 2035e5b829de2834aef8d84de24bafb5f4e5e03b Mon Sep 17 00:00:00 2001
From: Yazen Ghannam <yazen.ghannam@amd.com>
Date: Thu, 16 Dec 2021 19:06:14 +0000
Subject: [PATCH] [WIP] EDAC/mce_amd: Construct string before printing
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
drivers/edac/mce_amd.c | 64 +++++++++++++++++++++++++++++-------------
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index bd252cb3c38e..ea6dc7437cae 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -744,6 +744,8 @@ static void decode_smca_error(struct mce *m)
{
enum smca_bank_types bank_type = smca_get_bank_type(m->extcpu, m->bank);
u8 xec = XEC(m->status, xec_mask);
+ char buf[256];
+ char *p, *end;
if (bank_type >= N_SMCA_BANK_TYPES)
return;
@@ -753,7 +755,13 @@ static void decode_smca_error(struct mce *m)
return;
}
- pr_emerg(HW_ERR "%s Ext. Error Code: %d", smca_get_long_name(bank_type), xec);
+ memset(buf, 0, sizeof(buf));
+ p = buf;
+ end = buf + sizeof(buf);
+
+ p += scnprintf(p, end - p, "%s Ext. Error Code: %d", smca_get_long_name(bank_type), xec);
+
+ pr_emerg(HW_ERR "%s", buf);
if ((bank_type == SMCA_UMC || bank_type == SMCA_UMC_V2) &&
xec == 0 && decode_dram_ecc)
@@ -762,26 +770,33 @@ static void decode_smca_error(struct mce *m)
static inline void amd_decode_err_code(u16 ec)
{
+ char buf[256];
+ char *p, *end;
+
if (INT_ERROR(ec)) {
pr_emerg(HW_ERR "internal: %s\n", UU_MSG(ec));
return;
}
- pr_emerg(HW_ERR "cache level: %s", LL_MSG(ec));
+ memset(buf, 0, sizeof(buf));
+ p = buf;
+ end = buf + sizeof(buf);
+
+ p += scnprintf(p, end - p, "cache level: %s", LL_MSG(ec));
if (BUS_ERROR(ec))
- pr_cont(", mem/io: %s", II_MSG(ec));
+ p += scnprintf(p, end - p, ", mem/io: %s", II_MSG(ec));
else
- pr_cont(", tx: %s", TT_MSG(ec));
+ p += scnprintf(p, end - p, ", tx: %s", TT_MSG(ec));
if (MEM_ERROR(ec) || BUS_ERROR(ec)) {
- pr_cont(", mem-tx: %s", R4_MSG(ec));
+ p += scnprintf(p, end - p, ", mem-tx: %s", R4_MSG(ec));
if (BUS_ERROR(ec))
- pr_cont(", part-proc: %s (%s)", PP_MSG(ec), TO_MSG(ec));
+ p += scnprintf(p, end - p, ", part-proc: %s (%s)", PP_MSG(ec), TO_MSG(ec));
}
- pr_cont("\n");
+ pr_emerg(HW_ERR "%s", buf);
}
static const char *decode_error_status(struct mce *m)
@@ -807,6 +822,8 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
struct mce_hw_err *err = to_mce_hw_err(m);
unsigned int fam = x86_family(m->cpuid);
u32 mca_config_lo = 0, dummy;
+ char buf[256];
+ char *p, *end;
int ecc;
if (m->kflags & MCE_HANDLED_CEC)
@@ -814,7 +831,11 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
pr_emerg(HW_ERR "%s\n", decode_error_status(m));
- pr_emerg(HW_ERR "CPU:%d (%x:%x:%x) MC%d_STATUS[%s|%s|%s|%s|%s",
+ memset(buf, 0, sizeof(buf));
+ p = buf;
+ end = buf + sizeof(buf);
+
+ p += scnprintf(p, end - p, "CPU:%d (%x:%x:%x) MC%d_STATUS[%s|%s|%s|%s|%s",
m->extcpu,
fam, x86_model(m->cpuid), x86_stepping(m->cpuid),
m->bank,
@@ -829,28 +850,29 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
rdmsr_safe(MSR_AMD64_SMCA_MCx_CONFIG(m->bank), &mca_config_lo, &dummy);
if (mca_config_lo & MCI_CONFIG_MCAX)
- pr_cont("|%s", ((m->status & MCI_STATUS_TCC) ? "TCC" : "-"));
+ p += scnprintf(p, end - p, "|%s", ((m->status & MCI_STATUS_TCC) ? "TCC" : "-"));
- pr_cont("|%s", ((m->status & MCI_STATUS_SYNDV) ? "SyndV" : "-"));
+ p += scnprintf(p, end - p, "|%s", ((m->status & MCI_STATUS_SYNDV) ? "SyndV" : "-"));
}
/* do the two bits[14:13] together */
ecc = (m->status >> 45) & 0x3;
if (ecc)
- pr_cont("|%sECC", ((ecc == 2) ? "C" : "U"));
+ p += scnprintf(p, end - p, "|%sECC", ((ecc == 2) ? "C" : "U"));
if (fam >= 0x15) {
- pr_cont("|%s", (m->status & MCI_STATUS_DEFERRED ? "Deferred" : "-"));
+ p += scnprintf(p, end - p, "|%s", (m->status & MCI_STATUS_DEFERRED ? "Deferred" : "-"));
/* F15h, bank4, bit 43 is part of McaStatSubCache. */
if (fam != 0x15 || m->bank != 4)
- pr_cont("|%s", (m->status & MCI_STATUS_POISON ? "Poison" : "-"));
+ p += scnprintf(p, end - p, "|%s", (m->status & MCI_STATUS_POISON ? "Poison" : "-"));
}
if (fam >= 0x17)
- pr_cont("|%s", (m->status & MCI_STATUS_SCRUB ? "Scrub" : "-"));
+ p += scnprintf(p, end - p, "|%s", (m->status & MCI_STATUS_SCRUB ? "Scrub" : "-"));
- pr_cont("]: 0x%016llx\n", m->status);
+ p += scnprintf(p, end - p, "]: 0x%016llx\n", m->status);
+ pr_emerg(HW_ERR "%s", buf);
if (m->status & MCI_STATUS_ADDRV)
pr_emerg(HW_ERR "Error Addr: 0x%016llx\n", m->addr);
@@ -859,10 +881,14 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
pr_emerg(HW_ERR "PPIN: 0x%016llx\n", m->ppin);
if (boot_cpu_has(X86_FEATURE_SMCA)) {
- pr_emerg(HW_ERR "IPID: 0x%016llx", m->ipid);
+ memset(buf, 0, sizeof(buf));
+ p = buf;
+
+ p += scnprintf(p, end - p, "IPID: 0x%016llx", m->ipid);
if (m->status & MCI_STATUS_SYNDV) {
- pr_cont(", Syndrome: 0x%016llx\n", m->synd);
+ p += scnprintf(p, end - p, ", Syndrome: 0x%016llx", m->synd);
+ pr_emerg(HW_ERR "%s", buf);
if (mca_config_lo & MCI_CONFIG_FRUTEXT) {
char frutext[17];
@@ -872,10 +898,10 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
pr_emerg(HW_ERR "FRU Text: %s", frutext);
}
+ } else {
+ pr_emerg(HW_ERR "%s", buf);
}
- pr_cont("\n");
-
decode_smca_error(m);
goto err_code;
}
--
2.54.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] EDAC/mce_amd: Do not print a stray newline
2026-07-14 13:04 ` Yazen Ghannam
@ 2026-07-20 5:13 ` Srivatsa S. Bhat
0 siblings, 0 replies; 6+ messages in thread
From: Srivatsa S. Bhat @ 2026-07-20 5:13 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: Borislav Petkov, Borislav Petkov, linux-edac, LKML
Hi Yazen,
On Tue, Jul 14, 2026 at 09:04:55AM -0400, Yazen Ghannam wrote:
> On Tue, Jul 14, 2026 at 03:58:52PM +0530, Srivatsa S. Bhat wrote:
> > On Mon, Jul 13, 2026 at 04:22:19PM -0700, Borislav Petkov wrote:
> > > On Mon, Jul 13, 2026 at 02:44:02PM -0400, Yazen Ghannam wrote:
> > > > On Sat, Jul 11, 2026 at 08:16:48PM -0700, Borislav Petkov wrote:
> > > > > From: "Borislav Petkov (AMD)" <bp@alien8.de>
> > > > >
> > > > > With error records which don't have FRU text, an empty newline is
> > > > > printed like this:
> > > > >
> > > > > [28014.094610] mce: [Hardware Error]: Machine check events logged
> > > > > [28014.094674] [Hardware Error]: Corrected error, no action required.
> > > > > [28014.094687] [Hardware Error]: CPU:11 (19:44:1) MC0_STATUS[-|CE|MiscV|AddrV|-|-|SyndV|CECC|-|-|-]: 0x9c20400001010135
> > > > > [28014.094724] [Hardware Error]: Error Addr: 0x000000019ba9a974
> > > > > [28014.094732] [Hardware Error]: IPID: 0x001000b000000000, Syndrome: 0x000000081a1f2505
> > > > >
> > > > > [28014.094744] [Hardware Error]: Load Store Unit Ext. Error Code: 1
> > > > > [28014.094748] [Hardware Error]: cache level: L1, tx: DATA, mem-tx: DRD
> > > > >
> > > > > Move the \n into the last printk() call where it belongs.
> > > > >
> > > > > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> > > >
> > > > Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> > >
> > > Yeah, Sashiko has a point:
> > >
> > > https://sashiko.dev/#/patchset/20260712031648.486110-1-bp%40kernel.org
> > >
> > > So we want the error records to be compact and fit in a block so that you can
> > > recognize them easier and thus not print each thing with a newline.
> > >
> > > But those things are not unconditional so you cannot always know.
> > >
> > > I need to think of a slick scheme what to do here.
> > >
> >
> > Do you mean something other than moving the newline print into an else
> > block like this?
> >
> > ---
> > drivers/edac/mce_amd.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
> > index bd252cb3c38e..60d1a62e1a95 100644
> > --- a/drivers/edac/mce_amd.c
> > +++ b/drivers/edac/mce_amd.c
> > @@ -870,12 +870,16 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
> > memcpy(&frutext[0], &err->vendor.amd.synd1, 8);
> > memcpy(&frutext[8], &err->vendor.amd.synd2, 8);
> >
> > - pr_emerg(HW_ERR "FRU Text: %s", frutext);
> > + pr_emerg(HW_ERR "FRU Text: %s\n", frutext);
> > }
> > + } else {
> > + /*
> > + * Explicit line termination to handle nested
> > + * conditional prints correctly.
> > + */
> > + pr_cont("\n");
> > }
> >
> > - pr_cont("\n");
> > -
> > decode_smca_error(m);
> > goto err_code;
> > }
> > --
>
> Another option could be to build up each newline string and print it
> once. The gist is to get rid of pr_cont() usage.
>
I don't think getting rid of pr_cont() was the intention behind the
original patch by Boris; he was trying to address a user-visible
stray newline in the logs, if I understood correctly.
> I've attached an old patch for that. Would this work?
>
The approach to build up each string fully before printing it that you
demonstrated below does look like a nice improvement in general, but
I'm afraid it may not be a good fit for this particular subsystem
though, for the following reasons:
1. Given the EDAC subsystem's role in handling/reporting hardware
errors, I think the logging preference for this subsystem would be
to get the error message out as soon as possible, as the machine is
already likely to be in trouble if we are printing out kernel logs at
the criticality level of "emergency". So, delaying these prints to
construct the full string may not be desirable, as doing so can
potentially cripple the ability to get even the error indicators
known upto that point printed out (in case of further occurrence of
hardware errors).
2. The static buffer sizing for the character string (256) is
potentially prone to buffer overflows in the future as and when these
print statements evolve (without taking the buffer size into
careful consideration for every change to the string).
3. This is a much larger (cleanup) change than what Boris had posted
earlier (which was ~2 lines).
Thoughts?
Regards,
Srivatsa
Microsoft Linux Systems Group
>
>
> From 2035e5b829de2834aef8d84de24bafb5f4e5e03b Mon Sep 17 00:00:00 2001
> From: Yazen Ghannam <yazen.ghannam@amd.com>
> Date: Thu, 16 Dec 2021 19:06:14 +0000
> Subject: [PATCH] [WIP] EDAC/mce_amd: Construct string before printing
>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
> drivers/edac/mce_amd.c | 64 +++++++++++++++++++++++++++++-------------
> 1 file changed, 45 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
> index bd252cb3c38e..ea6dc7437cae 100644
> --- a/drivers/edac/mce_amd.c
> +++ b/drivers/edac/mce_amd.c
> @@ -744,6 +744,8 @@ static void decode_smca_error(struct mce *m)
> {
> enum smca_bank_types bank_type = smca_get_bank_type(m->extcpu, m->bank);
> u8 xec = XEC(m->status, xec_mask);
> + char buf[256];
> + char *p, *end;
>
> if (bank_type >= N_SMCA_BANK_TYPES)
> return;
> @@ -753,7 +755,13 @@ static void decode_smca_error(struct mce *m)
> return;
> }
>
> - pr_emerg(HW_ERR "%s Ext. Error Code: %d", smca_get_long_name(bank_type), xec);
> + memset(buf, 0, sizeof(buf));
> + p = buf;
> + end = buf + sizeof(buf);
> +
> + p += scnprintf(p, end - p, "%s Ext. Error Code: %d", smca_get_long_name(bank_type), xec);
> +
> + pr_emerg(HW_ERR "%s", buf);
>
> if ((bank_type == SMCA_UMC || bank_type == SMCA_UMC_V2) &&
> xec == 0 && decode_dram_ecc)
> @@ -762,26 +770,33 @@ static void decode_smca_error(struct mce *m)
>
> static inline void amd_decode_err_code(u16 ec)
> {
> + char buf[256];
> + char *p, *end;
> +
> if (INT_ERROR(ec)) {
> pr_emerg(HW_ERR "internal: %s\n", UU_MSG(ec));
> return;
> }
>
> - pr_emerg(HW_ERR "cache level: %s", LL_MSG(ec));
> + memset(buf, 0, sizeof(buf));
> + p = buf;
> + end = buf + sizeof(buf);
> +
> + p += scnprintf(p, end - p, "cache level: %s", LL_MSG(ec));
>
> if (BUS_ERROR(ec))
> - pr_cont(", mem/io: %s", II_MSG(ec));
> + p += scnprintf(p, end - p, ", mem/io: %s", II_MSG(ec));
> else
> - pr_cont(", tx: %s", TT_MSG(ec));
> + p += scnprintf(p, end - p, ", tx: %s", TT_MSG(ec));
>
> if (MEM_ERROR(ec) || BUS_ERROR(ec)) {
> - pr_cont(", mem-tx: %s", R4_MSG(ec));
> + p += scnprintf(p, end - p, ", mem-tx: %s", R4_MSG(ec));
>
> if (BUS_ERROR(ec))
> - pr_cont(", part-proc: %s (%s)", PP_MSG(ec), TO_MSG(ec));
> + p += scnprintf(p, end - p, ", part-proc: %s (%s)", PP_MSG(ec), TO_MSG(ec));
> }
>
> - pr_cont("\n");
> + pr_emerg(HW_ERR "%s", buf);
> }
>
> static const char *decode_error_status(struct mce *m)
> @@ -807,6 +822,8 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
> struct mce_hw_err *err = to_mce_hw_err(m);
> unsigned int fam = x86_family(m->cpuid);
> u32 mca_config_lo = 0, dummy;
> + char buf[256];
> + char *p, *end;
> int ecc;
>
> if (m->kflags & MCE_HANDLED_CEC)
> @@ -814,7 +831,11 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
>
> pr_emerg(HW_ERR "%s\n", decode_error_status(m));
>
> - pr_emerg(HW_ERR "CPU:%d (%x:%x:%x) MC%d_STATUS[%s|%s|%s|%s|%s",
> + memset(buf, 0, sizeof(buf));
> + p = buf;
> + end = buf + sizeof(buf);
> +
> + p += scnprintf(p, end - p, "CPU:%d (%x:%x:%x) MC%d_STATUS[%s|%s|%s|%s|%s",
> m->extcpu,
> fam, x86_model(m->cpuid), x86_stepping(m->cpuid),
> m->bank,
> @@ -829,28 +850,29 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
> rdmsr_safe(MSR_AMD64_SMCA_MCx_CONFIG(m->bank), &mca_config_lo, &dummy);
>
> if (mca_config_lo & MCI_CONFIG_MCAX)
> - pr_cont("|%s", ((m->status & MCI_STATUS_TCC) ? "TCC" : "-"));
> + p += scnprintf(p, end - p, "|%s", ((m->status & MCI_STATUS_TCC) ? "TCC" : "-"));
>
> - pr_cont("|%s", ((m->status & MCI_STATUS_SYNDV) ? "SyndV" : "-"));
> + p += scnprintf(p, end - p, "|%s", ((m->status & MCI_STATUS_SYNDV) ? "SyndV" : "-"));
> }
>
> /* do the two bits[14:13] together */
> ecc = (m->status >> 45) & 0x3;
> if (ecc)
> - pr_cont("|%sECC", ((ecc == 2) ? "C" : "U"));
> + p += scnprintf(p, end - p, "|%sECC", ((ecc == 2) ? "C" : "U"));
>
> if (fam >= 0x15) {
> - pr_cont("|%s", (m->status & MCI_STATUS_DEFERRED ? "Deferred" : "-"));
> + p += scnprintf(p, end - p, "|%s", (m->status & MCI_STATUS_DEFERRED ? "Deferred" : "-"));
>
> /* F15h, bank4, bit 43 is part of McaStatSubCache. */
> if (fam != 0x15 || m->bank != 4)
> - pr_cont("|%s", (m->status & MCI_STATUS_POISON ? "Poison" : "-"));
> + p += scnprintf(p, end - p, "|%s", (m->status & MCI_STATUS_POISON ? "Poison" : "-"));
> }
>
> if (fam >= 0x17)
> - pr_cont("|%s", (m->status & MCI_STATUS_SCRUB ? "Scrub" : "-"));
> + p += scnprintf(p, end - p, "|%s", (m->status & MCI_STATUS_SCRUB ? "Scrub" : "-"));
>
> - pr_cont("]: 0x%016llx\n", m->status);
> + p += scnprintf(p, end - p, "]: 0x%016llx\n", m->status);
> + pr_emerg(HW_ERR "%s", buf);
>
> if (m->status & MCI_STATUS_ADDRV)
> pr_emerg(HW_ERR "Error Addr: 0x%016llx\n", m->addr);
> @@ -859,10 +881,14 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
> pr_emerg(HW_ERR "PPIN: 0x%016llx\n", m->ppin);
>
> if (boot_cpu_has(X86_FEATURE_SMCA)) {
> - pr_emerg(HW_ERR "IPID: 0x%016llx", m->ipid);
> + memset(buf, 0, sizeof(buf));
> + p = buf;
> +
> + p += scnprintf(p, end - p, "IPID: 0x%016llx", m->ipid);
>
> if (m->status & MCI_STATUS_SYNDV) {
> - pr_cont(", Syndrome: 0x%016llx\n", m->synd);
> + p += scnprintf(p, end - p, ", Syndrome: 0x%016llx", m->synd);
> + pr_emerg(HW_ERR "%s", buf);
> if (mca_config_lo & MCI_CONFIG_FRUTEXT) {
> char frutext[17];
>
> @@ -872,10 +898,10 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
>
> pr_emerg(HW_ERR "FRU Text: %s", frutext);
> }
> + } else {
> + pr_emerg(HW_ERR "%s", buf);
> }
>
> - pr_cont("\n");
> -
> decode_smca_error(m);
> goto err_code;
> }
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-20 5:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-12 3:16 [PATCH] EDAC/mce_amd: Do not print a stray newline Borislav Petkov
2026-07-13 18:44 ` Yazen Ghannam
2026-07-13 23:22 ` Borislav Petkov
2026-07-14 10:28 ` Srivatsa S. Bhat
2026-07-14 13:04 ` Yazen Ghannam
2026-07-20 5:13 ` Srivatsa S. Bhat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome