[PATCH] mm: kmemleak: Change kmemleak default buffer size

[PATCH] mm: kmemleak: Change kmemleak default buffer size

[Sankar P]

If we try to find the memory leaks in kernel that is
compiled with 'make defconfig', the default buffer size
seem to be inadequate. Change the buffer size from
400 to 1000, which is sufficient in most cases.

Signed-off-by: Sankar P <sankar.curiosity@gmail.com>
---
 arch/sh/configs/sh7785lcr_32bit_defconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/sh/configs/sh7785lcr_32bit_defconfig b/arch/sh/configs/sh7785lcr_32bit_defconfig
index 71f39c7..b02e5ae 100644
--- a/arch/sh/configs/sh7785lcr_32bit_defconfig
+++ b/arch/sh/configs/sh7785lcr_32bit_defconfig
@@ -1710,7 +1710,7 @@ CONFIG_SCHEDSTATS=y
 # CONFIG_DEBUG_OBJECTS is not set
 # CONFIG_DEBUG_SLAB is not set
 CONFIG_DEBUG_KMEMLEAK=y
-CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE@0
+CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE\x1000
 # CONFIG_DEBUG_KMEMLEAK_TEST is not set
 CONFIG_DEBUG_PREEMPT=y
 # CONFIG_DEBUG_RT_MUTEXES is not set
-- 
1.6.4.2

Re: [PATCH] mm: kmemleak: Change kmemleak default buffer size

[Pekka Enberg]

Sankar P wrote:
> If we try to find the memory leaks in kernel that is
> compiled with 'make defconfig', the default buffer size
> seem to be inadequate. Change the buffer size from
> 400 to 1000, which is sufficient in most cases.
> 
> Signed-off-by: Sankar P <sankar.curiosity@gmail.com>
> ---
>  arch/sh/configs/sh7785lcr_32bit_defconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/sh/configs/sh7785lcr_32bit_defconfig b/arch/sh/configs/sh7785lcr_32bit_defconfig
> index 71f39c7..b02e5ae 100644
> --- a/arch/sh/configs/sh7785lcr_32bit_defconfig
> +++ b/arch/sh/configs/sh7785lcr_32bit_defconfig
> @@ -1710,7 +1710,7 @@ CONFIG_SCHEDSTATS=y
>  # CONFIG_DEBUG_OBJECTS is not set
>  # CONFIG_DEBUG_SLAB is not set
>  CONFIG_DEBUG_KMEMLEAK=y
> -CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE@0
> +CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE\x1000
>  # CONFIG_DEBUG_KMEMLEAK_TEST is not set
>  CONFIG_DEBUG_PREEMPT=y
>  # CONFIG_DEBUG_RT_MUTEXES is not set

I'm pretty sure you want to do this change in lib/Kconfig.debug.

[PATCH] kmemleak: config-options: Default buffer size for kmemleak

[Sankar P]

If we try to find the memory leaks in kernel that is
compiled with 'make defconfig', the default buffer size
of DEBUG_KMEMLEAK_EARLY_LOG_SIZE seem to be inadequate.

Change the buffer size from 400 to 1000,
which is sufficient for most cases.

Signed-off-by: Sankar P <sankar.curiosity@gmail.com>
---

Thanks to Pekka Enberg's comments on my previous mail, I am sending a better patch,
and adding new reviewers as suggested by the get_maintainer script. 

 lib/Kconfig.debug |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index e722e9d..5eb9463 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -382,7 +382,7 @@ config DEBUG_KMEMLEAK_EARLY_LOG_SIZE
 	int "Maximum kmemleak early log entries"
 	depends on DEBUG_KMEMLEAK
 	range 200 40000
-	default 400
+	default 1000
 	help
 	  Kmemleak must track all the memory allocations to avoid
 	  reporting false positives. Since memory may be allocated or
-- 
1.6.4.2

