Re: [PATCH 4/5] EDAC/versalnet: Fix device_register() error handling in init_one_mc()

[Borislav Petkov]

On Mon, Mar 23, 2026 at 12:38:57PM +0530, Prasanna Kumar T S M wrote:
> If kzalloc(dev) is done after edac_mc_alloc(), there is no need to decide
> between kfree(dev) or put_device(dev). This simplifies the error handling
> path. This is the reason behind re-ordering and keeping put_device(dev)
> under 'if (rc) { ... }'.

I don't think you're listening to me so lemme repeat:

edac_mc_alloc() is a lot more heavy-weight than a simple k*alloc(). Pls keep
the ordering as it is.

I don't care how much it simplifies the error handling path if you have to do
all the allocations and setup edac_mc_alloc() does for *nothing*!

So let's do it another way (totally untested ofc):

---
diff --git a/drivers/edac/versalnet_edac.c b/drivers/edac/versalnet_edac.c
index b87fe57aa842..bf17b3ff59d5 100644
--- a/drivers/edac/versalnet_edac.c
+++ b/drivers/edac/versalnet_edac.c
@@ -772,12 +772,11 @@ static void remove_one_mc(struct mc_priv *priv, int i)
 	edac_mc_free(mci);
 }
 
-static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i)
+static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, struct device *dev, int i)
 {
 	u32 num_chans, rank, dwidth, config;
 	struct edac_mc_layer layers[2];
 	struct mem_ctl_info *mci;
-	struct device *dev;
 	enum dev_type dt;
 	char *name;
 	int rc;
@@ -817,14 +816,10 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	if (!name)
 		return rc;
 
-	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-	if (!dev)
-		goto err_name_free;
-
 	mci = edac_mc_alloc(i, ARRAY_SIZE(layers), layers, sizeof(struct mc_priv));
 	if (!mci) {
 		edac_printk(KERN_ERR, EDAC_MC, "Failed memory allocation for MC%d\n", i);
-		goto err_dev_free;
+		goto err_name_free;
 	}
 
 	sprintf(name, "versal-net-ddrmc5-edac-%d", i);
@@ -856,8 +851,6 @@ static int init_one_mc(struct mc_priv *priv, struct platform_device *pdev, int i
 	device_unregister(mci->pdev);
 err_mc_free:
 	edac_mc_free(mci);
-err_dev_free:
-	kfree(dev);
 err_name_free:
 	kfree(name);
 
@@ -869,15 +862,21 @@ static int init_versalnet(struct mc_priv *priv, struct platform_device *pdev)
 	int rc, i;
 
 	for (i = 0; i < NUM_CONTROLLERS; i++) {
-		rc = init_one_mc(priv, pdev, i);
-		if (rc) {
-			while (i--)
-				remove_one_mc(priv, i);
+		struct device *dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+		if (!dev)
+			goto free;
 
-			return rc;
-		}
+		rc = init_one_mc(priv, pdev, dev, i);
+		if (rc)
+			goto free;
 	}
 	return 0;
+
+free:
+	while (i--)
+		remove_one_mc(priv, i);
+
+	return rc;
 }
 
 static void remove_versalnet(struct mc_priv *priv)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette