[PATCH] EDAC/amd64: Stop allocating ECC settings separately

[PATCH] EDAC/amd64: Stop allocating ECC settings separately

[Dmitry Torokhov]

There is no reason to allocate and manage ECC settings separately when
struct ecc_settings can be embedded directly into the driver-private
struct amd64_pvt.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Resending this standalone cleanup that was originally posted back in
2015 as part of an async-probe RFC series [1] and got lost in the
broader driver model discussion. Since amd64_edac has long moved away
from pci_register_driver() to probing nodes directly in
amd64_edac_init(), patches 2 and 3 from that series are obsolete, and
this patch has been rebased onto the current code.

[1] https://lore.kernel.org/all/1426726150-983-2-git-send-email-dmitry.torokhov@gmail.com/


 drivers/edac/amd64_edac.c | 40 ++++++---------------------------------
 drivers/edac/amd64_edac.h | 27 ++++++++++++++------------
 2 files changed, 21 insertions(+), 46 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 475235c402e8..6931b7e61652 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -30,9 +30,6 @@ static inline u32 get_umc_reg(struct amd64_pvt *pvt, u32 reg)
 	return 0;
 }
 
-/* Per-node stuff */
-static struct ecc_settings **ecc_stngs;
-
 /* Device for the PCI component */
 static struct device *pci_ctl_dev;
 
@@ -3999,19 +3996,12 @@ static int probe_one_instance(unsigned int nid)
 {
 	struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
 	struct amd64_pvt *pvt = NULL;
-	struct ecc_settings *s;
 	int ret;
 
 	ret = -ENOMEM;
-	s = kzalloc_obj(struct ecc_settings);
-	if (!s)
-		goto err_out;
-
-	ecc_stngs[nid] = s;
-
 	pvt = kzalloc_obj(struct amd64_pvt);
 	if (!pvt)
-		goto err_settings;
+		goto err_out;
 
 	pvt->mc_node_id	= nid;
 	pvt->F3 = F3;
@@ -4042,7 +4032,7 @@ static int probe_one_instance(unsigned int nid)
 		} else
 			amd64_warn("Forcing ECC on!\n");
 
-		if (!enable_ecc_error_reporting(s, nid, F3))
+		if (!enable_ecc_error_reporting(&pvt->ecc, nid, F3))
 			goto err_enable;
 	}
 
@@ -4051,7 +4041,7 @@ static int probe_one_instance(unsigned int nid)
 		amd64_err("Error probing instance: %d\n", nid);
 
 		if (boot_cpu_data.x86 < 0x17)
-			restore_ecc_error_reporting(s, nid, F3);
+			restore_ecc_error_reporting(&pvt->ecc, nid, F3);
 
 		goto err_enable;
 	}
@@ -4067,10 +4057,6 @@ static int probe_one_instance(unsigned int nid)
 	hw_info_put(pvt);
 	kfree(pvt);
 
-err_settings:
-	kfree(s);
-	ecc_stngs[nid] = NULL;
-
 err_out:
 	return ret;
 }
@@ -4078,7 +4064,6 @@ static int probe_one_instance(unsigned int nid)
 static void remove_one_instance(unsigned int nid)
 {
 	struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
-	struct ecc_settings *s = ecc_stngs[nid];
 	struct mem_ctl_info *mci;
 	struct amd64_pvt *pvt;
 
@@ -4089,10 +4074,7 @@ static void remove_one_instance(unsigned int nid)
 
 	pvt = mci->pvt_info;
 
-	restore_ecc_error_reporting(s, nid, F3);
-
-	kfree(ecc_stngs[nid]);
-	ecc_stngs[nid] = NULL;
+	restore_ecc_error_reporting(&pvt->ecc, nid, F3);
 
 	/* Free the EDAC CORE resources */
 	mci->pvt_info = NULL;
@@ -4149,13 +4131,10 @@ static int __init amd64_edac_init(void)
 	opstate_init();
 
 	err = -ENOMEM;
-	ecc_stngs = kzalloc_objs(ecc_stngs[0], amd_nb_num());
-	if (!ecc_stngs)
-		goto err_free;
 
 	msrs = msrs_alloc();
 	if (!msrs)
-		goto err_free;
+		goto err_ret;
 
 	for (i = 0; i < amd_nb_num(); i++) {
 		err = probe_one_instance(i);
@@ -4195,10 +4174,7 @@ static int __init amd64_edac_init(void)
 	msrs_free(msrs);
 	msrs = NULL;
 
-err_free:
-	kfree(ecc_stngs);
-	ecc_stngs = NULL;
-
+err_ret:
 	return err;
 }
 
@@ -4218,11 +4194,7 @@ static void __exit amd64_edac_exit(void)
 	for (i = 0; i < amd_nb_num(); i++)
 		remove_one_instance(i);
 
-	kfree(ecc_stngs);
-	ecc_stngs = NULL;
-
 	pci_ctl_dev = NULL;
-
 	msrs_free(msrs);
 	msrs = NULL;
 }
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 1757c1b99fc8..a93e0214143c 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -322,6 +322,19 @@ struct amd64_family_flags {
 	      __reserved	: 63;
 };
 
+/*
+ * per-node ECC settings descriptor
+ */
+struct ecc_settings {
+	u32 old_nbctl;
+	bool nbctl_valid;
+
+	struct flags {
+		unsigned long nb_mce_enable:1;
+		unsigned long nb_ecc_prev:1;
+	} flags;
+};
+
 struct amd64_pvt {
 	struct low_ops *ops;
 
@@ -372,6 +385,8 @@ struct amd64_pvt {
 	/* place to store error injection parameters prior to issue */
 	struct error_injection injection;
 
+	struct ecc_settings ecc;
+
 	/*
 	 * cache the dram_type
 	 *
@@ -441,18 +456,6 @@ static inline u8 dct_sel_interleave_addr(struct amd64_pvt *pvt)
 
 	return	((pvt)->dct_sel_lo >> 6) & 0x3;
 }
-/*
- * per-node ECC settings descriptor
- */
-struct ecc_settings {
-	u32 old_nbctl;
-	bool nbctl_valid;
-
-	struct flags {
-		unsigned long nb_mce_enable:1;
-		unsigned long nb_ecc_prev:1;
-	} flags;
-};
 
 /*
  * Each of the PCI Device IDs types have their own set of hardware accessor
-- 
2.55.0.1082.g2b9226bbc0-goog


-- 
Dmitry