[U-Boot-Users] Possible memory leak in cpu/ppc4xx/4xx_enet.c
Hi all,
I've seen an issue with my 1.3.1-based port of U-Boot where a relatively large number of pings issued from the command line will ultimately fail with the message:
Cannot allocate private hw data for eth_device...
I looked around this message and it seems that ppc_4xx_eth_initialize() malloc()’s a EMAC_4XX_HW_PST structure and assigns it to dev->priv. However, nothing ever frees this buffer so standing on a ping will ultimately pull down all the available memory.
One fix might be to free( dev->priv ) at the end of ppc_4xx_eth_halt(). I tried this today and noted no ill effects.
Thanks, Dave
Dave,
On Saturday 15 March 2008, Dave Littell wrote:
I've seen an issue with my 1.3.1-based port of U-Boot where a relatively large number of pings issued from the command line will ultimately fail with the message:
Cannot allocate private hw data for eth_device...
I looked around this message and it seems that ppc_4xx_eth_initialize() malloc()’s a EMAC_4XX_HW_PST structure and assigns it to dev->priv. However, nothing ever frees this buffer so standing on a ping will ultimately pull down all the available memory.
Hmmm. IIRC ppc_4xx_eth_initialize() is only called once upon bootup for ethernet driver initialization. The function called each time before network action is ppc_4xx_eth_init().
One fix might be to free( dev->priv ) at the end of ppc_4xx_eth_halt(). I tried this today and noted no ill effects.
So did it work for you?
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Stefan Roese wrote:
Dave,
On Saturday 15 March 2008, Dave Littell wrote:
I've seen an issue with my 1.3.1-based port of U-Boot where a relatively large number of pings issued from the command line will ultimately fail with the message:
Cannot allocate private hw data for eth_device...
I looked around this message and it seems that ppc_4xx_eth_initialize() malloc()’s a EMAC_4XX_HW_PST structure and assigns it to dev->priv. However, nothing ever frees this buffer so standing on a ping will ultimately pull down all the available memory.
Hmmm. IIRC ppc_4xx_eth_initialize() is only called once upon bootup for ethernet driver initialization. The function called each time before network action is ppc_4xx_eth_init().
It looks like ppc_4xx_eth_initialize() is called from eth_init() and ppc_4xx_eth_init() is only called if there's an emac0_dev.
Hm?
One fix might be to free( dev->priv ) at the end of ppc_4xx_eth_halt(). I tried this today and noted no ill effects.
So did it work for you?
It seems to be working well.
Thanks, Dave
On Saturday 15 March 2008, Dave Littell wrote:
I looked around this message and it seems that ppc_4xx_eth_initialize() malloc()’s a EMAC_4XX_HW_PST structure and assigns it to dev->priv. However, nothing ever frees this buffer so standing on a ping will ultimately pull down all the available memory.
Hmmm. IIRC ppc_4xx_eth_initialize() is only called once upon bootup for ethernet driver initialization. The function called each time before network action is ppc_4xx_eth_init().
It looks like ppc_4xx_eth_initialize() is called from eth_init() and ppc_4xx_eth_init() is only called if there's an emac0_dev.
Hm?
I see. You don't have CONFIG_NET_MULTI defined. Which 4xx platform are we talking about btw?
One fix might be to free( dev->priv ) at the end of ppc_4xx_eth_halt(). I tried this today and noted no ill effects.
So did it work for you?
It seems to be working well.
OK. But your patch will most likely break all platforms with CONFIG_NET_MULTI defined. Please give the attached patch a try and let me know if it works too. I'll push it online if it fixes the problem.
diff --git a/cpu/ppc4xx/4xx_enet.c b/cpu/ppc4xx/4xx_enet.c index d990250..479cb40 100644 --- a/cpu/ppc4xx/4xx_enet.c +++ b/cpu/ppc4xx/4xx_enet.c @@ -1998,9 +1998,12 @@ int ppc_4xx_eth_initialize (bd_t * bis) }
#if !defined(CONFIG_NET_MULTI) -void eth_halt (void) { +void eth_halt (void) +{ if (emac0_dev) { ppc_4xx_eth_halt(emac0_dev); + if (emac0_dev->priv) + free(emac0_dev->priv); free(emac0_dev); emac0_dev = NULL; }
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Stefan Roese wrote:
On Saturday 15 March 2008, Dave Littell wrote:
I looked around this message and it seems that ppc_4xx_eth_initialize() malloc()’s a EMAC_4XX_HW_PST structure and assigns it to dev->priv. However, nothing ever frees this buffer so standing on a ping will ultimately pull down all the available memory.
Hmmm. IIRC ppc_4xx_eth_initialize() is only called once upon bootup for ethernet driver initialization. The function called each time before network action is ppc_4xx_eth_init().
It looks like ppc_4xx_eth_initialize() is called from eth_init() and ppc_4xx_eth_init() is only called if there's an emac0_dev.
Hm?
I see. You don't have CONFIG_NET_MULTI defined. Which 4xx platform are we talking about btw?
This is a platform similar to the AMCC sequoia, but CONFIG_NET_MULTI is not defined.
One fix might be to free( dev->priv ) at the end of ppc_4xx_eth_halt(). I tried this today and noted no ill effects.
So did it work for you?
It seems to be working well.
OK. But your patch will most likely break all platforms with CONFIG_NET_MULTI defined. Please give the attached patch a try and let me know if it works too. I'll push it online if it fixes the problem.
It does indeed seem to work - thanks!
diff --git a/cpu/ppc4xx/4xx_enet.c b/cpu/ppc4xx/4xx_enet.c index d990250..479cb40 100644 --- a/cpu/ppc4xx/4xx_enet.c +++ b/cpu/ppc4xx/4xx_enet.c @@ -1998,9 +1998,12 @@ int ppc_4xx_eth_initialize (bd_t * bis) }
#if !defined(CONFIG_NET_MULTI) -void eth_halt (void) { +void eth_halt (void) +{ if (emac0_dev) { ppc_4xx_eth_halt(emac0_dev);
if (emac0_dev->priv)free(emac0_dev->priv); free(emac0_dev); emac0_dev = NULL; }
Thanks, Dave
participants (2)
-
Dave Littell -
Stefan Roese