[PATCH] EDAC: mc numbers refactor 1-of-2

Remove add_mc_to_global_list(). In next patch, this function will be reimplemented with different semantics. 1 Reimplement add_mc_to_global_list() with semantics that allow the caller to determine the ID number for a mem_ctl_info structure. Then modify edac_mc_add_mc() so that the caller specifies the ID number for the new mem_ctl_info structure. Platform-specific code should be able to assign the ID numbers in a platform-specific manner. For instance, on Opteron it makes sense to have the ID of the mem_ctl_info structure match the ID of the node that the memory controller belongs to. 2 Modify callers of edac_mc_add_mc() so they use the new semantics. Signed-off-by: Doug Thompson <norsk5@xmission.com> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[PATCH] EDAC: mc numbers refactor 1-of-2
Remove add_mc_to_global_list(). In next patch, this function will be reimplemented with different semantics. 1 Reimplement add_mc_to_global_list() with semantics that allow the caller to determine the ID number for a mem_ctl_info structure. Then modify edac_mc_add_mc() so that the caller specifies the ID number for the new mem_ctl_info structure. Platform-specific code should be able to assign the ID numbers in a platform-specific manner. For instance, on Opteron it makes sense to have the ID of the mem_ctl_info structure match the ID of the node that the memory controller belongs to. 2 Modify callers of edac_mc_add_mc() so they use the new semantics. Signed-off-by: Doug Thompson <norsk5@xmission.com> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Doug Thompson · Linus Torvalds
1 parent 37f04581ab
Showing 8 changed files with 55 additions and 36 deletions Side-by-side Diff
drivers/edac/amd76x_edac.c
drivers/edac/e752x_edac.c
drivers/edac/e7xxx_edac.c
drivers/edac/edac_mc.c
drivers/edac/edac_mc.h
drivers/edac/i82860_edac.c
drivers/edac/i82875p_edac.c
drivers/edac/r82600_edac.c
@@ -257,7 +257,10 @@
  
 	amd76x_get_error_info(mci, &discard);  /* clear counters */
  
-	if (edac_mc_add_mc(mci)) {
+	/* Here we assume that we will never see multiple instances of this
+	 * type of memory controller.  The ID is therefore hardcoded to 0.
+	 */
+	if (edac_mc_add_mc(mci,0)) {
 		debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
 		goto fail;
 	}
@@ -953,7 +953,10 @@
 		"tolm = %x, remapbase = %x, remaplimit = %x\n", pvt->tolm,
 		pvt->remapbase, pvt->remaplimit);
  
