drivers/edac: fix edac_mc sysfs completion code

8096cfaf · Doug Thompson <[email protected]> · 2007-07-19 01:50:27 -07:00

This patch refactors the 'releasing' of kobjects for the edac_mc type of
device.  The correct pattern of kobject release is followed.

As internal kobjs are allocated they bump a ref count on the top level kobj.
It in turn has a module ref count on the edac_core module.  When internal
kobjects are released, they dec the ref count on the top level kobj.  When the
top level kobj reaches zero, it decrements the ref count on the edac_core
object, allow it to be unloaded, as all resources have all now been released.

Cc: Alan Cox [email protected]
Signed-off-by: Doug Thompson <[email protected]>
Acked-by: Greg KH <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>

Commit: 8096cfafbb7ad3cb1a286ae7e8086167f4ebb4b6
Parents: d45e7823

Changes

5 files changed · +399 −249

File	Status	Add	Del	Content
drivers/edac/edac_core.h	modified	+1	−3	View file
drivers/edac/edac_mc.c	modified	+21	−15	View file
drivers/edac/edac_mc_sysfs.c	modified	+355	−213	View file
drivers/edac/edac_module.c	modified	+16	−16	View file
drivers/edac/edac_module.h	modified	+6	−2	View file

Only the first 600 diff lines are shown. Download the complete patch.

drivers/edac/edac_core.h

View file

diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
index 2c399c5..bd7f00c 100644
--- a/drivers/edac/edac_core.h
+++ b/drivers/edac/edac_core.h
@@ -317,9 +317,8 @@ 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 */
+	/* channel information for this csrow */
 	u32 nr_channels;
 	struct channel_info *channels;
 };
@@ -403,7 +402,6 @@ struct mem_ctl_info {
 
 	/* edac sysfs device control */
 	struct kobject edac_mci_kobj;
-	struct completion kobj_complete;
 
 	/* Additional top controller level attributes, but specified
 	 * by the low level driver.

drivers/edac/edac_mc.c

View file

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 6e4c94e..2d53cb3 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -137,6 +137,7 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
 	void *pvt;
 	unsigned size;
 	int row, chn;
+	int err;
 
 	/* Figure out the offsets of the various items from the start of an mc
 	 * structure.  We want the alignment of each item to be at least as
@@ -149,7 +150,8 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
 	pvt = edac_align_ptr(&chi[nr_chans * nr_csrows], sz_pvt);
 	size = ((unsigned long)pvt) + sz_pvt;
 
-	if ((mci = kmalloc(size, GFP_KERNEL)) == NULL)
+	mci = kzalloc(size, GFP_KERNEL);
+	if (mci == NULL)
 		return NULL;
 
 	/* Adjust pointers so they point within the memory we just allocated
@@ -182,20 +184,34 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt, unsigned nr_csrows,
 
 	mci->op_state = OP_ALLOC;
 
+	/*
+	 * Initialize the 'root' kobj for the edac_mc controller
+	 */
+	err = edac_mc_register_sysfs_main_kobj(mci);
+	if (err) {
+		kfree(mci);
+		return NULL;
+	}
+
+	/* at this point, the root kobj is valid, and in order to
+	 * 'free' the object, then the function:
+	 *      edac_mc_unregister_sysfs_main_kobj() must be called
+	 * which will perform kobj unregistration and the actual free
+	 * will occur during the kobject callback operation
+	 */
 	return mci;
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_alloc);
 
 /**
- * edac_mc_free:  Free a previously allocated 'mci' structure
+ * edac_mc_free
+ *	'Free' a previously allocated 'mci' structure
  * @mci: pointer to a struct mem_ctl_info structure
  */
 void edac_mc_free(struct mem_ctl_info *mci)
 {
-	kfree(mci);
+	edac_mc_unregister_sysfs_main_kobj(mci);
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_free);
 
 static struct mem_ctl_info *find_mci_by_dev(struct device *dev)
@@ -391,7 +407,6 @@ struct mem_ctl_info *edac_mc_find(int idx)
 
 	return NULL;
 }