Re: [PATCH] kmemleak: config-options: Default buffer size for kmemleak

[Paul Mundt]

On Tue, Jun 22, 2010 at 02:16:43PM +0530, Sankar P wrote:
> If we try to find the memory leaks in kernel that is
> compiled with 'make defconfig', the default buffer size
> of DEBUG_KMEMLEAK_EARLY_LOG_SIZE seem to be inadequate.
> 
> Change the buffer size from 400 to 1000,
> which is sufficient for most cases.
> 
Or you could just bump it up in your config where you seem to be hitting
this problem. The default of 400 is sufficient for most people, so
bloating it up for a corner case seems a bit premature. Perhaps
eventually we'll have no choice and have to tolerate the bloat, as we did
with LOG_BUF_SHIFT, but it's not obvious that we've hit that point with
kmemleak yet.

Re: [PATCH] kmemleak: config-options: Default buffer size for

[Catalin Marinas]

On Tue, 2010-06-22 at 12:31 +0100, Paul Mundt wrote:
> On Tue, Jun 22, 2010 at 02:16:43PM +0530, Sankar P wrote:
> > If we try to find the memory leaks in kernel that is
> > compiled with 'make defconfig', the default buffer size
> > of DEBUG_KMEMLEAK_EARLY_LOG_SIZE seem to be inadequate.
> >
> > Change the buffer size from 400 to 1000,
> > which is sufficient for most cases.
> >
> Or you could just bump it up in your config where you seem to be hitting
> this problem. The default of 400 is sufficient for most people, so
> bloating it up for a corner case seems a bit premature. Perhaps
> eventually we'll have no choice and have to tolerate the bloat, as we did
> with LOG_BUF_SHIFT, but it's not obvious that we've hit that point with
> kmemleak yet.

I agree. The 400 seems to be sufficient with standard kernel
configurations (I usually try some of the Ubuntu configs on x86). The
error message is hopefully clear enough about what needs to be changed.

The defconfig change for this specific platform may be a better option
but I thought defconfigs are to provide a stable (and maybe close to
optimal) configuration without all the debugging features enabled
(especially those slowing things down considerably).

-- 
Catalin

Re: [PATCH] kmemleak: config-options: Default buffer size for kmemleak

[Paul Mundt]

On Tue, Jun 22, 2010 at 01:05:51PM +0100, Catalin Marinas wrote:
> On Tue, 2010-06-22 at 12:31 +0100, Paul Mundt wrote:
> > On Tue, Jun 22, 2010 at 02:16:43PM +0530, Sankar P wrote:
> > > If we try to find the memory leaks in kernel that is
> > > compiled with 'make defconfig', the default buffer size
> > > of DEBUG_KMEMLEAK_EARLY_LOG_SIZE seem to be inadequate.
> > >
> > > Change the buffer size from 400 to 1000,
> > > which is sufficient for most cases.
> > >
> > Or you could just bump it up in your config where you seem to be hitting
> > this problem. The default of 400 is sufficient for most people, so
> > bloating it up for a corner case seems a bit premature. Perhaps
> > eventually we'll have no choice and have to tolerate the bloat, as we did
> > with LOG_BUF_SHIFT, but it's not obvious that we've hit that point with
> > kmemleak yet.
> 
> I agree. The 400 seems to be sufficient with standard kernel
> configurations (I usually try some of the Ubuntu configs on x86). The
> error message is hopefully clear enough about what needs to be changed.
> 
> The defconfig change for this specific platform may be a better option
> but I thought defconfigs are to provide a stable (and maybe close to
> optimal) configuration without all the debugging features enabled
> (especially those slowing things down considerably).
> 
I would be fine with that, but I don't see any correlation between the
posted dmesg and the defconfig? I've run the config in question without
hitting problems, so I'm a bit confused as to why that particular config
was singled out.

Re: [PATCH] kmemleak: config-options: Default buffer size for

[Pekka Enberg]