-	if (edac_mc_add_mc(mci)) {
+	/* Here we assume that we will never see multiple instances of this
+	 * type of memory controller.  The ID is therefore hardcoded to 0.
+	 */
+	if (edac_mc_add_mc(mci,0)) {
 		debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
 		goto fail;
 	}
@@ -463,7 +463,10 @@
 	/* clear any pending errors, or initial state bits */
 	e7xxx_get_error_info(mci, &discard);
  
-	if (edac_mc_add_mc(mci) != 0) {
+	/* Here we assume that we will never see multiple instances of this
+	 * type of memory controller.  The ID is therefore hardcoded to 0.
+	 */
+	if (edac_mc_add_mc(mci,0)) {
 		debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
 		goto fail;
 	}
@@ -1632,47 +1632,46 @@
 	return NULL;
 }
  
-static int add_mc_to_global_list(struct mem_ctl_info *mci)
+/* Return 0 on success, 1 on failure.
+ * Before calling this function, caller must
+ * assign a unique value to mci->mc_idx.
+ */
+static int add_mc_to_global_list (struct mem_ctl_info *mci)
 {
 	struct list_head *item, *insert_before;
 	struct mem_ctl_info *p;
-	int i;
  
-	if (list_empty(&mc_devices)) {
-		mci->mc_idx = 0;
-		insert_before = &mc_devices;
-	} else {
-		if (find_mci_by_dev(mci->dev)) {
-			edac_printk(KERN_WARNING, EDAC_MC,
-				"%s (%s) %s %s already assigned %d\n",
-				mci->dev->bus_id, dev_name(mci->dev),
-				mci->mod_name, mci->ctl_name,
-				mci->mc_idx);
-			return 1;
-		}
+	insert_before = &mc_devices;
  
-		insert_before = NULL;
-		i = 0;
+	if (unlikely((p = find_mci_by_dev(mci->dev)) != NULL))
+		goto fail0;
  
-		list_for_each(item, &mc_devices) {
-			p = list_entry(item, struct mem_ctl_info, link);
+	list_for_each(item, &mc_devices) {
+		p = list_entry(item, struct mem_ctl_info, link);
  
-			if (p->mc_idx != i) {
-				insert_before = item;
-				break;
-			}
+		if (p->mc_idx >= mci->mc_idx) {
+			if (unlikely(p->mc_idx == mci->mc_idx))
+				goto fail1;
  
-			i++;
+			insert_before = item;
+			break;
 		}
-
-		mci->mc_idx = i;
-
-		if (insert_before == NULL)
-			insert_before = &mc_devices;
 	}
  
 	list_add_tail_rcu(&mci->link, insert_before);
 	return 0;
+
+fail0:
+	edac_printk(KERN_WARNING, EDAC_MC,
+		    "%s (%s) %s %s already assigned %d\n", p->dev->bus_id,
+		    dev_name(p->dev), p->mod_name, p->ctl_name, p->mc_idx);
+	return 1;
+
+fail1:
+	edac_printk(KERN_WARNING, EDAC_MC,
+		    "bug in low-level driver: attempt to assign\n"
+		    "    duplicate mc_idx %d in %s()\n", p->mc_idx, __func__);
+	return 1;
 }
  
 static void complete_mc_list_del(struct rcu_head *head)
@@ -1696,6 +1695,7 @@
  * 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
+ * @mc_idx: A unique numeric identifier to be assigned to the 'mci' structure.
  *
  * Return:
  *	0	Success
  
@@ -1703,9 +1703,10 @@
  */
  
 /* FIXME - should a warning be printed if no error detection? correction? */
-int edac_mc_add_mc(struct mem_ctl_info *mci)
+int edac_mc_add_mc(struct mem_ctl_info *mci, int mc_idx)
 {
 	debugf0("%s()\n", __func__);
+	mci->mc_idx = mc_idx;
 #ifdef CONFIG_EDAC_DEBUG
 	if (edac_debug_level >= 3)
 		edac_mc_dump_mci(mci);
@@ -417,7 +417,7 @@
 void edac_mc_dump_csrow(struct csrow_info *csrow);
 #endif  /* CONFIG_EDAC_DEBUG */
  
-extern int edac_mc_add_mc(struct mem_ctl_info *mci);
+extern int edac_mc_add_mc(struct mem_ctl_info *mci,int mc_idx);
 extern struct mem_ctl_info * edac_mc_del_mc(struct device *dev);
 extern int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci,
 					unsigned long page);
@@ -208,7 +208,10 @@
  
 	i82860_get_error_info(mci, &discard);  /* clear counters */
  
-	if (edac_mc_add_mc(mci)) {
+	/* Here we assume that we will never see multiple instances of this
+	 * type of memory controller.  The ID is therefore hardcoded to 0.
+	 */
+	if (edac_mc_add_mc(mci,0)) {
 		debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
 		edac_mc_free(mci);
 	} else {
@@ -390,7 +390,10 @@
  
 	i82875p_get_error_info(mci, &discard);  /* clear counters */
  
-	if (edac_mc_add_mc(mci)) {
+	/* Here we assume that we will never see multiple instances of this
+	 * type of memory controller.  The ID is therefore hardcoded to 0.
+	 */
+	if (edac_mc_add_mc(mci,0)) {
 		debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
 		goto fail3;
 	}
@@ -304,7 +304,10 @@
  
 	r82600_get_error_info(mci, &discard);  /* clear counters */
  
-	if (edac_mc_add_mc(mci)) {
+	/* Here we assume that we will never see multiple instances of this
+	 * type of memory controller.  The ID is therefore hardcoded to 0.
+	 */
+	if (edac_mc_add_mc(mci,0)) {
 		debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
 		goto fail;
 	}
...	...	@@ -257,7 +257,10 @@
257	257
258	258	amd76x_get_error_info(mci, &discard); /* clear counters */
259	259
260		- if (edac_mc_add_mc(mci)) {
	260	+ /* Here we assume that we will never see multiple instances of this
	261	+ * type of memory controller. The ID is therefore hardcoded to 0.
	262	+ */
	263	+ if (edac_mc_add_mc(mci,0)) {
261	264	debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
262	265	goto fail;
263	266	}
...	...	@@ -953,7 +953,10 @@
953	953	"tolm = %x, remapbase = %x, remaplimit = %x\n", pvt->tolm,
954	954	pvt->remapbase, pvt->remaplimit);
955	955
956		- if (edac_mc_add_mc(mci)) {
	956	+ /* Here we assume that we will never see multiple instances of this
	957	+ * type of memory controller. The ID is therefore hardcoded to 0.
	958	+ */
	959	+ if (edac_mc_add_mc(mci,0)) {
957	960	debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
958	961	goto fail;
959	962	}
...	...	@@ -463,7 +463,10 @@
463	463	/* clear any pending errors, or initial state bits */
464	464	e7xxx_get_error_info(mci, &discard);
465	465
466		- if (edac_mc_add_mc(mci) != 0) {
	466	+ /* Here we assume that we will never see multiple instances of this
	467	+ * type of memory controller. The ID is therefore hardcoded to 0.
	468	+ */
	469	+ if (edac_mc_add_mc(mci,0)) {
467	470	debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
468	471	goto fail;
469	472	}
...	...	@@ -1632,47 +1632,46 @@
1632	1632	return NULL;
1633	1633	}
1634	1634
1635		-static int add_mc_to_global_list(struct mem_ctl_info *mci)
	1635	+/* Return 0 on success, 1 on failure.
	1636	+ * Before calling this function, caller must
	1637	+ * assign a unique value to mci->mc_idx.
	1638	+ */
	1639	+static int add_mc_to_global_list (struct mem_ctl_info *mci)
1636	1640	{
1637	1641	struct list_head item, insert_before;
1638	1642	struct mem_ctl_info *p;
1639		- int i;
1640	1643
1641		- if (list_empty(&mc_devices)) {
1642		- mci->mc_idx = 0;
1643		- insert_before = &mc_devices;
1644		- } else {
1645		- if (find_mci_by_dev(mci->dev)) {
1646		- edac_printk(KERN_WARNING, EDAC_MC,
1647		- "%s (%s) %s %s already assigned %d\n",
1648		- mci->dev->bus_id, dev_name(mci->dev),
1649		- mci->mod_name, mci->ctl_name,
1650		- mci->mc_idx);
1651		- return 1;
1652		- }
	1644	+ insert_before = &mc_devices;
1653	1645
1654		- insert_before = NULL;
1655		- i = 0;
	1646	+ if (unlikely((p = find_mci_by_dev(mci->dev)) != NULL))
	1647	+ goto fail0;
1656	1648
1657		- list_for_each(item, &mc_devices) {
1658		- p = list_entry(item, struct mem_ctl_info, link);
	1649	+ list_for_each(item, &mc_devices) {
	1650	+ p = list_entry(item, struct mem_ctl_info, link);
1659	1651
1660		- if (p->mc_idx != i) {
1661		- insert_before = item;
1662		- break;
1663		- }
	1652	+ if (p->mc_idx >= mci->mc_idx) {
	1653	+ if (unlikely(p->mc_idx == mci->mc_idx))
	1654	+ goto fail1;
1664	1655
1665		- i++;
	1656	+ insert_before = item;
	1657	+ break;
1666	1658	}
1667		-
1668		- mci->mc_idx = i;
1669		-
1670		- if (insert_before == NULL)
1671		- insert_before = &mc_devices;
1672	1659	}
1673	1660
1674	1661	list_add_tail_rcu(&mci->link, insert_before);
1675	1662	return 0;
	1663	+
	1664	+fail0:
	1665	+ edac_printk(KERN_WARNING, EDAC_MC,
	1666	+ "%s (%s) %s %s already assigned %d\n", p->dev->bus_id,
	1667	+ dev_name(p->dev), p->mod_name, p->ctl_name, p->mc_idx);
	1668	+ return 1;
	1669	+
	1670	+fail1:
	1671	+ edac_printk(KERN_WARNING, EDAC_MC,
	1672	+ "bug in low-level driver: attempt to assign\n"
	1673	+ " duplicate mc_idx %d in %s()\n", p->mc_idx, __func__);
	1674	+ return 1;
1676	1675	}
1677	1676
1678	1677	static void complete_mc_list_del(struct rcu_head *head)
...	...	@@ -1696,6 +1695,7 @@
1696	1695	* edac_mc_add_mc: Insert the 'mci' structure into the mci global list and
1697	1696	* create sysfs entries associated with mci structure
1698	1697	* @mci: pointer to the mci structure to be added to the list
	1698	+ * @mc_idx: A unique numeric identifier to be assigned to the 'mci' structure.
1699	1699	*
1700	1700	* Return:
1701	1701	* 0 Success
1702	1702
...	...	@@ -1703,9 +1703,10 @@
1703	1703	*/
1704	1704
1705	1705	/* FIXME - should a warning be printed if no error detection? correction? */
1706		-int edac_mc_add_mc(struct mem_ctl_info *mci)
	1706	+int edac_mc_add_mc(struct mem_ctl_info *mci, int mc_idx)
1707	1707	{
1708	1708	debugf0("%s()\n", __func__);
	1709	+ mci->mc_idx = mc_idx;
1709	1710	#ifdef CONFIG_EDAC_DEBUG
1710	1711	if (edac_debug_level >= 3)
1711	1712	edac_mc_dump_mci(mci);
...	...	@@ -417,7 +417,7 @@
417	417	void edac_mc_dump_csrow(struct csrow_info *csrow);
418	418	#endif /* CONFIG_EDAC_DEBUG */
419	419
420		-extern int edac_mc_add_mc(struct mem_ctl_info *mci);
	420	+extern int edac_mc_add_mc(struct mem_ctl_info *mci,int mc_idx);
421	421	extern struct mem_ctl_info * edac_mc_del_mc(struct device *dev);
422	422	extern int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci,
423	423	unsigned long page);
...	...	@@ -208,7 +208,10 @@
208	208
209	209	i82860_get_error_info(mci, &discard); /* clear counters */
210	210
211		- if (edac_mc_add_mc(mci)) {
	211	+ /* Here we assume that we will never see multiple instances of this
	212	+ * type of memory controller. The ID is therefore hardcoded to 0.
	213	+ */
	214	+ if (edac_mc_add_mc(mci,0)) {
212	215	debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
213	216	edac_mc_free(mci);
214	217	} else {
...	...	@@ -390,7 +390,10 @@
390	390
391	391	i82875p_get_error_info(mci, &discard); /* clear counters */
392	392
393		- if (edac_mc_add_mc(mci)) {
	393	+ /* Here we assume that we will never see multiple instances of this
	394	+ * type of memory controller. The ID is therefore hardcoded to 0.
	395	+ */
	396	+ if (edac_mc_add_mc(mci,0)) {
394	397	debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
395	398	goto fail3;
396	399	}
...	...	@@ -304,7 +304,10 @@
304	304
305	305	r82600_get_error_info(mci, &discard); /* clear counters */
306	306
307		- if (edac_mc_add_mc(mci)) {
	307	+ /* Here we assume that we will never see multiple instances of this
	308	+ * type of memory controller. The ID is therefore hardcoded to 0.
	309	+ */
	310	+ if (edac_mc_add_mc(mci,0)) {
308	311	debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
309	312	goto fail;
310	313	}