Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1235887 > unrolled thread

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

Started byNeil Armstrong <narmstrong@baylibre.com>
First post2015-09-30 10:30 +0200
Last post2015-09-30 23:50 +0200
Articles 5 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 1/3] net: dsa: Use devm_ prefixed allocations Neil Armstrong <narmstrong@baylibre.com> - 2015-09-30 10:30 +0200
    Re: [RFC PATCH 1/3] net: dsa: Use devm_ prefixed allocations Andrew Lunn <andrew@lunn.ch> - 2015-09-30 23:30 +0200
    Re: [RFC PATCH 1/3] net: dsa: Use devm_ prefixed allocations Fabio Estevam <festevam@gmail.com> - 2015-09-30 23:40 +0200
      Re: [RFC PATCH 1/3] net: dsa: Use devm_ prefixed allocations Florian Fainelli <f.fainelli@gmail.com> - 2015-09-30 23:50 +0200
    Re: [RFC PATCH 1/3] net: dsa: Use devm_ prefixed allocations Florian Fainelli <f.fainelli@gmail.com> - 2015-09-30 23:50 +0200

#1235887 — [RFC PATCH 1/3] net: dsa: Use devm_ prefixed allocations

FromNeil Armstrong <narmstrong@baylibre.com>
Date2015-09-30 10:30 +0200
Subject[RFC PATCH 1/3] net: dsa: Use devm_ prefixed allocations
Message-ID<qekVZ-7TY-23@gated-at.bofh.it>
To simplify and prevent memory leakage when unbinding, use
the devm_ memory allocation calls.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 net/dsa/dsa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index c59fa5d..98f94c2 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -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;
-- 
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1236775

FromAndrew Lunn <andrew@lunn.ch>
Date2015-09-30 23:30 +0200
Message-ID<qex6O-aB-17@gated-at.bofh.it>
In reply to#1235887
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 <andrew@lunn.ch>

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

    Thanks
	Andrew

On Wed, Sep 30, 2015 at 10:21:08AM +0200, Neil Armstrong wrote:
> To simplify and prevent memory leakage when unbinding, use
> the devm_ memory allocation calls.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  net/dsa/dsa.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index c59fa5d..98f94c2 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -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;
> -- 
> 1.9.1
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1236783

FromFabio Estevam <festevam@gmail.com>
Date2015-09-30 23:40 +0200
Message-ID<qexgu-oR-19@gated-at.bofh.it>
In reply to#1235887
On Wed, Sep 30, 2015 at 5:21 AM, Neil Armstrong <narmstrong@baylibre.com> wrote:
> To simplify and prevent memory leakage when unbinding, use
> the devm_ memory allocation calls.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  net/dsa/dsa.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index c59fa5d..98f94c2 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -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);

Shouldn't devm_ functions be called only from probe and remove functions?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1236791

FromFlorian Fainelli <f.fainelli@gmail.com>
Date2015-09-30 23:50 +0200
Message-ID<qexqa-AE-3@gated-at.bofh.it>
In reply to#1236783
On 30/09/15 14:34, Fabio Estevam wrote:
> On Wed, Sep 30, 2015 at 5:21 AM, Neil Armstrong <narmstrong@baylibre.com> wrote:
>> To simplify and prevent memory leakage when unbinding, use
>> the devm_ memory allocation calls.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  net/dsa/dsa.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index c59fa5d..98f94c2 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -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);
> 
> 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()
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1236792

FromFlorian Fainelli <f.fainelli@gmail.com>
Date2015-09-30 23:50 +0200
Message-ID<qexqa-AE-5@gated-at.bofh.it>
In reply to#1235887
On 30/09/15 01:21, Neil Armstrong wrote:
> To simplify and prevent memory leakage when unbinding, use
> the devm_ memory allocation calls.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

For this entire series, on BCM7445 (using bcm_sf2)

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks!
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web