-
 EXPORT_SYMBOL(edac_mc_find);
 
 /**
@@ -465,7 +480,6 @@ fail0:
 	mutex_unlock(&mem_ctls_mutex);
 	return 1;
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_add_mc);
 
 /**
@@ -501,7 +515,6 @@ struct mem_ctl_info *edac_mc_del_mc(struct device *dev)
 		mci->mod_name, mci->ctl_name, dev_name(mci));
 	return mci;
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_del_mc);
 
 static void edac_mc_scrub_block(unsigned long page, unsigned long offset,
@@ -571,7 +584,6 @@ int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci, unsigned long page)
 
 	return row;
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_find_csrow_by_page);
 
 /* FIXME - setable log (warning/emerg) levels */
@@ -636,7 +648,6 @@ void edac_mc_handle_ce(struct mem_ctl_info *mci,
 				mci->csrows[row].grain);
 	}
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_handle_ce);
 
 void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci, const char *msg)
@@ -648,7 +659,6 @@ void edac_mc_handle_ce_no_info(struct mem_ctl_info *mci, const char *msg)
 	mci->ce_noinfo_count++;
 	mci->ce_count++;
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_handle_ce_no_info);
 
 void edac_mc_handle_ue(struct mem_ctl_info *mci,
@@ -702,7 +712,6 @@ void edac_mc_handle_ue(struct mem_ctl_info *mci,
 	mci->ue_count++;
 	mci->csrows[row].ue_count++;
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_handle_ue);
 
 void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci, const char *msg)
@@ -716,7 +725,6 @@ void edac_mc_handle_ue_no_info(struct mem_ctl_info *mci, const char *msg)
 	mci->ue_noinfo_count++;
 	mci->ue_count++;
 }
-
 EXPORT_SYMBOL_GPL(edac_mc_handle_ue_no_info);
 
 /*************************************************************
@@ -784,7 +792,6 @@ void edac_mc_handle_fbd_ue(struct mem_ctl_info *mci,
 			"labels \"%s\": %s\n", csrow, channela,
 			channelb, labels, msg);
 }
-
 EXPORT_SYMBOL(edac_mc_handle_fbd_ue);
 
 /*************************************************************
@@ -824,7 +831,6 @@ void edac_mc_handle_fbd_ce(struct mem_ctl_info *mci,
 	mci->csrows[csrow].ce_count++;
 	mci->csrows[csrow].channels[channel].ce_count++;
 }
-
 EXPORT_SYMBOL(edac_mc_handle_fbd_ce);
 
 /*

drivers/edac/edac_mc_sysfs.c

View file

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 0843eaa..cd090b0 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -10,10 +10,12 @@
  */
 
 #include <linux/ctype.h>
+#include <linux/bug.h>
 
 #include "edac_core.h"
 #include "edac_module.h"
 
+
 /* MC EDAC Controls, setable by module parameter, and sysfs */
 static int edac_mc_log_ue = 1;
 static int edac_mc_log_ce = 1;
@@ -98,15 +100,7 @@ static const char *edac_caps[] = {
 	[EDAC_S16ECD16ED] = "S16ECD16ED"
 };
 
