All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@ti.com>
To: Aaro Koskinen <aaro.koskinen@nokia.com>
Cc: tony@atomide.com, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, vishwanath.bs@ti.com
Subject: Re: [PATCHv2] arm: mach-omap2: smartreflex: fix another memory leak
Date: Mon, 14 Feb 2011 14:38:42 -0800	[thread overview]
Message-ID: <87bp2ez1fx.fsf@ti.com> (raw)
In-Reply-To: <1297087684-19967-1-git-send-email-aaro.koskinen@nokia.com> (Aaro Koskinen's message of "Mon, 7 Feb 2011 16:08:04 +0200")

Aaro Koskinen <aaro.koskinen@nokia.com> writes:

> Temporary strings with volt_* file names should be released after the
> debugfs entries are created. While at it, also simplify the string
> allocation, and use just snprintf() to create the name.
>
> The patch eliminates kmemleak reports with the following stack trace
> (multiple objects depending on HW):
>
> unreferenced object 0xcedbc5a0 (size 64):
>   comm "swapper", pid 1, jiffies 4294929375 (age 423.734s)
>   hex dump (first 32 bytes):
>     76 6f 6c 74 5f 39 37 35 30 30 30 00 00 00 00 00  volt_975000.....
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<c012fee0>] create_object+0x104/0x208
>     [<c012dbc8>] kmem_cache_alloc_trace+0xf0/0x17c
>     [<c0013f64>] omap_sr_probe+0x314/0x420
>     [<c02a1724>] platform_drv_probe+0x18/0x1c
>     [<c02a088c>] driver_probe_device+0xc8/0x188
>     [<c02a09b4>] __driver_attach+0x68/0x8c
>     [<c02a00ac>] bus_for_each_dev+0x44/0x74
>     [<c029f9e0>] bus_add_driver+0xa0/0x228
>     [<c02a0cac>] driver_register+0xa8/0x130
>     [<c02a1b2c>] platform_driver_probe+0x18/0x8c
>     [<c0013c1c>] sr_init+0x40/0x74
>     [<c005a554>] do_one_initcall+0xc8/0x1a0
>     [<c00084f4>] kernel_init+0x150/0x218
>     [<c0065d64>] kernel_thread_exit+0x0/0x8
>     [<ffffffff>] 0xffffffff
>
> Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
> ---
>
> v2: Get rid of kmalloc().

Replied to the wrong patch earlier.

Just to be clear, I'm queuing v2 for the 2.6.38-rc cycle.

Kevin

>  arch/arm/mach-omap2/smartreflex.c |   15 +++------------
>  1 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
> index 77ecebf..e54db84 100644
> --- a/arch/arm/mach-omap2/smartreflex.c
> +++ b/arch/arm/mach-omap2/smartreflex.c
> @@ -927,19 +927,10 @@ static int __init omap_sr_probe(struct platform_device *pdev)
>  	}
>  
>  	for (i = 0; i < sr_info->nvalue_count; i++) {
> -		char *name;
> -		char volt_name[32];
> -
> -		name = kzalloc(NVALUE_NAME_LEN + 1, GFP_KERNEL);
> -		if (!name) {
> -			dev_err(&pdev->dev, "%s: Unable to allocate memory"
> -				" for n-value directory name\n",  __func__);
> -			return -ENOMEM;
> -		}
> +		char name[NVALUE_NAME_LEN + 1];
>  
> -		strcpy(name, "volt_");
> -		sprintf(volt_name, "%d", volt_data[i].volt_nominal);
> -		strcat(name, volt_name);
> +		snprintf(name, sizeof(name), "volt_%d",
> +			 volt_data[i].volt_nominal);
>  		(void) debugfs_create_x32(name, S_IRUGO | S_IWUGO, nvalue_dir,
>  				&(sr_info->nvalue_table[i].nvalue));
>  	}

