Skip to contentSkip to conversation

net: dsa: Use devm_ prefixed allocations

RFC PATCH In review Neil Armstrong 4 participants 5 messages Sep 30, 2015

Subsystems (3)
Networking Network drivers Networking DSA
lore
Share & tools
lore

Neil Armstrong

author
#0 lore raw

RFC PATCH 1/3 net: dsa: Use devm_ prefixed allocations

To simplify and prevent memory leakage when unbinding, use the devm_ memory allocation calls.

Signed-off-by Neil Armstrong <[email protected]>
---
 net/dsa/dsa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/dsa/dsa.c +3 -3
index c59fa5d..98f94c2 100644
@@ -305,7 +305,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 	if (ret < 0)
 		goto out;
 
-	ds->slave_mii_bus = mdiobus_alloc();
+	ds->slave_mii_bus = devm_mdiobus_alloc(parent);
 	if (ds->slave_mii_bus == NULL) {
 		ret = -ENOMEM;
 		goto out;

@@ -400,7 +400,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	/*
 	 * Allocate and initialise switch state.
 	 */
-	ds = kzalloc(sizeof(*ds) + drv->priv_size, GFP_KERNEL);
+	ds = devm_kzalloc(parent, sizeof(*ds) + drv->priv_size, GFP_KERNEL);
 	if (ds == NULL)
 		return ERR_PTR(-ENOMEM);
 

@@ -883,7 +883,7 @@ static int dsa_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	dst = kzalloc(sizeof(*dst), GFP_KERNEL);
+	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
 	if (dst == NULL) {
 		dev_put(dev);
 		ret = -ENOMEM;
Signature · 1 line
1.9.1
maintainer Tested-by on v1 1/3
#1 lore raw

Hi Neil

I tested all three patches on a board with three switches.

1) Normal boot
2) Bad address set for the 3rd switch so that it was not found, so causing the probe to fail.

No regressions observed.

Tested-by Andrew Lunn <[email protected]>

As Florian said, this is going in the right direction for modular DSA, but still quite a way to go...

Thanks
Andrew

Quoted 45 lines · Neil Armstrong

To simplify and prevent memory leakage when unbinding, use the devm_ memory allocation calls.

Signed-off-by Neil Armstrong <[email protected]>
---
 net/dsa/dsa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/dsa/dsa.c +3 -3
index c59fa5d..98f94c2 100644
@@ -305,7 +305,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 	if (ret < 0)
 		goto out;
 
-	ds->slave_mii_bus = mdiobus_alloc();
+	ds->slave_mii_bus = devm_mdiobus_alloc(parent);
 	if (ds->slave_mii_bus == NULL) {
 		ret = -ENOMEM;
 		goto out;

@@ -400,7 +400,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	/*
 	 * Allocate and initialise switch state.
 	 */
-	ds = kzalloc(sizeof(*ds) + drv->priv_size, GFP_KERNEL);
+	ds = devm_kzalloc(parent, sizeof(*ds) + drv->priv_size, GFP_KERNEL);
 	if (ds == NULL)
 		return ERR_PTR(-ENOMEM);
 

@@ -883,7 +883,7 @@ static int dsa_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	dst = kzalloc(sizeof(*dst), GFP_KERNEL);
+	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
 	if (dst == NULL) {
 		dev_put(dev);
 		ret = -ENOMEM;
Signature · 5 lines
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#2 lore raw
In reply to Neil Armstrong
Quoted 22 lines · Neil Armstrong

To simplify and prevent memory leakage when unbinding, use the devm_ memory allocation calls.

Signed-off-by Neil Armstrong <[email protected]>
---
 net/dsa/dsa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/dsa/dsa.c +1 -1
index c59fa5d..98f94c2 100644
@@ -305,7 +305,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
        if (ret < 0)
                goto out;
 
-       ds->slave_mii_bus = mdiobus_alloc();
+       ds->slave_mii_bus = devm_mdiobus_alloc(parent);
        if (ds->slave_mii_bus == NULL) {
                ret = -ENOMEM;
                goto out;

net/dsa/dsa.c dsa_switch_setup(struct dsa_switch_tree *dst, int index, · line 403

Quoted 5 lines
net/dsa/dsa.c +1 -1
@@ -400,7 +400,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
        /*
         * Allocate and initialise switch state.
         */
-       ds = kzalloc(sizeof(*ds) + drv->priv_size, GFP_KERNEL);
+       ds = devm_kzalloc(parent, sizeof(*ds) + drv->priv_size, GFP_KERNEL);

Shouldn't devm_ functions be called only from probe and remove functions?

#3 lore raw
Quoted 17 lines · Fabio Estevam
Quoted 16 lines · Neil Armstrong

To simplify and prevent memory leakage when unbinding, use the devm_ memory allocation calls.

Signed-off-by Neil Armstrong <[email protected]>
---
 net/dsa/dsa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/dsa/dsa.c +0 -0
index c59fa5d..98f94c2 100644
@@ -305,7 +305,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
        if (ret < 0)
                goto out;
Quoted 13 lines

net/dsa/dsa.c dsa_switch_setup(struct dsa_switch_tree *dst, int index, · line 403

Quoted 11 lines
net/dsa/dsa.c +2 -2
-       ds->slave_mii_bus = mdiobus_alloc();
+       ds->slave_mii_bus = devm_mdiobus_alloc(parent);
        if (ds->slave_mii_bus == NULL) {
                ret = -ENOMEM;
                goto out;

@@ -400,7 +400,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
        /*
         * Allocate and initialise switch state.
         */
-       ds = kzalloc(sizeof(*ds) + drv->priv_size, GFP_KERNEL);
+       ds = devm_kzalloc(parent, sizeof(*ds) + drv->priv_size, GFP_KERNEL);

Shouldn't devm_ functions be called only from probe and remove functions?

That's the case AFAICT, the call chain looks like this:

dsa_probe()

	-> dsa_setup_dst()
		-> dsa_switch_setup()
			-> dsa_switch_setup_one()
Signature · 1 line
Florian