diff options
author | Jarkko Lavinen <jlavi@iki.fi> | 2008-12-01 16:14:08 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2008-12-01 22:55:25 -0500 |
commit | 09a81269c7aadaec3375a7ebd9647acbb72f5a67 (patch) | |
tree | bb823a19c56cc4a9d6110485ff05acb24f2ee578 | |
parent | 307d114441f905e4576871ff28d06408a1af1a7e (diff) |
i82875p_edac: fix module remove
Fix module removal bugs of i82875p_edac. Also i82975x_edac code seems to
have the same module removal bugs as in i82875p_edac.
The problems were:
1. In module removal i82875p_remove_one() is never called.
Variable i82875p_registered is newer changed from 1, which
guarantees i82875p_remove_one() is not called (and even if it were
called, it would be called in wrong order).
As a result, the edac_mc workque is not stopped and keeps probing.
If kernel debugging options are not enabled, user may not notice
anything going wrong.
if debugging options are enabled and I do "rmmod i82875p_edac", I
get:
edac debug: edac_pci_workq_function() checking
BUG: unable to handle kernel paging request at f882d16f
...
call trace:
[<f8834df3>] ? edac_mc_workq_function+0x55/0x7e [edac_core]
[<c0233974>] ? run_workqueue+0xd7/0x1a5
[<c023392f>] ? run_workqueue+0x92/0x1a5
[<f8834d9e>] ? edac_mc_workq_function+0x0/0x7e [edac_core]
[<c0233af9>] ? worker_thread+0xb7/0xc3
[<c0236a7b>] ? autoremove_wake_function+0x0/0x33
[<c0233a42>] ? worker_thread+0x0/0xc3
[<c0236809>] ? kthread+0x3b/0x61
[<c02367ce>] ? kthread+0x0/0x61
[<c0204587>] ? kernel_thread_helper+0x7/0x10
Fix for this is to get rid of needles variable i82875p_registered
altogether and run i82875p_remove_one() *before*
pci_unregister_driver().
2. edac_mc_del_mc() uses mci after freeing mci
edac_mc_del_mc() calls calls edac_remove_sysfs_mci_device(). The
kobject refcount of mci drops to 0 and mci is freed. After this
mci is accessed via debug print and i82875p_remove_one() still
uses mci->pvt and tries to free mci again with edac_mc_free().
The fix for this is add kobject_get(&mci->edac_mci_kobj) after
edac_mc_alloc(). Then the mci is still available after returning
from edac_mc_del_mc() with refcount 1, and mci->pvt is still
available. When i82875p_remove_one() finally calls edac_mc_free(),
this will cause kobject_put() and mci is released properly.
Signed-off-by: Jarkko Lavinen <jlavi@iki.fi>
Cc: Doug Thompson <norsk5@yahoo.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | drivers/edac/i82875p_edac.c | 13 |
1 files changed, 7 insertions, 6 deletions
diff --git a/drivers/edac/i82875p_edac.c b/drivers/edac/i82875p_edac.c index 7f909972fecd..ebb037b78758 100644 --- a/drivers/edac/i82875p_edac.c +++ b/drivers/edac/i82875p_edac.c | |||
@@ -182,8 +182,6 @@ static struct pci_dev *mci_pdev; /* init dev: in case that AGP code has | |||
182 | * already registered driver | 182 | * already registered driver |
183 | */ | 183 | */ |
184 | 184 | ||
185 | static int i82875p_registered = 1; | ||
186 | |||
187 | static struct edac_pci_ctl_info *i82875p_pci; | 185 | static struct edac_pci_ctl_info *i82875p_pci; |
188 | 186 | ||
189 | static void i82875p_get_error_info(struct mem_ctl_info *mci, | 187 | static void i82875p_get_error_info(struct mem_ctl_info *mci, |
@@ -410,6 +408,9 @@ static int i82875p_probe1(struct pci_dev *pdev, int dev_idx) | |||
410 | goto fail0; | 408 | goto fail0; |
411 | } | 409 | } |
412 | 410 | ||
411 | /* Keeps mci available after edac_mc_del_mc() till edac_mc_free() */ | ||
412 | kobject_get(&mci->edac_mci_kobj); | ||
413 | |||
413 | debugf3("%s(): init mci\n", __func__); | 414 | debugf3("%s(): init mci\n", __func__); |
414 | mci->dev = &pdev->dev; | 415 | mci->dev = &pdev->dev; |
415 | mci->mtype_cap = MEM_FLAG_DDR; | 416 | mci->mtype_cap = MEM_FLAG_DDR; |
@@ -452,6 +453,7 @@ static int i82875p_probe1(struct pci_dev *pdev, int dev_idx) | |||
452 | return 0; | 453 | return 0; |
453 | 454 | ||
454 | fail1: | 455 | fail1: |
456 | kobject_put(&mci->edac_mci_kobj); | ||
455 | edac_mc_free(mci); | 457 | edac_mc_free(mci); |
456 | 458 | ||
457 | fail0: | 459 | fail0: |
@@ -579,12 +581,11 @@ static void __exit i82875p_exit(void) | |||
579 | { | 581 | { |
580 | debugf3("%s()\n", __func__); | 582 | debugf3("%s()\n", __func__); |
581 | 583 | ||
584 | i82875p_remove_one(mci_pdev); | ||
585 | pci_dev_put(mci_pdev); | ||
586 | |||
582 | pci_unregister_driver(&i82875p_driver); | 587 | pci_unregister_driver(&i82875p_driver); |
583 | 588 | ||
584 | if (!i82875p_registered) { | ||
585 | i82875p_remove_one(mci_pdev); | ||
586 | pci_dev_put(mci_pdev); | ||
587 | } | ||
588 | } | 589 | } |
589 | 590 | ||
590 | module_init(i82875p_init); | 591 | module_init(i82875p_init); |