[PATCH] EDAC: kobject/sysfs fixes

- After we unregister a kobject, wait for our kobject release method
  to call complete().  This causes us to wait until the kobject
  reference count reaches 0.  Otherwise, a task accessing the EDAC
  sysfs interface can hold the reference count above 0 until after the
  EDAC module has been unloaded.  When the reference count finally
  drops to 0, this will result in an attempt to call our release
  method inside the EDAC module after the module has already been
  unloaded.

  This isn't the best fix, since a process can get stuck sleeping forever
  uninterruptibly if the user does the following:

      rmmod my_module < /sys/my_sysfs/file

  I'll go back and implement a better fix later.  However this should
  be ok for now.

- Call edac_remove_sysfs_mci_device() from edac_mc_del_mc() rather
  than from edac_mc_free().  Since edac_mc_add_mc() calls
  edac_create_sysfs_mci_device(), edac_mc_del_mc() should call
  edac_remove_sysfs_mci_device().

Signed-off-by: David S. Peterson <dsp@llnl.gov>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index e6ecc7da38a5cb0fa9921de062b8cc10fac996d8..7ee9419234aa29ff32d7930b11b572f85d202abc 100644 (file)
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -139,6 +139,12 @@ static struct sysdev_class edac_class = {
 static struct kobject edac_memctrl_kobj;
 static struct kobject edac_pci_kobj;
 
+/* We use these to wait for the reference counts on edac_memctrl_kobj and
+ * edac_pci_kobj to reach 0.
+ */
+static struct completion edac_memctrl_kobj_complete;
+static struct completion edac_pci_kobj_complete;
+
 /*
  * /sys/devices/system/edac/mc;
  *     data structures and methods
@@ -244,6 +250,7 @@ static struct memctrl_dev_attribute *memctrl_attr[] = {
 static void edac_memctrl_master_release(struct kobject *kobj)
 {
        debugf1("%s()\n", __func__);
+       complete(&edac_memctrl_kobj_complete);
 }
 
 static struct kobj_type ktype_memctrl = {
@@ -309,8 +316,12 @@ static void edac_sysfs_memctrl_teardown(void)
 #ifndef DISABLE_EDAC_SYSFS
        debugf0("MC: " __FILE__ ": %s()\n", __func__);
 
-       /* Unregister the MC's kobject */
+       /* Unregister the MC's kobject and wait for reference count to reach
+        * 0.
+        */
+       init_completion(&edac_memctrl_kobj_complete);
        kobject_unregister(&edac_memctrl_kobj);
+       wait_for_completion(&edac_memctrl_kobj_complete);
 
        /* Unregister the 'edac' object */
        sysdev_class_unregister(&edac_class);
@@ -563,6 +574,7 @@ static struct edac_pci_dev_attribute *edac_pci_attr[] = {
 static void edac_pci_release(struct kobject *kobj)
 {
        debugf1("%s()\n", __func__);
+       complete(&edac_pci_kobj_complete);
 }
 
 static struct kobj_type ktype_edac_pci = {
@@ -610,8 +622,9 @@ static void edac_sysfs_pci_teardown(void)
 {
 #ifndef DISABLE_EDAC_SYSFS
        debugf0("%s()\n", __func__);
-
+       init_completion(&edac_pci_kobj_complete);
        kobject_unregister(&edac_pci_kobj);
+       wait_for_completion(&edac_pci_kobj_complete);
 #endif
 }
 
@@ -800,7 +813,11 @@ static struct csrowdev_attribute *csrow_attr[] = {
 /* No memory to release */
 static void edac_csrow_instance_release(struct kobject *kobj)
 {
+       struct csrow_info *cs;
+
        debugf1("%s()\n", __func__);
+       cs = container_of(kobj, struct csrow_info, kobj);
+       complete(&cs->kobj_complete);
 }
 
 static struct kobj_type ktype_csrow = {
@@ -1055,11 +1072,10 @@ static struct mcidev_attribute *mci_attr[] = {
 static void edac_mci_instance_release(struct kobject *kobj)
 {
        struct mem_ctl_info *mci;
-       mci = container_of(kobj,struct mem_ctl_info,edac_mci_kobj);
 
-       debugf0("%s() idx=%d calling kfree\n", __func__, mci->mc_idx);
-
-       kfree(mci);
+       mci = to_mci(kobj);
+       debugf0("%s() idx=%d\n", __func__, mci->mc_idx);
+       complete(&mci->kobj_complete);
 }
 
 static struct kobj_type ktype_mci = {
@@ -1131,21 +1147,23 @@ static int edac_create_sysfs_mci_device(struct mem_ctl_info *mci)
                }
        }
 
-       /* Mark this MCI instance as having sysfs entries */
-       mci->sysfs_active = MCI_SYSFS_ACTIVE;
-
        return 0;
 
 
        /* CSROW error: backout what has already been registered,  */
 fail1:
        for ( i--; i >= 0; i--) {
-               if (csrow->nr_pages > 0)
+               if (csrow->nr_pages > 0) {
+                       init_completion(&csrow->kobj_complete);
                        kobject_unregister(&mci->csrows[i].kobj);
+                       wait_for_completion(&csrow->kobj_complete);
+               }
        }
 
 fail0:
+       init_completion(&mci->kobj_complete);
        kobject_unregister(edac_mci_kobj);
+       wait_for_completion(&mci->kobj_complete);
 
        return err;
 }
@@ -1163,13 +1181,17 @@ static void edac_remove_sysfs_mci_device(struct mem_ctl_info *mci)
 
        /* remove all csrow kobjects */
        for (i = 0; i < mci->nr_csrows; i++) {
-               if (mci->csrows[i].nr_pages > 0)
+               if (mci->csrows[i].nr_pages > 0) {
+                       init_completion(&mci->csrows[i].kobj_complete);
                        kobject_unregister(&mci->csrows[i].kobj);
+                       wait_for_completion(&mci->csrows[i].kobj_complete);
+               }
        }
 
        sysfs_remove_link(&mci->edac_mci_kobj, EDAC_DEVICE_SYMLINK);
-
+       init_completion(&mci->kobj_complete);
        kobject_unregister(&mci->edac_mci_kobj);
+       wait_for_completion(&mci->kobj_complete);
 #endif  /* DISABLE_EDAC_SYSFS */
 }
 
@@ -1342,31 +1364,10 @@ EXPORT_SYMBOL(edac_mc_free);
 /**
  * edac_mc_free:  Free a previously allocated 'mci' structure
  * @mci: pointer to a struct mem_ctl_info structure
- *
- * Free up a previously allocated mci structure
- * A MCI structure can be in 2 states after being allocated
- * by edac_mc_alloc().
- *     1) Allocated in a MC driver's probe, but not yet committed
- *     2) Allocated and committed, by a call to  edac_mc_add_mc()
- * edac_mc_add_mc() is the function that adds the sysfs entries
- * thus, this free function must determine which state the 'mci'
- * structure is in, then either free it directly or
- * perform kobject cleanup by calling edac_remove_sysfs_mci_device().
- *
- * VOID Return
  */
 void edac_mc_free(struct mem_ctl_info *mci)
 {
-       /* only if sysfs entries for this mci instance exist
-        * do we remove them and defer the actual kfree via
-        * the kobject 'release()' callback.
-        *
-        * Otherwise, do a straight kfree now.
-        */
-       if (mci->sysfs_active == MCI_SYSFS_ACTIVE)
-               edac_remove_sysfs_mci_device(mci);
-       else
-               kfree(mci);
+       kfree(mci);
 }
 
 
@@ -1456,7 +1457,8 @@ static void del_mc_from_global_list (struct mem_ctl_info *mci)
 EXPORT_SYMBOL(edac_mc_add_mc);
 
 /**
- * edac_mc_add_mc: Insert the 'mci' structure into the mci global list
+ * edac_mc_add_mc: Insert the 'mci' structure into the mci global list and
+ *                 create sysfs entries associated with mci structure
  * @mci: pointer to the mci structure to be added to the list
  *
  * Return:
@@ -1516,7 +1518,8 @@ fail0:
 EXPORT_SYMBOL(edac_mc_del_mc);
 
 /**
- * edac_mc_del_mc:  Remove the specified mci structure from global list
+ * 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
  *
  * Returns:
@@ -1528,6 +1531,7 @@ int edac_mc_del_mc(struct mem_ctl_info *mci)
        int rc = 1;
 
        debugf0("MC%d: %s()\n", mci->mc_idx, __func__);
+       edac_remove_sysfs_mci_device(mci);
        down(&mem_ctls_mutex);
        del_mc_from_global_list(mci);
        edac_printk(KERN_INFO, EDAC_MC,

diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
index 0bcc3797c7535162f32fe93eebc0e414ced18967..c9c1590db7219f6d297527ca9d1a0c1ede9421a3 100644 (file)
--- a/drivers/edac/edac_mc.h
+++ b/drivers/edac/edac_mc.h
@@ -185,11 +185,6 @@ enum scrub_type {
 #define SCRUB_FLAG_HW_PROG_SRC BIT(SCRUB_HW_PROG_SRC_CORR)
 #define SCRUB_FLAG_HW_TUN      BIT(SCRUB_HW_TUNABLE)
 
-enum mci_sysfs_status {
-       MCI_SYSFS_INACTIVE = 0, /* sysfs entries NOT registered */
-       MCI_SYSFS_ACTIVE        /* sysfs entries ARE registered */
-};
-
 /* FIXME - should have notify capabilities: NMI, LOG, PROC, etc */
 
 /*
@@ -299,6 +294,7 @@ struct csrow_info {
        struct mem_ctl_info *mci;       /* the parent */
 
        struct kobject kobj;    /* sysfs kobject for this csrow */
+       struct completion kobj_complete;
 
        /* FIXME the number of CHANNELs might need to become dynamic */
        u32 nr_channels;
@@ -320,8 +316,6 @@ struct mem_ctl_info {
        unsigned long scrub_cap;        /* chipset scrub capabilities */
        enum scrub_type scrub_mode;     /* current scrub mode */
 
-       enum mci_sysfs_status sysfs_active;     /* status of sysfs */
-
        /* pointer to edac checking routine */
        void (*edac_check) (struct mem_ctl_info * mci);
        /*
@@ -359,6 +353,7 @@ struct mem_ctl_info {
 
        /* edac sysfs device control */
        struct kobject edac_mci_kobj;
+       struct completion kobj_complete;
 };