[PATCH] EDAC: protect memory controller list
- Fix code so we always hold mem_ctls_mutex while we are stepping
through the list of mem_ctl_info structures. Otherwise bad things
may happen if one task is stepping through the list while another
task is modifying it. We may eventually want to use reference
counting to manage the mem_ctl_info structures. In the meantime we
may as well fix this bug.
- Don't disable interrupts while we are walking the list of
mem_ctl_info structures in check_mc_devices(). This is unnecessary.
Signed-off-by: David S. Peterson <dsp@llnl.gov>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
diff --git a/drivers/edac/amd76x_edac.c b/drivers/edac/amd76x_edac.c
index 821c252..87bd8b4 100644
--- a/drivers/edac/amd76x_edac.c
+++ b/drivers/edac/amd76x_edac.c
@@ -314,10 +314,9 @@
debugf0("%s()\n", __func__);
- if ((mci = edac_mc_find_mci_by_pdev(pdev)) == NULL)
+ if ((mci = edac_mc_del_mc(pdev)) == NULL)
return;
- if (edac_mc_del_mc(mci))
- return;
+
edac_mc_free(mci);
}
diff --git a/drivers/edac/e752x_edac.c b/drivers/edac/e752x_edac.c
index 2444654..c86db23 100644
--- a/drivers/edac/e752x_edac.c
+++ b/drivers/edac/e752x_edac.c
@@ -976,10 +976,7 @@
debugf0("%s()\n", __func__);
- if ((mci = edac_mc_find_mci_by_pdev(pdev)) == NULL)
- return;
-
- if (edac_mc_del_mc(mci))
+ if ((mci = edac_mc_del_mc(pdev)) == NULL)
return;
pvt = (struct e752x_pvt *) mci->pvt_info;
diff --git a/drivers/edac/e7xxx_edac.c b/drivers/edac/e7xxx_edac.c
index 8b0da35..9b59c66 100644
--- a/drivers/edac/e7xxx_edac.c
+++ b/drivers/edac/e7xxx_edac.c
@@ -510,12 +510,12 @@
debugf0("%s()\n", __func__);
- if (((mci = edac_mc_find_mci_by_pdev(pdev)) != 0) &&
- !edac_mc_del_mc(mci)) {
- pvt = (struct e7xxx_pvt *) mci->pvt_info;
- pci_dev_put(pvt->bridge_ck);
- edac_mc_free(mci);
- }
+ if ((mci = edac_mc_del_mc(pdev)) == NULL)
+ return;
+
+ pvt = (struct e7xxx_pvt *) mci->pvt_info;
+ pci_dev_put(pvt->bridge_ck);
+ edac_mc_free(mci);
}
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 7ee9419..ad812a4 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -1370,11 +1370,7 @@
kfree(mci);
}
-
-
-EXPORT_SYMBOL(edac_mc_find_mci_by_pdev);
-
-struct mem_ctl_info *edac_mc_find_mci_by_pdev(struct pci_dev *pdev)
+static struct mem_ctl_info *find_mci_by_pdev(struct pci_dev *pdev)
{
struct mem_ctl_info *mci;
struct list_head *item;
@@ -1401,7 +1397,7 @@
mci->mc_idx = 0;
insert_before = &mc_devices;
} else {
- if (edac_mc_find_mci_by_pdev(mci->pdev)) {
+ if (find_mci_by_pdev(mci->pdev)) {
edac_printk(KERN_WARNING, EDAC_MC,
"%s (%s) %s %s already assigned %d\n",
mci->pdev->dev.bus_id,
@@ -1520,27 +1516,29 @@
/**
* edac_mc_del_mc: Remove sysfs entries for specified mci structure and
* remove mci structure from global list
- * @mci: Pointer to struct mem_ctl_info structure
+ * @pdev: Pointer to 'struct pci_dev' representing mci structure to remove.
*
- * Returns:
- * 0 Success
- * 1 Failure
+ * Return pointer to removed mci structure, or NULL if device not found.
*/
-int edac_mc_del_mc(struct mem_ctl_info *mci)
+struct mem_ctl_info * edac_mc_del_mc(struct pci_dev *pdev)
{
- int rc = 1;
+ struct mem_ctl_info *mci;
- debugf0("MC%d: %s()\n", mci->mc_idx, __func__);
- edac_remove_sysfs_mci_device(mci);
+ debugf0("MC: %s()\n", __func__);
down(&mem_ctls_mutex);
+
+ if ((mci = find_mci_by_pdev(pdev)) == NULL) {
+ up(&mem_ctls_mutex);
+ return NULL;
+ }
+
+ edac_remove_sysfs_mci_device(mci);
del_mc_from_global_list(mci);
+ up(&mem_ctls_mutex);
edac_printk(KERN_INFO, EDAC_MC,
"Removed device %d for %s %s: PCI %s\n", mci->mc_idx,
mci->mod_name, mci->ctl_name, pci_name(mci->pdev));
- rc = 0;
- up(&mem_ctls_mutex);
-
- return rc;
+ return mci;
}
@@ -2018,14 +2016,12 @@
*/
static inline void check_mc_devices (void)
{
- unsigned long flags;
struct list_head *item;
struct mem_ctl_info *mci;
debugf3("%s()\n", __func__);
- /* during poll, have interrupts off */
- local_irq_save(flags);
+ down(&mem_ctls_mutex);
list_for_each(item, &mc_devices) {
mci = list_entry(item, struct mem_ctl_info, link);
@@ -2034,7 +2030,7 @@
mci->edac_check(mci);
}
- local_irq_restore(flags);
+ up(&mem_ctls_mutex);
}
diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
index c9c1590..2cec157 100644
--- a/drivers/edac/edac_mc.h
+++ b/drivers/edac/edac_mc.h
@@ -410,14 +410,11 @@
#endif /* CONFIG_EDAC_DEBUG */
extern int edac_mc_add_mc(struct mem_ctl_info *mci);
-extern int edac_mc_del_mc(struct mem_ctl_info *mci);
+extern struct mem_ctl_info * edac_mc_del_mc(struct pci_dev *pdev);
extern int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci,
unsigned long page);
-extern struct mem_ctl_info *edac_mc_find_mci_by_pdev(struct pci_dev
- *pdev);
-
extern void edac_mc_scrub_block(unsigned long page,
unsigned long offset, u32 size);
diff --git a/drivers/edac/i82860_edac.c b/drivers/edac/i82860_edac.c
index 942129d..6888542 100644
--- a/drivers/edac/i82860_edac.c
+++ b/drivers/edac/i82860_edac.c
@@ -237,9 +237,10 @@
debugf0("%s()\n", __func__);
- mci = edac_mc_find_mci_by_pdev(pdev);
- if ((mci != NULL) && (edac_mc_del_mc(mci) == 0))
- edac_mc_free(mci);
+ if ((mci = edac_mc_del_mc(pdev)) == NULL)
+ return;
+
+ edac_mc_free(mci);
}
static const struct pci_device_id i82860_pci_tbl[] __devinitdata = {
diff --git a/drivers/edac/i82875p_edac.c b/drivers/edac/i82875p_edac.c
index 40ba2be..aad1900 100644
--- a/drivers/edac/i82875p_edac.c
+++ b/drivers/edac/i82875p_edac.c
@@ -452,7 +452,7 @@
debugf0("%s()\n", __func__);
- if ((mci = edac_mc_find_mci_by_pdev(pdev)) == NULL)
+ if ((mci = edac_mc_del_mc(pdev)) == NULL)
return;
pvt = (struct i82875p_pvt *) mci->pvt_info;
@@ -467,9 +467,6 @@
pci_dev_put(pvt->ovrfl_pdev);
}
- if (edac_mc_del_mc(mci))
- return;
-
edac_mc_free(mci);
}
diff --git a/drivers/edac/r82600_edac.c b/drivers/edac/r82600_edac.c
index 787a765..5966916 100644
--- a/drivers/edac/r82600_edac.c
+++ b/drivers/edac/r82600_edac.c
@@ -353,9 +353,10 @@
debugf0("%s()\n", __func__);
- if (((mci = edac_mc_find_mci_by_pdev(pdev)) != NULL) &&
- !edac_mc_del_mc(mci))
- edac_mc_free(mci);
+ if ((mci = edac_mc_del_mc(pdev)) == NULL)
+ return;
+
+ edac_mc_free(mci);
}
