* [PATCH 1/3] net: dsa: Use devm_ prefixed allocations
@ 2015-10-02 10:48 Neil Armstrong
2015-10-02 13:25 ` Felix Fietkau
2015-10-02 13:29 ` Sergei Shtylyov
0 siblings, 2 replies; 7+ messages in thread
From: Neil Armstrong @ 2015-10-02 10:48 UTC (permalink / raw)
To: David S. Miller, Jesper Dangaard Brouer; +Cc: netdev, linux-kernel
To simplify and prevent memory leakage when unbinding, use
the devm_ memory allocation calls.
Tested-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] net: dsa: Use devm_ prefixed allocations
2015-10-02 10:48 [PATCH 1/3] net: dsa: Use devm_ prefixed allocations Neil Armstrong
@ 2015-10-02 13:25 ` Felix Fietkau
2015-10-02 13:29 ` Sergei Shtylyov
1 sibling, 0 replies; 7+ messages in thread
From: Felix Fietkau @ 2015-10-02 13:25 UTC (permalink / raw)
To: Neil Armstrong, David S. Miller, Jesper Dangaard Brouer
Cc: netdev, linux-kernel
On 2015-10-02 12:48, Neil Armstrong wrote:
> To simplify and prevent memory leakage when unbinding, use
> the devm_ memory allocation calls.
>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
I think you also need to get rid of the corresponding free calls in the
error path, otherwise it will probably crash at some point.
- Felix
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] net: dsa: Use devm_ prefixed allocations
2015-10-02 10:48 [PATCH 1/3] net: dsa: Use devm_ prefixed allocations Neil Armstrong
2015-10-02 13:25 ` Felix Fietkau
@ 2015-10-02 13:29 ` Sergei Shtylyov
2015-10-02 13:30 ` Neil Armstrong
1 sibling, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2015-10-02 13:29 UTC (permalink / raw)
To: Neil Armstrong, David S. Miller, Jesper Dangaard Brouer
Cc: netdev, linux-kernel
On 10/2/2015 1:48 PM, Neil Armstrong wrote:
> To simplify and prevent memory leakage when unbinding, use
> the devm_ memory allocation calls.
>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
> 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;
>
Shouldn't you remove the correspoding kfree(), etc. calls?
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] net: dsa: Use devm_ prefixed allocations
2015-10-02 13:29 ` Sergei Shtylyov
@ 2015-10-02 13:30 ` Neil Armstrong
2015-10-02 13:37 ` Sergei Shtylyov
2015-10-03 12:39 ` Felix Fietkau
0 siblings, 2 replies; 7+ messages in thread
From: Neil Armstrong @ 2015-10-02 13:30 UTC (permalink / raw)
To: Sergei Shtylyov, David S. Miller, Jesper Dangaard Brouer
Cc: netdev, linux-kernel
On 10/02/2015 03:29 PM, Sergei Shtylyov wrote:
> On 10/2/2015 1:48 PM, Neil Armstrong wrote:
>
>> To simplify and prevent memory leakage when unbinding, use
>> the devm_ memory allocation calls.
>>
>> Tested-by: Andrew Lunn <andrew@lunn.ch>
>> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
>> 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;
>>
>
> Shouldn't you remove the correspoding kfree(), etc. calls?
>
> MBR, Sergei
>
The corresponding kfree() calls were all missing.
Neil
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] net: dsa: Use devm_ prefixed allocations
2015-10-02 13:30 ` Neil Armstrong
@ 2015-10-02 13:37 ` Sergei Shtylyov
2015-10-03 12:39 ` Felix Fietkau
1 sibling, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2015-10-02 13:37 UTC (permalink / raw)
To: Neil Armstrong, David S. Miller, Jesper Dangaard Brouer
Cc: netdev, linux-kernel
On 10/2/2015 4:30 PM, Neil Armstrong wrote:
>>> To simplify and prevent memory leakage when unbinding, use
>>> the devm_ memory allocation calls.
>>>
>>> Tested-by: Andrew Lunn <andrew@lunn.ch>
>>> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
>>> 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;
>>>
>>
>> Shouldn't you remove the correspoding kfree(), etc. calls?
>>
>> MBR, Sergei
>>
> The corresponding kfree() calls were all missing.
Then this patch should be for net, not net-next. Either that, or add the
kfree() calls first, then remove them in this net-next patch.
> Neil
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] net: dsa: Use devm_ prefixed allocations
2015-10-02 13:30 ` Neil Armstrong
2015-10-02 13:37 ` Sergei Shtylyov
@ 2015-10-03 12:39 ` Felix Fietkau
2015-10-03 13:55 ` Neil Armstrong
1 sibling, 1 reply; 7+ messages in thread
From: Felix Fietkau @ 2015-10-03 12:39 UTC (permalink / raw)
To: Neil Armstrong, Sergei Shtylyov, David S. Miller,
Jesper Dangaard Brouer
Cc: netdev, linux-kernel
On 2015-10-02 15:30, Neil Armstrong wrote:
> On 10/02/2015 03:29 PM, Sergei Shtylyov wrote:
>> On 10/2/2015 1:48 PM, Neil Armstrong wrote:
>>
>>> To simplify and prevent memory leakage when unbinding, use
>>> the devm_ memory allocation calls.
>>>
>>> Tested-by: Andrew Lunn <andrew@lunn.ch>
>>> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
>>> 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;
>>>
>>
>> Shouldn't you remove the correspoding kfree(), etc. calls?
>>
>> MBR, Sergei
>>
> The corresponding kfree() calls were all missing.
Not in the error handling path. mdiobus_alloc has a corresponding
mdiobus_free in the same function.
The ds kzalloc in dsa_switch_setup has a kfree in dsa_switch_setup_one.
- Felix
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] net: dsa: Use devm_ prefixed allocations
2015-10-03 12:39 ` Felix Fietkau
@ 2015-10-03 13:55 ` Neil Armstrong
0 siblings, 0 replies; 7+ messages in thread
From: Neil Armstrong @ 2015-10-03 13:55 UTC (permalink / raw)
To: Felix Fietkau, Sergei Shtylyov, David S. Miller; +Cc: netdev, linux-kernel
On 10/03/2015 02:39 PM, Felix Fietkau wrote:
> On 2015-10-02 15:30, Neil Armstrong wrote:
>> On 10/02/2015 03:29 PM, Sergei Shtylyov wrote:
>>> On 10/2/2015 1:48 PM, Neil Armstrong wrote:
>>>
>>>> To simplify and prevent memory leakage when unbinding, use
>>>> the devm_ memory allocation calls.
>>>>
>>>> Tested-by: Andrew Lunn <andrew@lunn.ch>
>>>> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> 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;
>>>>
>>>
>>> Shouldn't you remove the correspoding kfree(), etc. calls?
>>>
>>> MBR, Sergei
>>>
>> The corresponding kfree() calls were all missing.
> Not in the error handling path. mdiobus_alloc has a corresponding
> mdiobus_free in the same function.
> The ds kzalloc in dsa_switch_setup has a kfree in dsa_switch_setup_one.
>
> - Felix
>
Thanks Felix & Sergei,
I will repost based on net with an intermediate commit adding the missing kfree calls.
Neil
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-03 13:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-02 10:48 [PATCH 1/3] net: dsa: Use devm_ prefixed allocations Neil Armstrong
2015-10-02 13:25 ` Felix Fietkau
2015-10-02 13:29 ` Sergei Shtylyov
2015-10-02 13:30 ` Neil Armstrong
2015-10-02 13:37 ` Sergei Shtylyov
2015-10-03 12:39 ` Felix Fietkau
2015-10-03 13:55 ` Neil Armstrong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).