* [PATCH] edac: avoid mce decoding crash after edac driver unloaded
@ 2012-05-05 1:20 Chen Gong
2012-05-05 1:57 ` Chen Gong
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Chen Gong @ 2012-05-05 1:20 UTC (permalink / raw)
To: mchehab; +Cc: linux-edac, linux-kernel, stable, Chen Gong
Some edac drivers register themselves as mce decoders via
notifier_chain. But in current notifier_chain implementation logic,
it doesn't accept same notifier registered twice. If so, it will be
wrong when removing the element from the list. For example, on one
SandyBridge platform, remove module sb_edac and then trigger one
error, it will hit oops because it has no mce decoder registered
but related notifier_chain still points to an invalid callback
function. Here is an example:
Call Trace:
[<ffffffff8150ef6a>] atomic_notifier_call_chain+0x1a/0x20
[<ffffffff8102b936>] mce_log+0x46/0x180
[<ffffffff8102eaea>] apei_mce_report_mem_error+0x4a/0x60
[<ffffffff812e19d2>] ghes_do_proc+0x192/0x210
[<ffffffff812e2066>] ghes_proc+0x46/0x70
[<ffffffff812e20d8>] ghes_notify_sci+0x48/0x80
[<ffffffff8150ef05>] notifier_call_chain+0x55/0x80
[<ffffffff81076f1a>] __blocking_notifier_call_chain+0x5a/0x80
[<ffffffff812aea11>] ? acpi_os_wait_events_complete+0x23/0x23
[<ffffffff81076f56>] blocking_notifier_call_chain+0x16/0x20
[<ffffffff812ddc4d>] acpi_hed_notify+0x19/0x1b
[<ffffffff812b16bd>] acpi_device_notify+0x19/0x1b
[<ffffffff812beb38>] acpi_ev_notify_dispatch+0x67/0x7f
[<ffffffff812aea3a>] acpi_os_execute_deferred+0x29/0x36
[<ffffffff81069dc2>] process_one_work+0x132/0x450
[<ffffffff8106bbcb>] worker_thread+0x17b/0x3c0
[<ffffffff8106ba50>] ? manage_workers+0x120/0x120
[<ffffffff81070aee>] kthread+0x9e/0xb0
[<ffffffff81514724>] kernel_thread_helper+0x4/0x10
[<ffffffff81070a50>] ? kthread_freezable_should_stop+0x70/0x70
[<ffffffff81514720>] ? gs_change+0x13/0x13
Code: f3 49 89 d4 45 85 ed 4d 89 c6 48 8b 0f 74 48 48 85 c9 75 17 eb 41
0f 1f 80 00 00 00 00 41 83 ed 01 4c 89 f9 74 22 4d 85 ff 74 1d <4c> 8b
79 08 4c 89 e2 48 89 de 48 89 cf ff 11 4d 85 f6 74 04 41
RIP [<ffffffff8150eef6>] notifier_call_chain+0x46/0x80
RSP <ffff88042868fb20>
CR2: ffffffffa01af838
---[ end trace 0100930068e73e6f ]---
BUG: unable to handle kernel paging request at fffffffffffffff8
IP: [<ffffffff810705b0>] kthread_data+0x10/0x20
PGD 1a0d067 PUD 1a0e067 PMD 0
Oops: 0000 [#2] SMP
Only i7core_edac and sb_edac have such issues because they have more
than one memory controller which means they have to register mce
decoder many times.
Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
---
drivers/edac/i7core_edac.c | 12 ++++++++++--
drivers/edac/sb_edac.c | 12 ++++++++++--
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 85226cc..1852a52 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -2216,6 +2216,7 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
{
struct mem_ctl_info *mci = i7core_dev->mci;
struct i7core_pvt *pvt;
+ static int once;
if (unlikely(!mci || !mci->pvt_info)) {
debugf0("MC: " __FILE__ ": %s(): dev = %p\n",
@@ -2234,7 +2235,10 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
if (pvt->enable_scrub)
disable_sdram_scrub_setting(mci);
- mce_unregister_decode_chain(&i7_mce_dec);
+ if (!once) {
+ mce_unregister_decode_chain(&i7_mce_dec);
+ once = 1;
+ }
/* Disable EDAC polling */
i7core_pci_ctl_release(pvt);
@@ -2253,6 +2257,7 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
struct mem_ctl_info *mci;
struct i7core_pvt *pvt;
int rc, channels, csrows;
+ static int once;
/* Check the number of active and not disabled channels */
rc = i7core_get_active_channels(i7core_dev->socket, &channels, &csrows);
@@ -2336,7 +2341,10 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
/* DCLK for scrub rate setting */
pvt->dclk_freq = get_dclk_freq();
- mce_register_decode_chain(&i7_mce_dec);
+ if (!once) {
+ mce_register_decode_chain(&i7_mce_dec);
+ once = 1;
+ }
return 0;
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index a203536..20fabc8 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -1655,6 +1655,7 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
{
struct mem_ctl_info *mci = sbridge_dev->mci;
struct sbridge_pvt *pvt;
+ static int once;
if (unlikely(!mci || !mci->pvt_info)) {
debugf0("MC: " __FILE__ ": %s(): dev = %p\n",
@@ -1669,7 +1670,10 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
debugf0("MC: " __FILE__ ": %s(): mci = %p, dev = %p\n",
__func__, mci, &sbridge_dev->pdev[0]->dev);
- mce_unregister_decode_chain(&sbridge_mce_dec);
+ if (!once) {
+ mce_unregister_decode_chain(&sbridge_mce_dec);
+ once = 1;
+ }
/* Remove MC sysfs nodes */
edac_mc_del_mc(mci->dev);
@@ -1685,6 +1689,7 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
struct mem_ctl_info *mci;
struct sbridge_pvt *pvt;
int rc, channels, csrows;
+ static int once;
/* Check the number of active and not disabled channels */
rc = sbridge_get_active_channels(sbridge_dev->bus, &channels, &csrows);
@@ -1738,7 +1743,10 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
goto fail0;
}
- mce_register_decode_chain(&sbridge_mce_dec);
+ if (!once) {
+ mce_register_decode_chain(&sbridge_mce_dec);
+ once = 1;
+ }
return 0;
fail0:
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] edac: avoid mce decoding crash after edac driver unloaded
2012-05-05 1:20 [PATCH] edac: avoid mce decoding crash after edac driver unloaded Chen Gong
@ 2012-05-05 1:57 ` Chen Gong
2012-05-05 10:05 ` Borislav Petkov
2012-05-07 7:03 ` Chen Gong
2 siblings, 0 replies; 10+ messages in thread
From: Chen Gong @ 2012-05-05 1:57 UTC (permalink / raw)
To: Chen Gong; +Cc: mchehab, linux-edac, linux-kernel, stable
于 2012/5/5 9:20, Chen Gong 写道:
> Some edac drivers register themselves as mce decoders via
> notifier_chain. But in current notifier_chain implementation logic,
> it doesn't accept same notifier registered twice. If so, it will be
> wrong when removing the element from the list. For example, on one
> SandyBridge platform, remove module sb_edac and then trigger one
> error, it will hit oops because it has no mce decoder registered
> but related notifier_chain still points to an invalid callback
> function. Here is an example:
>
> Call Trace:
> [<ffffffff8150ef6a>] atomic_notifier_call_chain+0x1a/0x20
> [<ffffffff8102b936>] mce_log+0x46/0x180
> [<ffffffff8102eaea>] apei_mce_report_mem_error+0x4a/0x60
> [<ffffffff812e19d2>] ghes_do_proc+0x192/0x210
> [<ffffffff812e2066>] ghes_proc+0x46/0x70
> [<ffffffff812e20d8>] ghes_notify_sci+0x48/0x80
> [<ffffffff8150ef05>] notifier_call_chain+0x55/0x80
> [<ffffffff81076f1a>] __blocking_notifier_call_chain+0x5a/0x80
> [<ffffffff812aea11>] ? acpi_os_wait_events_complete+0x23/0x23
> [<ffffffff81076f56>] blocking_notifier_call_chain+0x16/0x20
> [<ffffffff812ddc4d>] acpi_hed_notify+0x19/0x1b
> [<ffffffff812b16bd>] acpi_device_notify+0x19/0x1b
> [<ffffffff812beb38>] acpi_ev_notify_dispatch+0x67/0x7f
> [<ffffffff812aea3a>] acpi_os_execute_deferred+0x29/0x36
> [<ffffffff81069dc2>] process_one_work+0x132/0x450
> [<ffffffff8106bbcb>] worker_thread+0x17b/0x3c0
> [<ffffffff8106ba50>] ? manage_workers+0x120/0x120
> [<ffffffff81070aee>] kthread+0x9e/0xb0
> [<ffffffff81514724>] kernel_thread_helper+0x4/0x10
> [<ffffffff81070a50>] ? kthread_freezable_should_stop+0x70/0x70
> [<ffffffff81514720>] ? gs_change+0x13/0x13
> Code: f3 49 89 d4 45 85 ed 4d 89 c6 48 8b 0f 74 48 48 85 c9 75 17 eb 41
> 0f 1f 80 00 00 00 00 41 83 ed 01 4c 89 f9 74 22 4d 85 ff 74 1d <4c> 8b
> 79 08 4c 89 e2 48 89 de 48 89 cf ff 11 4d 85 f6 74 04 41
> RIP [<ffffffff8150eef6>] notifier_call_chain+0x46/0x80
> RSP <ffff88042868fb20>
> CR2: ffffffffa01af838
> ---[ end trace 0100930068e73e6f ]---
> BUG: unable to handle kernel paging request at fffffffffffffff8
> IP: [<ffffffff810705b0>] kthread_data+0x10/0x20
> PGD 1a0d067 PUD 1a0e067 PMD 0
> Oops: 0000 [#2] SMP
>
> Only i7core_edac and sb_edac have such issues because they have more
> than one memory controller which means they have to register mce
> decoder many times.
>
> Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
> ---
> drivers/edac/i7core_edac.c | 12 ++++++++++--
> drivers/edac/sb_edac.c | 12 ++++++++++--
> 2 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
> index 85226cc..1852a52 100644
> --- a/drivers/edac/i7core_edac.c
> +++ b/drivers/edac/i7core_edac.c
> @@ -2216,6 +2216,7 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
> {
> struct mem_ctl_info *mci = i7core_dev->mci;
> struct i7core_pvt *pvt;
> + static int once;
>
> if (unlikely(!mci || !mci->pvt_info)) {
> debugf0("MC: " __FILE__ ": %s(): dev = %p\n",
> @@ -2234,7 +2235,10 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
> if (pvt->enable_scrub)
> disable_sdram_scrub_setting(mci);
>
> - mce_unregister_decode_chain(&i7_mce_dec);
> + if (!once) {
> + mce_unregister_decode_chain(&i7_mce_dec);
> + once = 1;
> + }
>
> /* Disable EDAC polling */
> i7core_pci_ctl_release(pvt);
> @@ -2253,6 +2257,7 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
> struct mem_ctl_info *mci;
> struct i7core_pvt *pvt;
> int rc, channels, csrows;
> + static int once;
>
> /* Check the number of active and not disabled channels */
> rc = i7core_get_active_channels(i7core_dev->socket, &channels, &csrows);
> @@ -2336,7 +2341,10 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
> /* DCLK for scrub rate setting */
> pvt->dclk_freq = get_dclk_freq();
>
> - mce_register_decode_chain(&i7_mce_dec);
> + if (!once) {
> + mce_register_decode_chain(&i7_mce_dec);
> + once = 1;
> + }
>
> return 0;
>
> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
> index a203536..20fabc8 100644
> --- a/drivers/edac/sb_edac.c
> +++ b/drivers/edac/sb_edac.c
> @@ -1655,6 +1655,7 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
> {
> struct mem_ctl_info *mci = sbridge_dev->mci;
> struct sbridge_pvt *pvt;
> + static int once;
>
> if (unlikely(!mci || !mci->pvt_info)) {
> debugf0("MC: " __FILE__ ": %s(): dev = %p\n",
> @@ -1669,7 +1670,10 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
> debugf0("MC: " __FILE__ ": %s(): mci = %p, dev = %p\n",
> __func__, mci, &sbridge_dev->pdev[0]->dev);
>
> - mce_unregister_decode_chain(&sbridge_mce_dec);
> + if (!once) {
> + mce_unregister_decode_chain(&sbridge_mce_dec);
> + once = 1;
> + }
>
> /* Remove MC sysfs nodes */
> edac_mc_del_mc(mci->dev);
> @@ -1685,6 +1689,7 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
> struct mem_ctl_info *mci;
> struct sbridge_pvt *pvt;
> int rc, channels, csrows;
> + static int once;
>
> /* Check the number of active and not disabled channels */
> rc = sbridge_get_active_channels(sbridge_dev->bus, &channels, &csrows);
> @@ -1738,7 +1743,10 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
> goto fail0;
> }
>
> - mce_register_decode_chain(&sbridge_mce_dec);
> + if (!once) {
> + mce_register_decode_chain(&sbridge_mce_dec);
> + once = 1;
> + }
> return 0;
>
> fail0:
IMO, function notifier_chain_register is very tricky. It implies once
the same nb is added,
new added nb->next and old one's existing in the chain are changed at
the same time. It is
very dangerous because the whole linked list are corrupted. If we still
keep current logic
to make it simple and beautiful, I hope we should add some comments on
the function at least.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] edac: avoid mce decoding crash after edac driver unloaded
2012-05-05 1:20 [PATCH] edac: avoid mce decoding crash after edac driver unloaded Chen Gong
2012-05-05 1:57 ` Chen Gong
@ 2012-05-05 10:05 ` Borislav Petkov
2012-05-07 7:03 ` Chen Gong
2 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2012-05-05 10:05 UTC (permalink / raw)
To: Chen Gong; +Cc: mchehab, linux-edac, linux-kernel, stable, Tony Luck
+ Tony.
On Sat, May 05, 2012 at 09:20:36AM +0800, Chen Gong wrote:
> Some edac drivers register themselves as mce decoders via
> notifier_chain. But in current notifier_chain implementation logic,
> it doesn't accept same notifier registered twice. If so, it will be
> wrong when removing the element from the list. For example, on one
> SandyBridge platform, remove module sb_edac and then trigger one
> error, it will hit oops because it has no mce decoder registered
> but related notifier_chain still points to an invalid callback
> function. Here is an example:
>
> Call Trace:
> [<ffffffff8150ef6a>] atomic_notifier_call_chain+0x1a/0x20
> [<ffffffff8102b936>] mce_log+0x46/0x180
> [<ffffffff8102eaea>] apei_mce_report_mem_error+0x4a/0x60
> [<ffffffff812e19d2>] ghes_do_proc+0x192/0x210
> [<ffffffff812e2066>] ghes_proc+0x46/0x70
> [<ffffffff812e20d8>] ghes_notify_sci+0x48/0x80
> [<ffffffff8150ef05>] notifier_call_chain+0x55/0x80
> [<ffffffff81076f1a>] __blocking_notifier_call_chain+0x5a/0x80
> [<ffffffff812aea11>] ? acpi_os_wait_events_complete+0x23/0x23
> [<ffffffff81076f56>] blocking_notifier_call_chain+0x16/0x20
> [<ffffffff812ddc4d>] acpi_hed_notify+0x19/0x1b
> [<ffffffff812b16bd>] acpi_device_notify+0x19/0x1b
> [<ffffffff812beb38>] acpi_ev_notify_dispatch+0x67/0x7f
> [<ffffffff812aea3a>] acpi_os_execute_deferred+0x29/0x36
> [<ffffffff81069dc2>] process_one_work+0x132/0x450
> [<ffffffff8106bbcb>] worker_thread+0x17b/0x3c0
> [<ffffffff8106ba50>] ? manage_workers+0x120/0x120
> [<ffffffff81070aee>] kthread+0x9e/0xb0
> [<ffffffff81514724>] kernel_thread_helper+0x4/0x10
> [<ffffffff81070a50>] ? kthread_freezable_should_stop+0x70/0x70
> [<ffffffff81514720>] ? gs_change+0x13/0x13
> Code: f3 49 89 d4 45 85 ed 4d 89 c6 48 8b 0f 74 48 48 85 c9 75 17 eb 41
> 0f 1f 80 00 00 00 00 41 83 ed 01 4c 89 f9 74 22 4d 85 ff 74 1d <4c> 8b
> 79 08 4c 89 e2 48 89 de 48 89 cf ff 11 4d 85 f6 74 04 41
> RIP [<ffffffff8150eef6>] notifier_call_chain+0x46/0x80
> RSP <ffff88042868fb20>
> CR2: ffffffffa01af838
> ---[ end trace 0100930068e73e6f ]---
> BUG: unable to handle kernel paging request at fffffffffffffff8
> IP: [<ffffffff810705b0>] kthread_data+0x10/0x20
> PGD 1a0d067 PUD 1a0e067 PMD 0
> Oops: 0000 [#2] SMP
>
> Only i7core_edac and sb_edac have such issues because they have more
> than one memory controller which means they have to register mce
> decoder many times.
Right, so the idea of the MCE notifier is to register on the chain
_once_ per whole driver, i.e. once per MCE error source. Having an EDAC
driver register memory controller every instance is simply not needed.
It could probably be done correctly by registering on the chain
at the end of i7core_init(), for the i7 edac driver for example,
after everything else has been successful. From the looks ot it,
i7core_mce_check_error() seems to select the proper node, i.e. memory
controller instance when invoked.
Thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] edac: avoid mce decoding crash after edac driver unloaded
2012-05-05 1:20 [PATCH] edac: avoid mce decoding crash after edac driver unloaded Chen Gong
2012-05-05 1:57 ` Chen Gong
2012-05-05 10:05 ` Borislav Petkov
@ 2012-05-07 7:03 ` Chen Gong
2012-05-07 10:42 ` Mauro Carvalho Chehab
2012-05-07 13:05 ` [PATCH V3] " Chen Gong
2 siblings, 2 replies; 10+ messages in thread
From: Chen Gong @ 2012-05-07 7:03 UTC (permalink / raw)
To: mchehab, bp; +Cc: tony.luck, linux-edac, linux-kernel, stable, Chen Gong
Some edac drivers register themselves as mce decoders via
notifier_chain. But in current notifier_chain implementation logic,
it doesn't accept same notifier registered twice. If so, it will be
wrong when adding/removing the element from the list. For example,
on one SandyBridge platform, remove module sb_edac and then trigger
one error, it will hit oops because it has no mce decoder registered
but related notifier_chain still points to an invalid callback
function. Here is an example:
Call Trace:
[<ffffffff8150ef6a>] atomic_notifier_call_chain+0x1a/0x20
[<ffffffff8102b936>] mce_log+0x46/0x180
[<ffffffff8102eaea>] apei_mce_report_mem_error+0x4a/0x60
[<ffffffff812e19d2>] ghes_do_proc+0x192/0x210
[<ffffffff812e2066>] ghes_proc+0x46/0x70
[<ffffffff812e20d8>] ghes_notify_sci+0x48/0x80
[<ffffffff8150ef05>] notifier_call_chain+0x55/0x80
[<ffffffff81076f1a>] __blocking_notifier_call_chain+0x5a/0x80
[<ffffffff812aea11>] ? acpi_os_wait_events_complete+0x23/0x23
[<ffffffff81076f56>] blocking_notifier_call_chain+0x16/0x20
[<ffffffff812ddc4d>] acpi_hed_notify+0x19/0x1b
[<ffffffff812b16bd>] acpi_device_notify+0x19/0x1b
[<ffffffff812beb38>] acpi_ev_notify_dispatch+0x67/0x7f
[<ffffffff812aea3a>] acpi_os_execute_deferred+0x29/0x36
[<ffffffff81069dc2>] process_one_work+0x132/0x450
[<ffffffff8106bbcb>] worker_thread+0x17b/0x3c0
[<ffffffff8106ba50>] ? manage_workers+0x120/0x120
[<ffffffff81070aee>] kthread+0x9e/0xb0
[<ffffffff81514724>] kernel_thread_helper+0x4/0x10
[<ffffffff81070a50>] ? kthread_freezable_should_stop+0x70/0x70
[<ffffffff81514720>] ? gs_change+0x13/0x13
Code: f3 49 89 d4 45 85 ed 4d 89 c6 48 8b 0f 74 48 48 85 c9 75 17 eb 41
0f 1f 80 00 00 00 00 41 83 ed 01 4c 89 f9 74 22 4d 85 ff 74 1d <4c> 8b
79 08 4c 89 e2 48 89 de 48 89 cf ff 11 4d 85 f6 74 04 41
RIP [<ffffffff8150eef6>] notifier_call_chain+0x46/0x80
RSP <ffff88042868fb20>
CR2: ffffffffa01af838
---[ end trace 0100930068e73e6f ]---
BUG: unable to handle kernel paging request at fffffffffffffff8
IP: [<ffffffff810705b0>] kthread_data+0x10/0x20
PGD 1a0d067 PUD 1a0e067 PMD 0
Oops: 0000 [#2] SMP
Only i7core_edac and sb_edac have such issues because they have more
than one memory controller which means they have to register mce
decoder many times.
v2->v1:
move register/unregister to the init/exit part
Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
---
drivers/edac/i7core_edac.c | 9 ++++-----
drivers/edac/sb_edac.c | 8 ++++----
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 85226cc..6fdf68c 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -2234,8 +2234,6 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
if (pvt->enable_scrub)
disable_sdram_scrub_setting(mci);
- mce_unregister_decode_chain(&i7_mce_dec);
-
/* Disable EDAC polling */
i7core_pci_ctl_release(pvt);
@@ -2336,8 +2334,6 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
/* DCLK for scrub rate setting */
pvt->dclk_freq = get_dclk_freq();
- mce_register_decode_chain(&i7_mce_dec);
-
return 0;
fail0:
@@ -2481,8 +2477,10 @@ static int __init i7core_init(void)
pci_rc = pci_register_driver(&i7core_driver);
- if (pci_rc >= 0)
+ if (pci_rc >= 0) {
+ mce_register_decode_chain(&i7_mce_dec);
return 0;
+ }
i7core_printk(KERN_ERR, "Failed to register device with error %d.\n",
pci_rc);
@@ -2498,6 +2496,7 @@ static void __exit i7core_exit(void)
{
debugf2("MC: " __FILE__ ": %s()\n", __func__);
pci_unregister_driver(&i7core_driver);
+ mce_unregister_decode_chain(&i7_mce_dec);
}
module_init(i7core_init);
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index a203536..e9858ba 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -1669,8 +1669,6 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
debugf0("MC: " __FILE__ ": %s(): mci = %p, dev = %p\n",
__func__, mci, &sbridge_dev->pdev[0]->dev);
- mce_unregister_decode_chain(&sbridge_mce_dec);
-
/* Remove MC sysfs nodes */
edac_mc_del_mc(mci->dev);
@@ -1738,7 +1736,6 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
goto fail0;
}
- mce_register_decode_chain(&sbridge_mce_dec);
return 0;
fail0:
@@ -1867,8 +1864,10 @@ static int __init sbridge_init(void)
pci_rc = pci_register_driver(&sbridge_driver);
- if (pci_rc >= 0)
+ if (pci_rc >= 0) {
+ mce_register_decode_chain(&sbridge_mce_dec);
return 0;
+ }
sbridge_printk(KERN_ERR, "Failed to register device with error %d.\n",
pci_rc);
@@ -1884,6 +1883,7 @@ static void __exit sbridge_exit(void)
{
debugf2("MC: " __FILE__ ": %s()\n", __func__);
pci_unregister_driver(&sbridge_driver);
+ mce_unregister_decode_chain(&sbridge_mce_dec);
}
module_init(sbridge_init);
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] edac: avoid mce decoding crash after edac driver unloaded
2012-05-07 7:03 ` Chen Gong
@ 2012-05-07 10:42 ` Mauro Carvalho Chehab
2012-05-07 13:05 ` [PATCH V3] " Chen Gong
1 sibling, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2012-05-07 10:42 UTC (permalink / raw)
To: Chen Gong; +Cc: bp, tony.luck, linux-edac, linux-kernel, stable
Em 07-05-2012 04:03, Chen Gong escreveu:
> Some edac drivers register themselves as mce decoders via
> notifier_chain. But in current notifier_chain implementation logic,
> it doesn't accept same notifier registered twice. If so, it will be
> wrong when adding/removing the element from the list. For example,
> on one SandyBridge platform, remove module sb_edac and then trigger
> one error, it will hit oops because it has no mce decoder registered
> but related notifier_chain still points to an invalid callback
> function. Here is an example:
>
> Call Trace:
> [<ffffffff8150ef6a>] atomic_notifier_call_chain+0x1a/0x20
> [<ffffffff8102b936>] mce_log+0x46/0x180
> [<ffffffff8102eaea>] apei_mce_report_mem_error+0x4a/0x60
> [<ffffffff812e19d2>] ghes_do_proc+0x192/0x210
> [<ffffffff812e2066>] ghes_proc+0x46/0x70
> [<ffffffff812e20d8>] ghes_notify_sci+0x48/0x80
> [<ffffffff8150ef05>] notifier_call_chain+0x55/0x80
> [<ffffffff81076f1a>] __blocking_notifier_call_chain+0x5a/0x80
> [<ffffffff812aea11>] ? acpi_os_wait_events_complete+0x23/0x23
> [<ffffffff81076f56>] blocking_notifier_call_chain+0x16/0x20
> [<ffffffff812ddc4d>] acpi_hed_notify+0x19/0x1b
> [<ffffffff812b16bd>] acpi_device_notify+0x19/0x1b
> [<ffffffff812beb38>] acpi_ev_notify_dispatch+0x67/0x7f
> [<ffffffff812aea3a>] acpi_os_execute_deferred+0x29/0x36
> [<ffffffff81069dc2>] process_one_work+0x132/0x450
> [<ffffffff8106bbcb>] worker_thread+0x17b/0x3c0
> [<ffffffff8106ba50>] ? manage_workers+0x120/0x120
> [<ffffffff81070aee>] kthread+0x9e/0xb0
> [<ffffffff81514724>] kernel_thread_helper+0x4/0x10
> [<ffffffff81070a50>] ? kthread_freezable_should_stop+0x70/0x70
> [<ffffffff81514720>] ? gs_change+0x13/0x13
> Code: f3 49 89 d4 45 85 ed 4d 89 c6 48 8b 0f 74 48 48 85 c9 75 17 eb 41
> 0f 1f 80 00 00 00 00 41 83 ed 01 4c 89 f9 74 22 4d 85 ff 74 1d <4c> 8b
> 79 08 4c 89 e2 48 89 de 48 89 cf ff 11 4d 85 f6 74 04 41
> RIP [<ffffffff8150eef6>] notifier_call_chain+0x46/0x80
> RSP <ffff88042868fb20>
> CR2: ffffffffa01af838
> ---[ end trace 0100930068e73e6f ]---
> BUG: unable to handle kernel paging request at fffffffffffffff8
> IP: [<ffffffff810705b0>] kthread_data+0x10/0x20
> PGD 1a0d067 PUD 1a0e067 PMD 0
> Oops: 0000 [#2] SMP
>
> Only i7core_edac and sb_edac have such issues because they have more
> than one memory controller which means they have to register mce
> decoder many times.
>
> v2->v1:
> move register/unregister to the init/exit part
>
> Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
> ---
> drivers/edac/i7core_edac.c | 9 ++++-----
> drivers/edac/sb_edac.c | 8 ++++----
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
> index 85226cc..6fdf68c 100644
> --- a/drivers/edac/i7core_edac.c
> +++ b/drivers/edac/i7core_edac.c
> @@ -2234,8 +2234,6 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
> if (pvt->enable_scrub)
> disable_sdram_scrub_setting(mci);
>
> - mce_unregister_decode_chain(&i7_mce_dec);
> -
> /* Disable EDAC polling */
> i7core_pci_ctl_release(pvt);
>
> @@ -2336,8 +2334,6 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
> /* DCLK for scrub rate setting */
> pvt->dclk_freq = get_dclk_freq();
>
> - mce_register_decode_chain(&i7_mce_dec);
> -
> return 0;
>
> fail0:
> @@ -2481,8 +2477,10 @@ static int __init i7core_init(void)
>
> pci_rc = pci_register_driver(&i7core_driver);
>
> - if (pci_rc >= 0)
> + if (pci_rc >= 0) {
> + mce_register_decode_chain(&i7_mce_dec);
> return 0;
> + }
>
> i7core_printk(KERN_ERR, "Failed to register device with error %d.\n",
> pci_rc);
It seems that handling the same error twice and causing OOPSes are not
the only things bad on changeset 4140c542. Now that i7core_mce_check_error()
uses get_i7core_dev(), this code is dead:
#ifdef CONFIG_SMP
/* Only handle if it is the right mc controller */
if (mce->socketid != pvt->i7core_dev->socket)
return NOTIFY_DONE;
#endif
As get_i7core_dev() logic warrants that this is always true.
Could you please remove this code on your patch?
> @@ -2498,6 +2496,7 @@ static void __exit i7core_exit(void)
> {
> debugf2("MC: " __FILE__ ": %s()\n", __func__);
> pci_unregister_driver(&i7core_driver);
> + mce_unregister_decode_chain(&i7_mce_dec);
> }
>
> module_init(i7core_init);
> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
> index a203536..e9858ba 100644
> --- a/drivers/edac/sb_edac.c
> +++ b/drivers/edac/sb_edac.c
> @@ -1669,8 +1669,6 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
> debugf0("MC: " __FILE__ ": %s(): mci = %p, dev = %p\n",
> __func__, mci, &sbridge_dev->pdev[0]->dev);
>
> - mce_unregister_decode_chain(&sbridge_mce_dec);
> -
> /* Remove MC sysfs nodes */
> edac_mc_del_mc(mci->dev);
>
> @@ -1738,7 +1736,6 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
> goto fail0;
> }
>
> - mce_register_decode_chain(&sbridge_mce_dec);
> return 0;
>
> fail0:
> @@ -1867,8 +1864,10 @@ static int __init sbridge_init(void)
>
> pci_rc = pci_register_driver(&sbridge_driver);
>
> - if (pci_rc >= 0)
> + if (pci_rc >= 0) {
> + mce_register_decode_chain(&sbridge_mce_dec);
> return 0;
> + }
>
> sbridge_printk(KERN_ERR, "Failed to register device with error %d.\n",
> pci_rc);
> @@ -1884,6 +1883,7 @@ static void __exit sbridge_exit(void)
> {
> debugf2("MC: " __FILE__ ": %s()\n", __func__);
> pci_unregister_driver(&sbridge_driver);
> + mce_unregister_decode_chain(&sbridge_mce_dec);
> }
>
> module_init(sbridge_init);
Thanks,
Mauro
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH V3] edac: avoid mce decoding crash after edac driver unloaded
2012-05-07 7:03 ` Chen Gong
2012-05-07 10:42 ` Mauro Carvalho Chehab
@ 2012-05-07 13:05 ` Chen Gong
2012-05-07 15:52 ` Greg KH
1 sibling, 1 reply; 10+ messages in thread
From: Chen Gong @ 2012-05-07 13:05 UTC (permalink / raw)
To: mchehab; +Cc: bp, tony.luck, linux-edac, linux-kernel, stable, Chen Gong
Some edac drivers register themselves as mce decoders via
notifier_chain. But in current notifier_chain implementation logic,
it doesn't accept same notifier registered twice. If so, it will be
wrong when adding/removing the element from the list. For example,
on one SandyBridge platform, remove module sb_edac and then trigger
one error, it will hit oops because it has no mce decoder registered
but related notifier_chain still points to an invalid callback
function. Here is an example:
Call Trace:
[<ffffffff8150ef6a>] atomic_notifier_call_chain+0x1a/0x20
[<ffffffff8102b936>] mce_log+0x46/0x180
[<ffffffff8102eaea>] apei_mce_report_mem_error+0x4a/0x60
[<ffffffff812e19d2>] ghes_do_proc+0x192/0x210
[<ffffffff812e2066>] ghes_proc+0x46/0x70
[<ffffffff812e20d8>] ghes_notify_sci+0x48/0x80
[<ffffffff8150ef05>] notifier_call_chain+0x55/0x80
[<ffffffff81076f1a>] __blocking_notifier_call_chain+0x5a/0x80
[<ffffffff812aea11>] ? acpi_os_wait_events_complete+0x23/0x23
[<ffffffff81076f56>] blocking_notifier_call_chain+0x16/0x20
[<ffffffff812ddc4d>] acpi_hed_notify+0x19/0x1b
[<ffffffff812b16bd>] acpi_device_notify+0x19/0x1b
[<ffffffff812beb38>] acpi_ev_notify_dispatch+0x67/0x7f
[<ffffffff812aea3a>] acpi_os_execute_deferred+0x29/0x36
[<ffffffff81069dc2>] process_one_work+0x132/0x450
[<ffffffff8106bbcb>] worker_thread+0x17b/0x3c0
[<ffffffff8106ba50>] ? manage_workers+0x120/0x120
[<ffffffff81070aee>] kthread+0x9e/0xb0
[<ffffffff81514724>] kernel_thread_helper+0x4/0x10
[<ffffffff81070a50>] ? kthread_freezable_should_stop+0x70/0x70
[<ffffffff81514720>] ? gs_change+0x13/0x13
Code: f3 49 89 d4 45 85 ed 4d 89 c6 48 8b 0f 74 48 48 85 c9 75 17 eb 41
0f 1f 80 00 00 00 00 41 83 ed 01 4c 89 f9 74 22 4d 85 ff 74 1d <4c> 8b
79 08 4c 89 e2 48 89 de 48 89 cf ff 11 4d 85 f6 74 04 41
RIP [<ffffffff8150eef6>] notifier_call_chain+0x46/0x80
RSP <ffff88042868fb20>
CR2: ffffffffa01af838
---[ end trace 0100930068e73e6f ]---
BUG: unable to handle kernel paging request at fffffffffffffff8
IP: [<ffffffff810705b0>] kthread_data+0x10/0x20
PGD 1a0d067 PUD 1a0e067 PMD 0
Oops: 0000 [#2] SMP
Only i7core_edac and sb_edac have such issues because they have more
than one memory controller which means they have to register mce
decoder many times.
v3->v2:
remove dead code in i7core_edac, suggested by Mauro
v2->v1:
move register/unregister to the init/exit part
Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
---
drivers/edac/i7core_edac.c | 15 ++++-----------
drivers/edac/sb_edac.c | 8 ++++----
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 85226cc..0fe2277 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -1932,12 +1932,6 @@ static int i7core_mce_check_error(struct notifier_block *nb, unsigned long val,
if (mce->bank != 8)
return NOTIFY_DONE;
-#ifdef CONFIG_SMP
- /* Only handle if it is the right mc controller */
- if (mce->socketid != pvt->i7core_dev->socket)
- return NOTIFY_DONE;
-#endif
-
smp_rmb();
if ((pvt->mce_out + 1) % MCE_LOG_LEN == pvt->mce_in) {
smp_wmb();
@@ -2234,8 +2228,6 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
if (pvt->enable_scrub)
disable_sdram_scrub_setting(mci);
- mce_unregister_decode_chain(&i7_mce_dec);
-
/* Disable EDAC polling */
i7core_pci_ctl_release(pvt);
@@ -2336,8 +2328,6 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
/* DCLK for scrub rate setting */
pvt->dclk_freq = get_dclk_freq();
- mce_register_decode_chain(&i7_mce_dec);
-
return 0;
fail0:
@@ -2481,8 +2471,10 @@ static int __init i7core_init(void)
pci_rc = pci_register_driver(&i7core_driver);
- if (pci_rc >= 0)
+ if (pci_rc >= 0) {
+ mce_register_decode_chain(&i7_mce_dec);
return 0;
+ }
i7core_printk(KERN_ERR, "Failed to register device with error %d.\n",
pci_rc);
@@ -2498,6 +2490,7 @@ static void __exit i7core_exit(void)
{
debugf2("MC: " __FILE__ ": %s()\n", __func__);
pci_unregister_driver(&i7core_driver);
+ mce_unregister_decode_chain(&i7_mce_dec);
}
module_init(i7core_init);
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index a203536..e9858ba 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -1669,8 +1669,6 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
debugf0("MC: " __FILE__ ": %s(): mci = %p, dev = %p\n",
__func__, mci, &sbridge_dev->pdev[0]->dev);
- mce_unregister_decode_chain(&sbridge_mce_dec);
-
/* Remove MC sysfs nodes */
edac_mc_del_mc(mci->dev);
@@ -1738,7 +1736,6 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
goto fail0;
}
- mce_register_decode_chain(&sbridge_mce_dec);
return 0;
fail0:
@@ -1867,8 +1864,10 @@ static int __init sbridge_init(void)
pci_rc = pci_register_driver(&sbridge_driver);
- if (pci_rc >= 0)
+ if (pci_rc >= 0) {
+ mce_register_decode_chain(&sbridge_mce_dec);
return 0;
+ }
sbridge_printk(KERN_ERR, "Failed to register device with error %d.\n",
pci_rc);
@@ -1884,6 +1883,7 @@ static void __exit sbridge_exit(void)
{
debugf2("MC: " __FILE__ ": %s()\n", __func__);
pci_unregister_driver(&sbridge_driver);
+ mce_unregister_decode_chain(&sbridge_mce_dec);
}
module_init(sbridge_init);
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH V3] edac: avoid mce decoding crash after edac driver unloaded
2012-05-07 13:05 ` [PATCH V3] " Chen Gong
@ 2012-05-07 15:52 ` Greg KH
2012-05-08 6:59 ` Chen Gong
0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2012-05-07 15:52 UTC (permalink / raw)
To: Chen Gong; +Cc: mchehab, bp, tony.luck, linux-edac, linux-kernel, stable
On Mon, May 07, 2012 at 09:05:50PM +0800, Chen Gong wrote:
> Some edac drivers register themselves as mce decoders via
> notifier_chain. But in current notifier_chain implementation logic,
> it doesn't accept same notifier registered twice. If so, it will be
> wrong when adding/removing the element from the list. For example,
> on one SandyBridge platform, remove module sb_edac and then trigger
> one error, it will hit oops because it has no mce decoder registered
> but related notifier_chain still points to an invalid callback
> function. Here is an example:
>
> Call Trace:
> [<ffffffff8150ef6a>] atomic_notifier_call_chain+0x1a/0x20
> [<ffffffff8102b936>] mce_log+0x46/0x180
> [<ffffffff8102eaea>] apei_mce_report_mem_error+0x4a/0x60
> [<ffffffff812e19d2>] ghes_do_proc+0x192/0x210
> [<ffffffff812e2066>] ghes_proc+0x46/0x70
> [<ffffffff812e20d8>] ghes_notify_sci+0x48/0x80
> [<ffffffff8150ef05>] notifier_call_chain+0x55/0x80
> [<ffffffff81076f1a>] __blocking_notifier_call_chain+0x5a/0x80
> [<ffffffff812aea11>] ? acpi_os_wait_events_complete+0x23/0x23
> [<ffffffff81076f56>] blocking_notifier_call_chain+0x16/0x20
> [<ffffffff812ddc4d>] acpi_hed_notify+0x19/0x1b
> [<ffffffff812b16bd>] acpi_device_notify+0x19/0x1b
> [<ffffffff812beb38>] acpi_ev_notify_dispatch+0x67/0x7f
> [<ffffffff812aea3a>] acpi_os_execute_deferred+0x29/0x36
> [<ffffffff81069dc2>] process_one_work+0x132/0x450
> [<ffffffff8106bbcb>] worker_thread+0x17b/0x3c0
> [<ffffffff8106ba50>] ? manage_workers+0x120/0x120
> [<ffffffff81070aee>] kthread+0x9e/0xb0
> [<ffffffff81514724>] kernel_thread_helper+0x4/0x10
> [<ffffffff81070a50>] ? kthread_freezable_should_stop+0x70/0x70
> [<ffffffff81514720>] ? gs_change+0x13/0x13
> Code: f3 49 89 d4 45 85 ed 4d 89 c6 48 8b 0f 74 48 48 85 c9 75 17 eb 41
> 0f 1f 80 00 00 00 00 41 83 ed 01 4c 89 f9 74 22 4d 85 ff 74 1d <4c> 8b
> 79 08 4c 89 e2 48 89 de 48 89 cf ff 11 4d 85 f6 74 04 41
> RIP [<ffffffff8150eef6>] notifier_call_chain+0x46/0x80
> RSP <ffff88042868fb20>
> CR2: ffffffffa01af838
> ---[ end trace 0100930068e73e6f ]---
> BUG: unable to handle kernel paging request at fffffffffffffff8
> IP: [<ffffffff810705b0>] kthread_data+0x10/0x20
> PGD 1a0d067 PUD 1a0e067 PMD 0
> Oops: 0000 [#2] SMP
>
> Only i7core_edac and sb_edac have such issues because they have more
> than one memory controller which means they have to register mce
> decoder many times.
>
> v3->v2:
> remove dead code in i7core_edac, suggested by Mauro
>
> v2->v1:
> move register/unregister to the init/exit part
>
> Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
> ---
> drivers/edac/i7core_edac.c | 15 ++++-----------
> drivers/edac/sb_edac.c | 8 ++++----
> 2 files changed, 8 insertions(+), 15 deletions(-)
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.
</formletter>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3] edac: avoid mce decoding crash after edac driver unloaded
2012-05-07 15:52 ` Greg KH
@ 2012-05-08 6:59 ` Chen Gong
2012-05-08 13:24 ` Greg KH
2012-05-08 23:40 ` [PATCH RESEND " Chen Gong
0 siblings, 2 replies; 10+ messages in thread
From: Chen Gong @ 2012-05-08 6:59 UTC (permalink / raw)
To: Greg KH; +Cc: mchehab, bp, tony.luck, linux-edac, linux-kernel, stable
...
>
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree. Please read
> Documentation/stable_kernel_rules.txt for how to do this properly.
>
> </formletter> -- To unsubscribe from this list: send the line
> "unsubscribe linux-edac" in the body of a message to
> majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
>
Hi, Greg
Do you mean I need to add some information in my patch as below:
Cc: <stable@vger.kernel.org> # 3.3.y:
Cc: <stable@vger.kernel.org> # 3.2.y: 3653ada: x86, mce: Add wrappers
Cc: <stable@vger.kernel.org> # 3.2.y:
For 3.3.y, commit 3653ada has been there so it is not necessary. Is it
OK to send one patch to two -stable trees?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V3] edac: avoid mce decoding crash after edac driver unloaded
2012-05-08 6:59 ` Chen Gong
@ 2012-05-08 13:24 ` Greg KH
2012-05-08 23:40 ` [PATCH RESEND " Chen Gong
1 sibling, 0 replies; 10+ messages in thread
From: Greg KH @ 2012-05-08 13:24 UTC (permalink / raw)
To: Chen Gong; +Cc: mchehab, bp, tony.luck, linux-edac, linux-kernel, stable
On Tue, May 08, 2012 at 02:59:22PM +0800, Chen Gong wrote:
> ...
> >
> > <formletter>
> >
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree. Please read
> > Documentation/stable_kernel_rules.txt for how to do this properly.
> >
> > </formletter> -- To unsubscribe from this list: send the line
> > "unsubscribe linux-edac" in the body of a message to
> > majordomo@vger.kernel.org More majordomo info at
> > http://vger.kernel.org/majordomo-info.html
> >
>
> Hi, Greg
>
> Do you mean I need to add some information in my patch as below:
>
> Cc: <stable@vger.kernel.org> # 3.3.y:
> Cc: <stable@vger.kernel.org> # 3.2.y: 3653ada: x86, mce: Add wrappers
> Cc: <stable@vger.kernel.org> # 3.2.y:
>
>
> For 3.3.y, commit 3653ada has been there so it is not necessary. Is it
> OK to send one patch to two -stable trees?
Yes, if you just add the line once, it will be applied to all
appliciable stable trees when it is accepted into Linus's tree.
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH RESEND V3] edac: avoid mce decoding crash after edac driver unloaded
2012-05-08 6:59 ` Chen Gong
2012-05-08 13:24 ` Greg KH
@ 2012-05-08 23:40 ` Chen Gong
1 sibling, 0 replies; 10+ messages in thread
From: Chen Gong @ 2012-05-08 23:40 UTC (permalink / raw)
To: mchehab, greg; +Cc: bp, tony.luck, linux-edac, linux-kernel, stable, Chen Gong
Some edac drivers register themselves as mce decoders via
notifier_chain. But in current notifier_chain implementation logic,
it doesn't accept same notifier registered twice. If so, it will be
wrong when adding/removing the element from the list. For example,
on one SandyBridge platform, remove module sb_edac and then trigger
one error, it will hit oops because it has no mce decoder registered
but related notifier_chain still points to an invalid callback
function. Here is an example:
Call Trace:
[<ffffffff8150ef6a>] atomic_notifier_call_chain+0x1a/0x20
[<ffffffff8102b936>] mce_log+0x46/0x180
[<ffffffff8102eaea>] apei_mce_report_mem_error+0x4a/0x60
[<ffffffff812e19d2>] ghes_do_proc+0x192/0x210
[<ffffffff812e2066>] ghes_proc+0x46/0x70
[<ffffffff812e20d8>] ghes_notify_sci+0x48/0x80
[<ffffffff8150ef05>] notifier_call_chain+0x55/0x80
[<ffffffff81076f1a>] __blocking_notifier_call_chain+0x5a/0x80
[<ffffffff812aea11>] ? acpi_os_wait_events_complete+0x23/0x23
[<ffffffff81076f56>] blocking_notifier_call_chain+0x16/0x20
[<ffffffff812ddc4d>] acpi_hed_notify+0x19/0x1b
[<ffffffff812b16bd>] acpi_device_notify+0x19/0x1b
[<ffffffff812beb38>] acpi_ev_notify_dispatch+0x67/0x7f
[<ffffffff812aea3a>] acpi_os_execute_deferred+0x29/0x36
[<ffffffff81069dc2>] process_one_work+0x132/0x450
[<ffffffff8106bbcb>] worker_thread+0x17b/0x3c0
[<ffffffff8106ba50>] ? manage_workers+0x120/0x120
[<ffffffff81070aee>] kthread+0x9e/0xb0
[<ffffffff81514724>] kernel_thread_helper+0x4/0x10
[<ffffffff81070a50>] ? kthread_freezable_should_stop+0x70/0x70
[<ffffffff81514720>] ? gs_change+0x13/0x13
Code: f3 49 89 d4 45 85 ed 4d 89 c6 48 8b 0f 74 48 48 85 c9 75 17 eb 41
0f 1f 80 00 00 00 00 41 83 ed 01 4c 89 f9 74 22 4d 85 ff 74 1d <4c> 8b
79 08 4c 89 e2 48 89 de 48 89 cf ff 11 4d 85 f6 74 04 41
RIP [<ffffffff8150eef6>] notifier_call_chain+0x46/0x80
RSP <ffff88042868fb20>
CR2: ffffffffa01af838
---[ end trace 0100930068e73e6f ]---
BUG: unable to handle kernel paging request at fffffffffffffff8
IP: [<ffffffff810705b0>] kthread_data+0x10/0x20
PGD 1a0d067 PUD 1a0e067 PMD 0
Oops: 0000 [#2] SMP
Only i7core_edac and sb_edac have such issues because they have more
than one memory controller which means they have to register mce
decoder many times.
v3->v2:
remove dead code in i7core_edac, suggested by Mauro
v2->v1:
move register/unregister to the init/exit part
Cc: <stable@vger.kernel.org> # 3.3.y:
Cc: <stable@vger.kernel.org> # 3.2.y: 3653ada: x86, mce: Add wrappers
Cc: <stable@vger.kernel.org> # 3.2.y:
Signed-off-by: Chen Gong <gong.chen@linux.intel.com>
---
drivers/edac/i7core_edac.c | 15 ++++-----------
drivers/edac/sb_edac.c | 8 ++++----
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 85226cc..0fe2277 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -1932,12 +1932,6 @@ static int i7core_mce_check_error(struct notifier_block *nb, unsigned long val,
if (mce->bank != 8)
return NOTIFY_DONE;
-#ifdef CONFIG_SMP
- /* Only handle if it is the right mc controller */
- if (mce->socketid != pvt->i7core_dev->socket)
- return NOTIFY_DONE;
-#endif
-
smp_rmb();
if ((pvt->mce_out + 1) % MCE_LOG_LEN == pvt->mce_in) {
smp_wmb();
@@ -2234,8 +2228,6 @@ static void i7core_unregister_mci(struct i7core_dev *i7core_dev)
if (pvt->enable_scrub)
disable_sdram_scrub_setting(mci);
- mce_unregister_decode_chain(&i7_mce_dec);
-
/* Disable EDAC polling */
i7core_pci_ctl_release(pvt);
@@ -2336,8 +2328,6 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev)
/* DCLK for scrub rate setting */
pvt->dclk_freq = get_dclk_freq();
- mce_register_decode_chain(&i7_mce_dec);
-
return 0;
fail0:
@@ -2481,8 +2471,10 @@ static int __init i7core_init(void)
pci_rc = pci_register_driver(&i7core_driver);
- if (pci_rc >= 0)
+ if (pci_rc >= 0) {
+ mce_register_decode_chain(&i7_mce_dec);
return 0;
+ }
i7core_printk(KERN_ERR, "Failed to register device with error %d.\n",
pci_rc);
@@ -2498,6 +2490,7 @@ static void __exit i7core_exit(void)
{
debugf2("MC: " __FILE__ ": %s()\n", __func__);
pci_unregister_driver(&i7core_driver);
+ mce_unregister_decode_chain(&i7_mce_dec);
}
module_init(i7core_init);
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index a203536..e9858ba 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -1669,8 +1669,6 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
debugf0("MC: " __FILE__ ": %s(): mci = %p, dev = %p\n",
__func__, mci, &sbridge_dev->pdev[0]->dev);
- mce_unregister_decode_chain(&sbridge_mce_dec);
-
/* Remove MC sysfs nodes */
edac_mc_del_mc(mci->dev);
@@ -1738,7 +1736,6 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev)
goto fail0;
}
- mce_register_decode_chain(&sbridge_mce_dec);
return 0;
fail0:
@@ -1867,8 +1864,10 @@ static int __init sbridge_init(void)
pci_rc = pci_register_driver(&sbridge_driver);
- if (pci_rc >= 0)
+ if (pci_rc >= 0) {
+ mce_register_decode_chain(&sbridge_mce_dec);
return 0;
+ }
sbridge_printk(KERN_ERR, "Failed to register device with error %d.\n",
pci_rc);
@@ -1884,6 +1883,7 @@ static void __exit sbridge_exit(void)
{
debugf2("MC: " __FILE__ ": %s()\n", __func__);
pci_unregister_driver(&sbridge_driver);
+ mce_unregister_decode_chain(&sbridge_mce_dec);
}
module_init(sbridge_init);
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-05-08 23:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-05 1:20 [PATCH] edac: avoid mce decoding crash after edac driver unloaded Chen Gong
2012-05-05 1:57 ` Chen Gong
2012-05-05 10:05 ` Borislav Petkov
2012-05-07 7:03 ` Chen Gong
2012-05-07 10:42 ` Mauro Carvalho Chehab
2012-05-07 13:05 ` [PATCH V3] " Chen Gong
2012-05-07 15:52 ` Greg KH
2012-05-08 6:59 ` Chen Gong
2012-05-08 13:24 ` Greg KH
2012-05-08 23:40 ` [PATCH RESEND " Chen Gong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).