On Tue, Jun 22, 2010 at 3:05 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> The defconfig change for this specific platform may be a better option
> but I thought defconfigs are to provide a stable (and maybe close to
> optimal) configuration without all the debugging features enabled
> (especially those slowing things down considerably).

The defconfig change was definitely not a clean solution to this
problem. Better bake the fix in Kconfig proper even if it means
dependency on CONFIG_SH or something.

Re: [PATCH] kmemleak: config-options: Default buffer size for

[Catalin Marinas]

On Tue, 2010-06-22 at 13:21 +0100, Pekka Enberg wrote:
> On Tue, Jun 22, 2010 at 3:05 PM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > The defconfig change for this specific platform may be a better option
> > but I thought defconfigs are to provide a stable (and maybe close to
> > optimal) configuration without all the debugging features enabled
> > (especially those slowing things down considerably).
> 
> The defconfig change was definitely not a clean solution to this
> problem. Better bake the fix in Kconfig proper even if it means
> dependency on CONFIG_SH or something.

OK, maybe something like this

	default 1000 if SH
	default 400

It seems that kbuild only considers the first encounter of "default".

Thanks.

-- 
Catalin

Re: [PATCH] kmemleak: config-options: Default buffer size for

[Pekka Enberg]

On Tue, Jun 22, 2010 at 3:35 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Tue, 2010-06-22 at 13:21 +0100, Pekka Enberg wrote:
>> On Tue, Jun 22, 2010 at 3:05 PM, Catalin Marinas
>> <catalin.marinas@arm.com> wrote:
>> > The defconfig change for this specific platform may be a better option
>> > but I thought defconfigs are to provide a stable (and maybe close to
>> > optimal) configuration without all the debugging features enabled
>> > (especially those slowing things down considerably).
>>
>> The defconfig change was definitely not a clean solution to this
>> problem. Better bake the fix in Kconfig proper even if it means
>> dependency on CONFIG_SH or something.
>
> OK, maybe something like this
>
>        default 1000 if SH
>        default 400
>
> It seems that kbuild only considers the first encounter of "default".

Sure, I'm fine with that.

Re: [PATCH] mm: kmemleak: Change kmemleak default buffer size

[Luis R. Rodriguez]

On Mon, Jun 21, 2010 at 11:58:29PM -0700, Sankar P wrote:
> If we try to find the memory leaks in kernel that is
> compiled with 'make defconfig', the default buffer size
> seem to be inadequate. Change the buffer size from
> 400 to 1000, which is sufficient in most cases.
> 
> Signed-off-by: Sankar P <sankar.curiosity@gmail.com>

What's your full name? Please read the "Developer's Certificate of Origin 1.1"
It says:

then you just add a line saying

        Signed-off-by: Random J Developer <random@developer.example.org>

using your real name (sorry, no pseudonyms or anonymous contributions.)


Also you may want to post on a new thread instead of using this old thread
unless the maintainer is reading this and wants to pick it up.

  Luis

> ---
>  arch/sh/configs/sh7785lcr_32bit_defconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/sh/configs/sh7785lcr_32bit_defconfig b/arch/sh/configs/sh7785lcr_32bit_defconfig
> index 71f39c7..b02e5ae 100644
> --- a/arch/sh/configs/sh7785lcr_32bit_defconfig
> +++ b/arch/sh/configs/sh7785lcr_32bit_defconfig
> @@ -1710,7 +1710,7 @@ CONFIG_SCHEDSTATS=y
>  # CONFIG_DEBUG_OBJECTS is not set
>  # CONFIG_DEBUG_SLAB is not set
>  CONFIG_DEBUG_KMEMLEAK=y
> -CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE@0
> +CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE\x1000
>  # CONFIG_DEBUG_KMEMLEAK_TEST is not set
>  CONFIG_DEBUG_PREEMPT=y
>  # CONFIG_DEBUG_RT_MUTEXES is not set
> -- 
> 1.6.4.2
> 