-/* sysfs object:
- *	/sys/devices/system/edac/mc
- */
-static struct kobject edac_memctrl_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;
 
 /*
  * /sys/devices/system/edac/mc;
@@ -128,153 +122,6 @@ static ssize_t memctrl_int_store(void *ptr, const char *buffer, size_t count)
 	return count;
 }
 
-struct memctrl_dev_attribute {
-	struct attribute attr;
-	void *value;
-	 ssize_t(*show) (void *, char *);
-	 ssize_t(*store) (void *, const char *, size_t);
-};
-
-/* Set of show/store abstract level functions for memory control object */
-static ssize_t memctrl_dev_show(struct kobject *kobj,
-				struct attribute *attr, char *buffer)
-{
-	struct memctrl_dev_attribute *memctrl_dev;
-	memctrl_dev = (struct memctrl_dev_attribute *)attr;
-
-	if (memctrl_dev->show)
-		return memctrl_dev->show(memctrl_dev->value, buffer);
-
-	return -EIO;
-}
-
-static ssize_t memctrl_dev_store(struct kobject *kobj, struct attribute *attr,
-				 const char *buffer, size_t count)
-{
-	struct memctrl_dev_attribute *memctrl_dev;
-	memctrl_dev = (struct memctrl_dev_attribute *)attr;
-
-	if (memctrl_dev->store)
-		return memctrl_dev->store(memctrl_dev->value, buffer, count);
-
-	return -EIO;
-}
-
-static struct sysfs_ops memctrlfs_ops = {
-	.show = memctrl_dev_show,
-	.store = memctrl_dev_store
-};
-
-#define MEMCTRL_ATTR(_name,_mode,_show,_store)			\
-static struct memctrl_dev_attribute attr_##_name = {			\
-	.attr = {.name = __stringify(_name), .mode = _mode },	\
-	.value  = &_name,					\
-	.show   = _show,					\
-	.store  = _store,					\
-};
-
-#define MEMCTRL_STRING_ATTR(_name,_data,_mode,_show,_store)	\
-static struct memctrl_dev_attribute attr_##_name = {			\
-	.attr = {.name = __stringify(_name), .mode = _mode },	\
-	.value  = _data,					\
-	.show   = _show,					\
-	.store  = _store,					\
-};
-
-/* csrow<id> control files */
-MEMCTRL_ATTR(edac_mc_panic_on_ue,
-	S_IRUGO | S_IWUSR, memctrl_int_show, memctrl_int_store);
-
-MEMCTRL_ATTR(edac_mc_log_ue,
-	S_IRUGO | S_IWUSR, memctrl_int_show, memctrl_int_store);
-
-MEMCTRL_ATTR(edac_mc_log_ce,
-	S_IRUGO | S_IWUSR, memctrl_int_show, memctrl_int_store);
-
-MEMCTRL_ATTR(edac_mc_poll_msec,
-	S_IRUGO | S_IWUSR, memctrl_int_show, memctrl_int_store);
-
-/* Base Attributes of the memory ECC object */
-static struct memctrl_dev_attribute *memctrl_attr[] = {
-	&attr_edac_mc_panic_on_ue,
-	&attr_edac_mc_log_ue,
-	&attr_edac_mc_log_ce,
-	&attr_edac_mc_poll_msec,
-	NULL,
-};
-
-/* Main MC kobject release() function */
-static void edac_memctrl_master_release(struct kobject *kobj)
-{
-	debugf1("%s()\n", __func__);
-	complete(&edac_memctrl_kobj_complete);
-}
-
-static struct kobj_type ktype_memctrl = {
-	.release = edac_memctrl_master_release,
-	.sysfs_ops = &memctrlfs_ops,
-	.default_attrs = (struct attribute **)memctrl_attr,
-};
-
-/* Initialize the main sysfs entries for edac:
- *   /sys/devices/system/edac
- *
- * and children
- *
- * Return:  0 SUCCESS
- *         !0 FAILURE
- */
-int edac_sysfs_memctrl_setup(void)
-{
-	int err = 0;
-	struct sysdev_class *edac_class;
-
-	debugf1("%s()\n", __func__);
-
-	/* get the /sys/devices/system/edac class reference */
-	edac_class = edac_get_edac_class();
-	if (edac_class == NULL) {
-		debugf1("%s() no edac_class error=%d\n", __func__, err);
-		return err;
-	}
-
-	/* Init the MC's kobject */
-	memset(&edac_memctrl_kobj, 0, sizeof(edac_memctrl_kobj));
-	edac_memctrl_kobj.parent = &edac_class->kset.kobj;
-	edac_memctrl_kobj.ktype = &ktype_memctrl;
-
-	/* generate sysfs "..../edac/mc"   */
-	err = kobject_set_name(&edac_memctrl_kobj, "mc");
-	if (err) {
-		debugf1("%s() Failed to set name '.../edac/mc'\n", __func__);
-		return err;
-	}
-
-	/* FIXME: maybe new sysdev_create_subdir() */
-	err = kobject_register(&edac_memctrl_kobj);
-	if (err) {
-		debugf1("%s() Failed to register '.../edac/mc'\n", __func__);
-		return err;
-	}
-
-	debugf1("%s() Registered '.../edac/mc' kobject\n", __func__);
-	return 0;
-}
-
-/*
- * MC teardown:
- *	the '..../edac/mc' kobject followed by '..../edac' itself
- */
-void edac_sysfs_memctrl_teardown(void)
-{
-	debugf0("MC: " __FILE__ ": %s()\n", __func__);
-
-	/* 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);
-}
 
 /* EDAC sysfs CSROW data structures and methods
  */
@@ -486,10 +333,15 @@ static int edac_create_channel_files(struct kobject *kobj, int chan)
 /* No memory to release for this kobj */
 static void edac_csrow_instance_release(struct kobject *kobj)
 {
+	struct mem_ctl_info *mci;
 	struct csrow_info *cs;
 
+	debugf1("%s()\n", __func__);
+
 	cs = container_of(kobj, struct csrow_info, kobj);
-	complete(&cs->kobj_complete);
+	mci = cs->mci;
+
+	kobject_put(&mci->edac_mci_kobj);
 }
 
 /* the kobj_type instance for a CSROW */
@@ -500,38 +352,61 @@ static struct kobj_type ktype_csrow = {
 };
 
 /* Create a CSROW object under specifed edac_mc_device */
-static int edac_create_csrow_object(struct kobject *edac_mci_kobj,
-				struct csrow_info *csrow, int index)
+static int edac_create_csrow_object(struct mem_ctl_info *mci,
+					struct csrow_info *csrow, int index)
 {
-	int err = 0;
+	struct kobject *kobj_mci = &mci->edac_mci_kobj;
+	struct kobject *kobj;
 	int chan;
-
-	memset(&csrow->kobj, 0, sizeof(csrow->kobj));
+	int err;
 
 	/* generate ..../edac/mc/mc<id>/csrow<index>   */
-
-	csrow->kobj.parent = edac_mci_kobj;
+	memset(&csrow->kobj, 0, sizeof(csrow->kobj));
+	csrow->mci = mci;	/* include container up link */
+	csrow->kobj.parent = kobj_mci;
 	csrow->kobj.ktype = &ktype_csrow;
 
 	/* name this instance of csrow<id> */
 	err = kobject_set_name(&csrow->kobj, "csrow%d", index);
 	if (err)
-		goto error_exit;
+		goto err_out;
+
+	/* bump the mci instance's kobject's ref count */
+	kobj = kobject_get(&mci->edac_mci_kobj);
+	if (!kobj) {
+		err = -ENODEV;
+		goto err_out;
+	}
 
 	/* Instanstiate the csrow object */
 	err = kobject_register(&csrow->kobj);
-	if (!err) {
-		/* Create the dyanmic attribute files on this csrow,
-		 * namely, the DIMM labels and the channel ce_count
-		 */
-		for (chan = 0; chan < csrow->nr_channels; chan++) {
-			err = edac_create_channel_files(&csrow->kobj, chan);
-			if (err)
-				break;
+	if (err)
+		goto err_release_top_kobj;
+
+	/* At this point, to release a csrow kobj, one must
+	 * call the kobject_unregister and allow that tear down
+	 * to work the releasing
+	 */
+
+	/* Create the dyanmic attribute files on this csrow,
+	 * namely, the DIMM labels and the channel ce_count
+	 */
+	for (chan = 0; chan < csrow->nr_channels; chan++) {
+		err = edac_create_channel_files(&csrow->kobj, chan);
+		if (err) {
+			/* special case the unregister here */
+			kobject_unregister(&csrow->kobj);
+			goto err_out;
 		}
 	}
 
-error_exit:
+	return 0;
+
+	/* error unwind stack */
+err_release_top_kobj:
+	kobject_put(&mci->edac_mci_kobj);
+
+err_out:
 	return err;
 }
 
@@ -688,6 +563,7 @@ static ssize_t mcidev_store(struct kobject *kobj, struct attribute *attr,
 	return -EIO;
 }
 
+/* Intermediate show/store table */
 static struct sysfs_ops mci_ops = {
 	.show = mcidev_show,
 	.store = mcidev_store
@@ -729,32 +605,213 @@ static struct mcidev_sysfs_attribute *mci_attr[] = {
 	NULL
 };
 
+
 /*
  * Release of a MC controlling instance
+ *
+ *	each MC control instance has the following resources upon entry:
+ *		a) a ref count on the top memctl kobj
+ *		b) a ref count on this module
+ *
+ *	this function must decrement those ref counts and then
+ *	issue a free on the instance's memory
  */
-static void edac_mci_instance_release(struct kobject *kobj)
+static void edac_mci_control_release(struct kobject *kobj)
 {
 	struct mem_ctl_info *mci;
 
 	mci = to_mci(kobj);
-	debugf0("%s() idx=%d\n", __func__, mci->mc_idx);
-	complete(&mci->kobj_complete);
+
+	debugf0("%s() mci instance idx=%d releasing\n", __func__, mci->mc_idx);
+
+	/* decrement the module ref count */
+	module_put(mci->owner);
+
+	/* free the mci instance memory here */
+	kfree(mci);
 }
 
 static struct kobj_type ktype_mci = {
-	.release = edac_mci_instance_release,
+	.release = edac_mci_control_release,
 	.sysfs_ops = &mci_ops,
 	.default_attrs = (struct attribute **)mci_attr,
 };
 
+/* show/store, tables, etc for the MC kset */
+
+
+struct memctrl_dev_attribute {
+	struct attribute attr;
+	void *value;
+	 ssize_t(*show) (void *, char *);
+	 ssize_t(*store) (void *, const char *, size_t);
+};
+
+/* Set of show/store abstract level functions for memory control object */
+static ssize_t memctrl_dev_show(struct kobject *kobj,
+				struct attribute *attr, char *buffer)
+{
+	struct memctrl_dev_attribute *memctrl_dev;
+	memctrl_dev = (struct memctrl_dev_attribute *)attr;
+
+	if (memctrl_dev->show)
+		return memctrl_dev->show(memctrl_dev->value, buffer);
+
+	return -EIO;
+}
+
+static ssize_t memctrl_dev_store(struct kobject *kobj, struct attribute *attr,
+				 const char *buffer, size_t count)
+{
+	struct memctrl_dev_attribute *memctrl_dev;
+	memctrl_dev = (struct memctrl_dev_attribute *)attr;
+
+	if (memctrl_dev->store)
+		return memctrl_dev->store(memctrl_dev->value, buffer, count);
+
+	return -EIO;
+}
+
+static struct sysfs_ops memctrlfs_ops = {
+	.show = memctrl_dev_show,
+	.store = memctrl_dev_store
+};
+
+#define MEMCTRL_ATTR(_name, _mode, _show, _store)			\
+static struct memctrl_dev_attribute attr_##_name = {			\
+	.attr = {.name = __stringify(_name), .mode = _mode },	\
+	.value  = &_name,					\
+	.show   = _show,					\
+	.store  = _store,					\
+};
+
+#define MEMCTRL_STRING_ATTR(_name, _data, _mode, _show, _store)	\
+static struct memctrl_dev_attribute attr_##_name = {			\
+	.attr = {.name = __stringify(_name), .mode = _mode },	\
+	.value  = _data,					\
+	.show   = _show,					\
+	.store  = _store,					\
+};
+
+/* csrow<id> control files */
+MEMCTRL_ATTR(edac_mc_panic_on_ue,
+	S_IRUGO | S_IWUSR, memctrl_int_show, memctrl_int_store);
+
+MEMCTRL_ATTR(edac_mc_log_ue,
+	S_IRUGO | S_IWUSR, memctrl_int_show, memctrl_int_store);
+
+MEMCTRL_ATTR(edac_mc_log_ce,
+	S_IRUGO | S_IWUSR, memctrl_int_show, memctrl_int_store);
+
+MEMCTRL_ATTR(edac_mc_poll_msec,
+	S_IRUGO | S_IWUSR, memctrl_int_show, memctrl_int_store);
+
+/* Base Attributes of the memory ECC object */
+static struct memctrl_dev_attribute *memctrl_attr[] = {
+	&attr_edac_mc_panic_on_ue,
+	&attr_edac_mc_log_ue,
+	&attr_edac_mc_log_ce,
+	&attr_edac_mc_poll_msec,
+	NULL,
+};
+
+
+/* the ktype for the mc_kset internal kobj */
+static struct kobj_type ktype_mc_set_attribs = {
+	.sysfs_ops = &memctrlfs_ops,
+	.default_attrs = (struct attribute **)memctrl_attr,
+};
+
+/* EDAC memory controller sysfs kset:
+ *	/sys/devices/system/edac/mc
+ */
+static struct kset mc_kset = {
+	.kobj = {.name = "mc", .ktype = &ktype_mc_set_attribs },
+	.ktype = &ktype_mci,
+};
+
+
+/*
+ * edac_mc_register_sysfs_main_kobj
+ *
+ *	setups and registers the main kobject for each mci
+ */
+int edac_mc_register_sysfs_main_kobj(struct mem_ctl_info *mci)
+{
+	struct kobject *kobj_mci;
+	int err;
+
+	debugf1("%s()\n", __func__);

drivers/edac: fix edac_mc sysfs completion code

Changes

drivers/edac/edac_core.h

drivers/edac/edac_mc.c

drivers/edac/edac_mc_sysfs.c

drivers/edac/edac_module.c

drivers/edac/edac_module.h