From 1288c18f48d9bf373dbed6b688cde36dc970b1ed Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Tue, 10 Aug 2010 18:57:01 -0300 Subject: i7core_edac: Properly mark const static vars as such There are two groups of sysfs attributes: one for rdimm and another for udimm. Instead of changing dynamically the unique static struct for handling udimm's, declare two vars and make them constant. This avoids the risk of having two or more memory controllers, each needing a different set of attributes. While here, use const on all places where it is applicable. Signed-off-by: Mauro Carvalho Chehab edac_core: use const for constant sysfs arguments Signed-off-by: Mauro Carvalho Chehab --- drivers/edac/edac_mc_sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/edac/edac_mc_sysfs.c') diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index 8aad94d10c0c..aacffe5d0f33 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -783,7 +783,7 @@ static struct kobj_type ktype_inst_grp = { * object tree. */ static int edac_create_mci_instance_attributes(struct mem_ctl_info *mci, - struct mcidev_sysfs_attribute *sysfs_attrib, + const struct mcidev_sysfs_attribute *sysfs_attrib, struct kobject *kobj) { int err; @@ -842,7 +842,7 @@ static int edac_create_mci_instance_attributes(struct mem_ctl_info *mci, * directory of this mci instance. */ static void edac_remove_mci_instance_attributes(struct mem_ctl_info *mci, - struct mcidev_sysfs_attribute *sysfs_attrib, + const struct mcidev_sysfs_attribute *sysfs_attrib, struct kobject *kobj, int count) { struct mcidev_sysfs_group_kobj *grp_kobj, *tmp; -- cgit v1.2.3 From 6fe1108f14f4f9581af97cab752f37dc8fa9fdec Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Thu, 12 Aug 2010 00:30:25 -0300 Subject: edac_core: Do a better job with node removal Make sure we remove groups at the right order Signed-off-by: Mauro Carvalho Chehab --- drivers/edac/edac_mc.c | 1 + drivers/edac/edac_mc_sysfs.c | 71 ++++++++++++++++++++++++++------------------ 2 files changed, 43 insertions(+), 29 deletions(-) (limited to 'drivers/edac/edac_mc_sysfs.c') diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 6b7e723e46be..b10b45cc7870 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -207,6 +207,7 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows, } mci->op_state = OP_ALLOC; + INIT_LIST_HEAD(&mci->grp_kobj_list); /* * Initialize the 'root' kobj for the edac_mc controller diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index aacffe5d0f33..5a5734c1f6a5 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -791,6 +791,7 @@ static int edac_create_mci_instance_attributes(struct mem_ctl_info *mci, debugf1("%s()\n", __func__); while (sysfs_attrib) { + debugf1("%s() sysfs_attrib = %p\n",__func__, sysfs_attrib); if (sysfs_attrib->grp) { struct mcidev_sysfs_group_kobj *grp_kobj; @@ -798,10 +799,9 @@ static int edac_create_mci_instance_attributes(struct mem_ctl_info *mci, if (!grp_kobj) return -ENOMEM; - list_add_tail(&grp_kobj->list, &mci->grp_kobj_list); - grp_kobj->grp = sysfs_attrib->grp; grp_kobj->mci = mci; + list_add_tail(&grp_kobj->list, &mci->grp_kobj_list); debugf0("%s() grp %s, mci %p\n", __func__, sysfs_attrib->grp->name, mci); @@ -810,26 +810,28 @@ static int edac_create_mci_instance_attributes(struct mem_ctl_info *mci, &ktype_inst_grp, &mci->edac_mci_kobj, sysfs_attrib->grp->name); - if (err) + if (err < 0) { + printk(KERN_ERR "kobject_init_and_add failed: %d\n", err); return err; - + } err = edac_create_mci_instance_attributes(mci, grp_kobj->grp->mcidev_attr, &grp_kobj->kobj); - if (err) + if (err < 0) return err; } else if (sysfs_attrib->attr.name) { debugf0("%s() file %s\n", __func__, sysfs_attrib->attr.name); err = sysfs_create_file(kobj, &sysfs_attrib->attr); + if (err < 0) { + printk(KERN_ERR "sysfs_create_file failed: %d\n", err); + return err; + } } else break; - if (err) { - return err; - } sysfs_attrib++; } @@ -854,13 +856,24 @@ static void edac_remove_mci_instance_attributes(struct mem_ctl_info *mci, * Remove first all the atributes */ while (sysfs_attrib) { + debugf1("%s() sysfs_attrib = %p\n",__func__, sysfs_attrib); if (sysfs_attrib->grp) { - list_for_each_entry(grp_kobj, &mci->grp_kobj_list, - list) - if (grp_kobj->grp == sysfs_attrib->grp) + debugf1("%s() seeking for group %s\n", + __func__, sysfs_attrib->grp->name); + list_for_each_entry(grp_kobj, + &mci->grp_kobj_list, list) { + debugf1("%s() grp_kobj->grp = %p\n",__func__, grp_kobj->grp); + if (grp_kobj->grp == sysfs_attrib->grp) { edac_remove_mci_instance_attributes(mci, grp_kobj->grp->mcidev_attr, &grp_kobj->kobj, count + 1); + debugf0("%s() group %s\n", __func__, + sysfs_attrib->grp->name); + kobject_put(&grp_kobj->kobj); + } + } + debugf1("%s() end of seeking for group %s\n", + __func__, sysfs_attrib->grp->name); } else if (sysfs_attrib->attr.name) { debugf0("%s() file %s\n", __func__, sysfs_attrib->attr.name); @@ -870,15 +883,14 @@ static void edac_remove_mci_instance_attributes(struct mem_ctl_info *mci, sysfs_attrib++; } - /* - * Now that all attributes got removed, it is save to remove all groups - */ - if (!count) - list_for_each_entry_safe(grp_kobj, tmp, &mci->grp_kobj_list, - list) { - debugf0("%s() grp %s\n", __func__, grp_kobj->grp->name); - kobject_put(&grp_kobj->kobj); - } + /* Remove the group objects */ + if (count) + return; + list_for_each_entry_safe(grp_kobj, tmp, + &mci->grp_kobj_list, list) { + list_del(&grp_kobj->list); + kfree(grp_kobj); + } } @@ -970,6 +982,7 @@ void edac_remove_sysfs_mci_device(struct mem_ctl_info *mci) debugf0("%s()\n", __func__); /* remove all csrow kobjects */ + debugf0("%s() unregister this mci kobj\n", __func__); for (i = 0; i < mci->nr_csrows; i++) { if (mci->csrows[i].nr_pages > 0) { debugf0("%s() unreg csrow-%d\n", __func__, i); @@ -977,20 +990,20 @@ void edac_remove_sysfs_mci_device(struct mem_ctl_info *mci) } } - debugf0("%s() remove_link\n", __func__); + /* remove this mci instance's attribtes */ + if (mci->mc_driver_sysfs_attributes) { + debugf0("%s() unregister mci private attributes\n", __func__); + edac_remove_mci_instance_attributes(mci, + mci->mc_driver_sysfs_attributes, + &mci->edac_mci_kobj, 0); + } /* remove the symlink */ + debugf0("%s() remove_link\n", __func__); sysfs_remove_link(&mci->edac_mci_kobj, EDAC_DEVICE_SYMLINK); - debugf0("%s() remove_mci_instance\n", __func__); - - /* remove this mci instance's attribtes */ - edac_remove_mci_instance_attributes(mci, - mci->mc_driver_sysfs_attributes, - &mci->edac_mci_kobj, 0); - debugf0("%s() unregister this mci kobj\n", __func__); - /* unregister this instance's kobject */ + debugf0("%s() remove_mci_instance\n", __func__); kobject_put(&mci->edac_mci_kobj); } -- cgit v1.2.3 From ac99768c534ebde637b506ce9a6f5638d2049a5d Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Thu, 12 Aug 2010 13:09:21 -0300 Subject: edac_core: Don't let free(mci) happen while using it A very nasty bug were happening on edac core, due to the way mci objects are freed. mci memory is freed when kobject count reaches zero, by edac_mci_control_release(). However, from the logs, this is clearly happening before the final usage of mci struct: [15799.607454] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 640: edac_mci_control_release() mci instance idx=0 releasing [15799.618773] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 769: edac_inst_grp_release() [15799.627326] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 894: edac_remove_mci_instance_attributes() end of seeking for group all_channel_counts [15799.640887] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 877: edac_remove_mci_instance_attributes() sysfs_attrib = ffffffffa01d7240 [15799.653412] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 1020: edac_remove_sysfs_mci_device() remove_link [15799.663753] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 1024: edac_remove_sysfs_mci_device() remove_mci_instance Signed-off-by: Mauro Carvalho Chehab --- drivers/edac/edac_mc_sysfs.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/edac/edac_mc_sysfs.c') diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index 5a5734c1f6a5..7024b873a3b9 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -759,8 +759,6 @@ static void edac_inst_grp_release(struct kobject *kobj) grp = container_of(kobj, struct mcidev_sysfs_group_kobj, kobj); mci = grp->mci; - - kobject_put(&mci->edac_mci_kobj); } /* Intermediate show/store table */ -- cgit v1.2.3 From bbc560ae677c0f4d7ff8404a21409c99f35b297b Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Mon, 16 Aug 2010 18:22:43 -0300 Subject: edac_core: Print debug messages at release calls This is important to track a nasty bug at the free logic. Signed-off-by: Mauro Carvalho Chehab --- drivers/edac/edac_mc.c | 2 ++ drivers/edac/edac_mc_sysfs.c | 2 ++ drivers/edac/i7core_edac.c | 1 + 3 files changed, 5 insertions(+) (limited to 'drivers/edac/edac_mc_sysfs.c') diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index b10b45cc7870..889ce7566b56 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -235,6 +235,8 @@ EXPORT_SYMBOL_GPL(edac_mc_alloc); */ void edac_mc_free(struct mem_ctl_info *mci) { + debugf1("%s()\n", __func__); + edac_mc_unregister_sysfs_main_kobj(mci); } EXPORT_SYMBOL_GPL(edac_mc_free); diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index 7024b873a3b9..ddd765253630 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -712,6 +712,8 @@ fail_out: */ void edac_mc_unregister_sysfs_main_kobj(struct mem_ctl_info *mci) { + debugf1("%s()\n", __func__); + /* delete the kobj from the mc_kset */ kobject_put(&mci->edac_mci_kobj); } diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c index 11c61b4d8149..b0559973c66f 100644 --- a/drivers/edac/i7core_edac.c +++ b/drivers/edac/i7core_edac.c @@ -2086,6 +2086,7 @@ static void __devexit i7core_remove(struct pci_dev *pdev) edac_mc_del_mc(&i7core_dev->pdev[0]->dev); /* Free data */ + debugf1("%s: free structs\n"); kfree(mci->ctl_name); edac_mc_free(mci); -- cgit v1.2.3 From accf74fff36315a31dc7319dae2927af06e9296f Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Mon, 16 Aug 2010 18:34:37 -0300 Subject: i7core_edac: don't use a freed mci struct This is a nasty bug. Since kobject count will be reduced by zero by edac_mc_del_mc(), and this triggers the kobj release method, the mci memory will be freed automatically. So, all we have left is ctl_name, as shown by enabling debug: [ 80.822186] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 1020: edac_remove_sysfs_mci_device() remove_link [ 80.832590] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 1024: edac_remove_sysfs_mci_device() remove_mci_instance [ 80.843776] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 640: edac_mci_control_release() mci instance idx=0 releasing [ 80.855163] EDAC MC: Removed device 0 for i7core_edac.c i7 core #0: DEV 0000:3f:03.0 [ 80.862936] EDAC DEBUG: in drivers/edac/i7core_edac.c, line at 2089: (null): free structs [ 80.871134] EDAC DEBUG: in drivers/edac/edac_mc.c, line at 238: edac_mc_free() [ 80.878379] EDAC DEBUG: in drivers/edac/edac_mc_sysfs.c, line at 726: edac_mc_unregister_sysfs_main_kobj() [ 80.888043] EDAC DEBUG: in drivers/edac/i7core_edac.c, line at 1232: drivers/edac/i7core_edac.c: i7core_put_devices() Also, kfree(mci) shouldn't happen at the kobj.release, as it happens when edac_remove_sysfs_mci_device() is called, but the logic is: edac_remove_sysfs_mci_device(mci); edac_printk(KERN_INFO, EDAC_MC, "Removed device %d for %s %s: DEV %s\n", mci->mc_idx, mci->mod_name, mci->ctl_name, edac_dev_name(mci)); So, as the edac_printk() needs the mci struct, this generates an OOPS. Signed-off-by: Mauro Carvalho Chehab --- drivers/edac/edac_mc.c | 3 +++ drivers/edac/edac_mc_sysfs.c | 3 --- drivers/edac/i7core_edac.c | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) (limited to 'drivers/edac/edac_mc_sysfs.c') diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 889ce7566b56..ba6586a69ccc 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -238,6 +238,9 @@ void edac_mc_free(struct mem_ctl_info *mci) debugf1("%s()\n", __func__); edac_mc_unregister_sysfs_main_kobj(mci); + + /* free the mci instance memory here */ + kfree(mci); } EXPORT_SYMBOL_GPL(edac_mc_free); diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index ddd765253630..2905dc103393 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -630,9 +630,6 @@ static void edac_mci_control_release(struct kobject *kobj) /* decrement the module ref count */ module_put(mci->owner); - - /* free the mci instance memory here */ - kfree(mci); } static struct kobj_type ktype_mci = { diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c index b0559973c66f..8e789a2e35d6 100644 --- a/drivers/edac/i7core_edac.c +++ b/drivers/edac/i7core_edac.c @@ -2085,8 +2085,7 @@ static void __devexit i7core_remove(struct pci_dev *pdev) /* Remove MC sysfs nodes */ edac_mc_del_mc(&i7core_dev->pdev[0]->dev); - /* Free data */ - debugf1("%s: free structs\n"); + debugf1("%s: free mci struct\n", mci->ctl_name); kfree(mci->ctl_name); edac_mc_free(mci); -- cgit v1.2.3