Re: [PATCH] mm: kmemleak: Change kmemleak default buffer size

[Sankar P]

On Tue, Jun 22, 2010 at 10:25 PM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> On Mon, Jun 21, 2010 at 11:58:29PM -0700, Sankar P wrote:
>> If we try to find the memory leaks in kernel that is
>> compiled with 'make defconfig', the default buffer size
>> seem to be inadequate. Change the buffer size from
>> 400 to 1000, which is sufficient in most cases.
>>
>> Signed-off-by: Sankar P <sankar.curiosity@gmail.com>
>
> What's your full name? Please read the "Developer's Certificate of Origin 1.1"
> It says:
>
> then you just add a line saying
>
>        Signed-off-by: Random J Developer <random@developer.example.org>
>
> using your real name (sorry, no pseudonyms or anonymous contributions.)
>
>
> Also you may want to post on a new thread instead of using this old thread
> unless the maintainer is reading this and wants to pick it up.
>

In our part of the world, we dont have lastnames. We just use the
first letter of our father's name as the last name.

I will send the updated patch as a new mail, I thought it will be
easier to follow if all mails belongs to the same thread.

Thanks

>  Luis
>
>> ---
>>  arch/sh/configs/sh7785lcr_32bit_defconfig |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/sh/configs/sh7785lcr_32bit_defconfig b/arch/sh/configs/sh7785lcr_32bit_defconfig
>> index 71f39c7..b02e5ae 100644
>> --- a/arch/sh/configs/sh7785lcr_32bit_defconfig
>> +++ b/arch/sh/configs/sh7785lcr_32bit_defconfig
>> @@ -1710,7 +1710,7 @@ CONFIG_SCHEDSTATS=y
>>  # CONFIG_DEBUG_OBJECTS is not set
>>  # CONFIG_DEBUG_SLAB is not set
>>  CONFIG_DEBUG_KMEMLEAK=y
>> -CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE@0
>> +CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE\x1000
>>  # CONFIG_DEBUG_KMEMLEAK_TEST is not set
>>  CONFIG_DEBUG_PREEMPT=y
>>  # CONFIG_DEBUG_RT_MUTEXES is not set
>> --
>> 1.6.4.2
>>
>



-- 
Sankar P
http://psankar.blogspot.com

Re: [PATCH] mm: kmemleak: Change kmemleak default buffer size

[Luis R. Rodriguez]

On Thu, Jun 24, 2010 at 12:31:11AM -0700, Sankar P wrote:
> On Tue, Jun 22, 2010 at 10:25 PM, Luis R. Rodriguez
> <lrodriguez@atheros.com> wrote:
> > On Mon, Jun 21, 2010 at 11:58:29PM -0700, Sankar P wrote:
> >> If we try to find the memory leaks in kernel that is
> >> compiled with 'make defconfig', the default buffer size
> >> seem to be inadequate. Change the buffer size from
> >> 400 to 1000, which is sufficient in most cases.
> >>
> >> Signed-off-by: Sankar P <sankar.curiosity@gmail.com>
> >
> > What's your full name? Please read the "Developer's Certificate of Origin 1.1"
> > It says:
> >
> > then you just add a line saying
> >
> >        Signed-off-by: Random J Developer <random@developer.example.org>
> >
> > using your real name (sorry, no pseudonyms or anonymous contributions.)
> >
> >
> > Also you may want to post on a new thread instead of using this old thread
> > unless the maintainer is reading this and wants to pick it up.
> >
> 
> In our part of the world, we dont have lastnames. We just use the
> first letter of our father's name as the last name.

Oh wow, what part of the world is that? Interesting.

> I will send the updated patch as a new mail, I thought it will be
> easier to follow if all mails belongs to the same thread.

It does help in-thread, but patches should be sent separately unless
you know for sure the maintainer *will* read this.

  Luis