10:20 p.m.
FYI, we noticed the following commit:
commit: e3c4ff6d8c949fa9a9ea1bd005bf1967efe09d5d ("EDAC: Remove EDAC_MM_EDAC")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
in testcase: unixbench
with following parameters:
runtime: 300s
nr_task: 100%
test: pipe
cpufreq_governor: performance
test-description: UnixBench is the original BYTE UNIX benchmark suite aims to test
performance of Unix-like system.
test-url: https://github.com/kdlucas/byte-unixbench
on test machine: 88 threads Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz with 48G memory
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
kern :err : [ 31.707838] EDAC sbridge: Couldn't find mci handler
kern :err : [ 31.707839] EDAC sbridge: Couldn't find mci handler
kern :err : [ 31.707841] EDAC sbridge: Failed to register device with error -22.
To reproduce:
git clone https://github.com/01org/lkp-tests.git
cd lkp-tests
bin/lkp install job.yaml # job file is attached in this email
bin/lkp run job.yaml
Thanks,
Xiaolong
4:19 a.m.
On Tue, Apr 25, 2017 at 10:20:09AM +0800, kernel test robot wrote:
FYI, we noticed the following commit:
commit: e3c4ff6d8c949fa9a9ea1bd005bf1967efe09d5d ("EDAC: Remove EDAC_MM_EDAC")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
Can you send me full dmesg of a kernel with this patch reverted?
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)
--
9:51 p.m.
On 04/25, Borislav Petkov wrote:
On Tue, Apr 25, 2017 at 10:20:09AM +0800, kernel test robot wrote:
>
> FYI, we noticed the following commit:
>
> commit: e3c4ff6d8c949fa9a9ea1bd005bf1967efe09d5d ("EDAC: Remove
EDAC_MM_EDAC")
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
Can you send me full dmesg of a kernel with this patch reverted?
Sure, I'll provide it later.
Thanks,
Xiaolong
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)
--
1:50 a.m.
On 04/25, Borislav Petkov wrote:
On Tue, Apr 25, 2017 at 10:20:09AM +0800, kernel test robot wrote:
>
> FYI, we noticed the following commit:
>
> commit: e3c4ff6d8c949fa9a9ea1bd005bf1967efe09d5d ("EDAC: Remove
EDAC_MM_EDAC")
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
Can you send me full dmesg of a kernel with this patch reverted?
Please see attached.
Thanks,
Xiaolong
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)
--
3:20 a.m.
On Wed, Apr 26, 2017 at 01:50:35PM +0800, Ye Xiaolong wrote:
On 04/25, Borislav Petkov wrote:
>On Tue, Apr 25, 2017 at 10:20:09AM +0800, kernel test robot wrote:
>>
>> FYI, we noticed the following commit:
>>
>> commit: e3c4ff6d8c949fa9a9ea1bd005bf1967efe09d5d ("EDAC: Remove
EDAC_MM_EDAC")
>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
>
>Can you send me full dmesg of a kernel with this patch reverted?
Please see attached.
Thanks, I see what the problem is: this can happen even without my patch
- my patch just made it happen unconditionally now :)
As a temporary workaround, you could boot with "ghes.disable=1" to make
sb_edac load successfully again.
Now on to the bigger question: so Tony we have this ghes_edac thing
which is not a module as it gets stuff delivered from ghes through
ghes_edac_report_mem_error() and ACPI_APEI_GHES is bool itself so it
can't be a module and so on.
Now, the problem then is that ghes_edac gets registered first and the
platform-specific drivers like sb_edac then fail to register as they
come later when built as modules.
Which kinda needs us to talk about prio and what we want to do: do we
want for ghes_edac to have a higher priority in registering with the
system - this is basically what Mauro was aiming at:
77c5f5d2f212 ("ghes_edac: Register at EDAC core the BIOS report")
or do we want the platform-specific drivers to have prio?
Or do we want the user to decide?
I mean, a lot has changed since then, here's what I'm seeing from here:
from all the boxes I have access to - and there are a bunch - GHES is
either colossally broken or doesn't work as expected or hasn't been
implemented properly or ...
So my reality of it is that I really don't trust BIOS writers (what
else is new?! :)) and I think we're better off having the
platform-specific drivers get higher priority and use ghes_edac only
as a fallback. And the platform-specific drivers get much more work and
scrutiny than some BIOS somewhere. And I'd tend to make that be the
preferred decision but still leave it to the user to decide by stating
edac=prefer_ghes or whatever.
Oh and I've hever heard of people actually using that ghes_edac driver
at all while I do get bug reports about the prominent platform-specific
ones.
So I'd really appreciate your thoughts here...
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)
--
6:08 a.m.
Btw,
the least we should do before the merge window is below because
otherwise now with CONFIG_EDAC=y, apei_ghes will be always builtin.
---
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 27337ef9874b..96afb2aeed18 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -55,7 +55,6 @@ config EDAC_DECODE_MCE
config EDAC_GHES
bool "Output ACPI APEI/GHES BIOS detected errors via EDAC"
depends on ACPI_APEI_GHES && (EDAC=y)
- default y
help
Not all machines support hardware-driven error report. Some of those
provide a BIOS-driven error report mechanism via ACPI, using the
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)
--
6:27 a.m.
New subject: [PATCH] EDAC, ghes: Do not enable it by default
Now as a proper patch:
From: Borislav Petkov <bp(a)suse.de>
Date: Wed, 26 Apr 2017 12:22:08 +0200
Subject: [PATCH] EDAC, ghes: Do not enable it by default
Leave it to the user to decide whether to enable this or not. Otherwise,
platform-specific drivers won't initialize (currently, EDAC supports
only a single platform driver loaded).
Signed-off-by: Borislav Petkov <bp(a)suse.de>
---
drivers/edac/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 27337ef9874b..96afb2aeed18 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -55,7 +55,6 @@ config EDAC_DECODE_MCE
config EDAC_GHES
bool "Output ACPI APEI/GHES BIOS detected errors via EDAC"
depends on ACPI_APEI_GHES && (EDAC=y)
- default y
help
Not all machines support hardware-driven error report. Some of those
provide a BIOS-driven error report mechanism via ACPI, using the
--
2.11.0
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)
--
5:06 p.m.
On Wed, Apr 26, 2017 at 09:20:05AM +0200, Borislav Petkov wrote:
On Wed, Apr 26, 2017 at 01:50:35PM +0800, Ye Xiaolong wrote:
> On 04/25, Borislav Petkov wrote:
> >On Tue, Apr 25, 2017 at 10:20:09AM +0800, kernel test robot wrote:
> >>
> >> FYI, we noticed the following commit:
> >>
> >> commit: e3c4ff6d8c949fa9a9ea1bd005bf1967efe09d5d ("EDAC: Remove
EDAC_MM_EDAC")
> >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> >
> >Can you send me full dmesg of a kernel with this patch reverted?
>
> Please see attached.
Thanks, I see what the problem is: this can happen even without my patch
- my patch just made it happen unconditionally now :)
As a temporary workaround, you could boot with "ghes.disable=1" to make
sb_edac load successfully again.
Now on to the bigger question: so Tony we have this ghes_edac thing
which is not a module as it gets stuff delivered from ghes through
ghes_edac_report_mem_error() and ACPI_APEI_GHES is bool itself so it
can't be a module and so on.
Now, the problem then is that ghes_edac gets registered first and the
platform-specific drivers like sb_edac then fail to register as they
come later when built as modules.
Which kinda needs us to talk about prio and what we want to do: do we
want for ghes_edac to have a higher priority in registering with the
system - this is basically what Mauro was aiming at:
77c5f5d2f212 ("ghes_edac: Register at EDAC core the BIOS report")
or do we want the platform-specific drivers to have prio?
Or do we want the user to decide?
If there were a sane way for the ghes_edac driver to find out
that it was running on a platform that supported these GHES
error reports from the BIOS, then it would be able to decide
whether to register.
But I don't think there is :-(
So we can either let the user pick. Or change the EDAC registration
mechanism to allow a "better" driver to bump out some other driver
that just beat us to register first.
I.e. we'd keep ghes_edac as a built-in driver, and so it would always
register first.
Then we'd try to load some other driver: sb_edac, skx_edac, etc. If
they get through the platform tests to find they are on the right
cpu model, and they find all the right PCIe devices to access memory
controller registers ... then they call edac_mc_add_mc() to tell
EDAC core they want to run things ... and that supplants ghes_edac.
-Tony
6:20 p.m.
On Wed, Apr 26, 2017 at 02:06:02PM -0700, Luck, Tony wrote:
Then we'd try to load some other driver: sb_edac, skx_edac, etc.
If
they get through the platform tests to find they are on the right
cpu model, and they find all the right PCIe devices to access memory
controller registers ... then they call edac_mc_add_mc() to tell
EDAC core they want to run things ... and that supplants ghes_edac.
Yeah, but that means a serious EDAC core surgery as we've never
supported more than one EDAC driver. What would be easier, IMO,
is to move the reporting functionality from ghes_edac.c to
drivers/acpi/apei/ghes.c and export only the EDAC tracepoint so that
GHES can call into it. Through a notifier or so.
This way we'll get rid of the ghes_edac.c thing completely the
functionality will be completely part of ghes.c in drivers/acpi/ and
then we can load our platform drivers just the same.
But that's definitely future work.
I'd still like to apply the oneliner for 4.12 so that the bug Xiaolong
reported doesn't happen. Ok?
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)
--
6:52 p.m.
On Thu, Apr 27, 2017 at 12:20:09AM +0200, Borislav Petkov wrote:
On Wed, Apr 26, 2017 at 02:06:02PM -0700, Luck, Tony wrote:
> Then we'd try to load some other driver: sb_edac, skx_edac, etc. If
> they get through the platform tests to find they are on the right
> cpu model, and they find all the right PCIe devices to access memory
> controller registers ... then they call edac_mc_add_mc() to tell
> EDAC core they want to run things ... and that supplants ghes_edac.
Yeah, but that means a serious EDAC core surgery as we've never
supported more than one EDAC driver. What would be easier, IMO,
is to move the reporting functionality from ghes_edac.c to
drivers/acpi/apei/ghes.c and export only the EDAC tracepoint so that
GHES can call into it. Through a notifier or so.
Ah, that sounds simpler.
This way we'll get rid of the ghes_edac.c thing completely the
functionality will be completely part of ghes.c in drivers/acpi/ and
then we can load our platform drivers just the same.
But that's definitely future work.
I'd still like to apply the oneliner for 4.12 so that the bug Xiaolong
reported doesn't happen. Ok?
Sure. That makes sense.
-Tony
1:13 p.m.
On Wed, Apr 26, 2017 at 02:06:02PM -0700, Luck, Tony wrote:
If there were a sane way for the ghes_edac driver to find out
that it was running on a platform that supported these GHES
error reports from the BIOS, then it would be able to decide
whether to register.
But I don't think there is :-(
So we can either let the user pick.
So, in thinking about this more, I am gravitating towards letting the
user pick. For the simple reason that, most of the time she/he should
know better than some heuristic mechanism we have in the kernel.
I mean, how do you detect if the firmware generates correct error
reports and the specific platform driver is not needed?
Lemme see how ugly the conversion of ghes_edac into a proper module
would look...
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)
--
1271
days inactive
1283
days old
11 comments
4 participants
participants (4)
-
Borislav Petkov -
kernel test robot -
Luck, Tony -
Ye Xiaolong