WARNING: multiple messages have this Message-ID (diff)
From: khilman@ti.com (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2] arm: mach-omap2: smartreflex: fix another memory leak
Date: Mon, 14 Feb 2011 14:38:42 -0800	[thread overview]
Message-ID: <87bp2ez1fx.fsf@ti.com> (raw)
In-Reply-To: <1297087684-19967-1-git-send-email-aaro.koskinen@nokia.com> (Aaro Koskinen's message of "Mon, 7 Feb 2011 16:08:04 +0200")

Aaro Koskinen <aaro.koskinen@nokia.com> writes:

> Temporary strings with volt_* file names should be released after the
> debugfs entries are created. While at it, also simplify the string
> allocation, and use just snprintf() to create the name.
>
> The patch eliminates kmemleak reports with the following stack trace
> (multiple objects depending on HW):
>
> unreferenced object 0xcedbc5a0 (size 64):
>   comm "swapper", pid 1, jiffies 4294929375 (age 423.734s)
>   hex dump (first 32 bytes):
>     76 6f 6c 74 5f 39 37 35 30 30 30 00 00 00 00 00  volt_975000.....
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<c012fee0>] create_object+0x104/0x208
>     [<c012dbc8>] kmem_cache_alloc_trace+0xf0/0x17c
>     [<c0013f64>] omap_sr_probe+0x314/0x420
>     [<c02a1724>] platform_drv_probe+0x18/0x1c
>     [<c02a088c>] driver_probe_device+0xc8/0x188
>     [<c02a09b4>] __driver_attach+0x68/0x8c
>     [<c02a00ac>] bus_for_each_dev+0x44/0x74
>     [<c029f9e0>] bus_add_driver+0xa0/0x228
>     [<c02a0cac>] driver_register+0xa8/0x130
>     [<c02a1b2c>] platform_driver_probe+0x18/0x8c
>     [<c0013c1c>] sr_init+0x40/0x74
>     [<c005a554>] do_one_initcall+0xc8/0x1a0
>     [<c00084f4>] kernel_init+0x150/0x218
>     [<c0065d64>] kernel_thread_exit+0x0/0x8
>     [<ffffffff>] 0xffffffff
>
> Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
> ---
>
> v2: Get rid of kmalloc().

Replied to the wrong patch earlier.

Just to be clear, I'm queuing v2 for the 2.6.38-rc cycle.

Kevin

>  arch/arm/mach-omap2/smartreflex.c |   15 +++------------
>  1 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
> index 77ecebf..e54db84 100644
> --- a/arch/arm/mach-omap2/smartreflex.c
> +++ b/arch/arm/mach-omap2/smartreflex.c
> @@ -927,19 +927,10 @@ static int __init omap_sr_probe(struct platform_device *pdev)
>  	}
>  
>  	for (i = 0; i < sr_info->nvalue_count; i++) {
> -		char *name;
> -		char volt_name[32];
> -
> -		name = kzalloc(NVALUE_NAME_LEN + 1, GFP_KERNEL);
> -		if (!name) {
> -			dev_err(&pdev->dev, "%s: Unable to allocate memory"
> -				" for n-value directory name\n",  __func__);
> -			return -ENOMEM;
> -		}
> +		char name[NVALUE_NAME_LEN + 1];
>  
> -		strcpy(name, "volt_");
> -		sprintf(volt_name, "%d", volt_data[i].volt_nominal);
> -		strcat(name, volt_name);
> +		snprintf(name, sizeof(name), "volt_%d",
> +			 volt_data[i].volt_nominal);
>  		(void) debugfs_create_x32(name, S_IRUGO | S_IWUGO, nvalue_dir,
>  				&(sr_info->nvalue_table[i].nvalue));
>  	}

  parent reply	other threads:[~2011-02-14 22:38 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-07 14:08 [PATCHv2] arm: mach-omap2: smartreflex: fix another memory leak Aaro Koskinen
2011-02-07 14:08 ` Aaro Koskinen
2011-02-07 15:04 ` Vishwanath Sripathy
2011-02-07 15:04   ` Vishwanath Sripathy
2011-02-07 15:14   ` Aaro Koskinen
2011-02-07 15:14     ` Aaro Koskinen
2011-02-14 22:38 ` Kevin Hilman [this message]
2011-02-14 22:38   ` Kevin Hilman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bp2ez1fx.fsf@ti.com \
    --to=khilman@ti.com \
    --cc=aaro.koskinen@nokia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=tony@atomide.com \
    --cc=vishwanath.bs@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.