* [PATCH] introduce a default off mode for kmemleak @ 2010-07-13 20:31 Jason Baron 2010-07-13 21:02 ` Catalin Marinas 0 siblings, 1 reply; 4+ messages in thread From: Jason Baron @ 2010-07-13 20:31 UTC (permalink / raw) To: catalin.marinas; +Cc: linux-kernel Hi, Kmemleak can be very useful in figuring out where a kernel leak is suspected, but I've run into issues it which makes it hard to enable by default - reports false positives, oom kills on low memory systems (512 MB). Although these can be addressed, there is still the memory footprint, and runtime overhead, so I think it might make sense to create mode where it is disabled by default, but can still be enabled on the command line via: kmemleak=on. thanks, -Jason Introduce a new DEBUG_KMEMLEAK_DEFAULT_OFF config parameter that allows kmemleak to be disabled by default, but enabled on the command line via: kmemleak=on. Although a reboot is required to turn it on, its still useful to not require a re-compile. Signed-off-by: Jason Baron <jbaron@redhat.com> --- lib/Kconfig.debug | 7 +++++++ mm/kmemleak.c | 26 +++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index e722e9d..95ab402 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -400,6 +400,13 @@ config DEBUG_KMEMLEAK_TEST If unsure, say N. +config DEBUG_KMEMLEAK_DEFAULT_OFF + bool "Default kmemleak to off" + depends on DEBUG_KMEMLEAK + help + Say Y here to disable kmemleak by default. It can then be enabled + on the command line via kmemleak=on. + config DEBUG_PREEMPT bool "Debug preemptible kernel" depends on DEBUG_KERNEL && PREEMPT && TRACE_IRQFLAGS_SUPPORT diff --git a/mm/kmemleak.c b/mm/kmemleak.c index 2c0d032..1006267 100644 --- a/mm/kmemleak.c +++ b/mm/kmemleak.c @@ -211,6 +211,11 @@ static signed long jiffies_scan_wait; static int kmemleak_stack_scan = 1; /* protects the memory scanning, parameters and debug/kmemleak file access */ static DEFINE_MUTEX(scan_mutex); +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF +/* setting kmemleak=on, will set this var, skipping the disable */ +static int kmemleak_skip_disable; +#endif + /* * Early object allocation/freeing logging. Kmemleak is initialized after the @@ -1600,10 +1605,22 @@ static int kmemleak_boot_config(char *str) { if (!str) return -EINVAL; + if (strcmp(str, "off") == 0) +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF + return -EINVAL; +#else kmemleak_disable(); - else if (strcmp(str, "on") != 0) +#endif + else if (strcmp(str, "on") == 0) +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF + kmemleak_skip_disable = 1; +#else + return -EINVAL; +#endif + else return -EINVAL; + return 0; } early_param("kmemleak", kmemleak_boot_config); @@ -1616,6 +1633,13 @@ void __init kmemleak_init(void) int i; unsigned long flags; +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF + if (!kmemleak_skip_disable) { + kmemleak_disable(); + return; + } +#endif + jiffies_min_age = msecs_to_jiffies(MSECS_MIN_AGE); jiffies_scan_wait = msecs_to_jiffies(SECS_SCAN_WAIT * 1000); -- 1.7.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] introduce a default off mode for kmemleak 2010-07-13 20:31 [PATCH] introduce a default off mode for kmemleak Jason Baron @ 2010-07-13 21:02 ` Catalin Marinas 2010-07-13 21:55 ` Jason Baron 0 siblings, 1 reply; 4+ messages in thread From: Catalin Marinas @ 2010-07-13 21:02 UTC (permalink / raw) To: Jason Baron; +Cc: linux-kernel Hi Jason, On Tue, 2010-07-13 at 21:31 +0100, Jason Baron wrote: > Introduce a new DEBUG_KMEMLEAK_DEFAULT_OFF config parameter that allows > kmemleak to be disabled by default, but enabled on the command line via: > kmemleak=on. Although a reboot is required to turn it on, its still > useful to not require a re-compile. > > Signed-off-by: Jason Baron <jbaron@redhat.com> The patch makes sense. I have a few comments below. > diff --git a/mm/kmemleak.c b/mm/kmemleak.c > index 2c0d032..1006267 100644 > --- a/mm/kmemleak.c > +++ b/mm/kmemleak.c > @@ -211,6 +211,11 @@ static signed long jiffies_scan_wait; > static int kmemleak_stack_scan = 1; > /* protects the memory scanning, parameters and debug/kmemleak file access */ > static DEFINE_MUTEX(scan_mutex); > +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF > +/* setting kmemleak=on, will set this var, skipping the disable */ > +static int kmemleak_skip_disable; > +#endif Can we have kmemleak_skip_disable always and remove the #ifdef? > @@ -1600,10 +1605,22 @@ static int kmemleak_boot_config(char *str) > { > if (!str) > return -EINVAL; > + > if (strcmp(str, "off") == 0) > +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF > + return -EINVAL; > +#else > kmemleak_disable(); > - else if (strcmp(str, "on") != 0) > +#endif > + else if (strcmp(str, "on") == 0) > +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF > + kmemleak_skip_disable = 1; > +#else > + return -EINVAL; > +#endif > + else > return -EINVAL; > + > return 0; > } Here I would remove all these #ifdef's. It doesn't really matter if you pass kmemleak=off/on on the command line and kmemleak was already disable/enabled. If we remove the #ifdef for the kmemleak_skip_disable, we can always set the variable here. > @@ -1616,6 +1633,13 @@ void __init kmemleak_init(void) > int i; > unsigned long flags; > > +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF > + if (!kmemleak_skip_disable) { > + kmemleak_disable(); > + return; > + } > +#endif That's the only place where we could keep the #ifdef. Thanks. -- Catalin ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] introduce a default off mode for kmemleak 2010-07-13 21:02 ` Catalin Marinas @ 2010-07-13 21:55 ` Jason Baron 2010-07-14 9:42 ` Catalin Marinas 0 siblings, 1 reply; 4+ messages in thread From: Jason Baron @ 2010-07-13 21:55 UTC (permalink / raw) To: Catalin Marinas; +Cc: linux-kernel On Tue, Jul 13, 2010 at 10:02:12PM +0100, Catalin Marinas wrote: > Hi Jason, > > On Tue, 2010-07-13 at 21:31 +0100, Jason Baron wrote: > > Introduce a new DEBUG_KMEMLEAK_DEFAULT_OFF config parameter that allows > > kmemleak to be disabled by default, but enabled on the command line via: > > kmemleak=on. Although a reboot is required to turn it on, its still > > useful to not require a re-compile. > > > > Signed-off-by: Jason Baron <jbaron@redhat.com> > > The patch makes sense. I have a few comments below. > > > diff --git a/mm/kmemleak.c b/mm/kmemleak.c > > index 2c0d032..1006267 100644 > > --- a/mm/kmemleak.c > > +++ b/mm/kmemleak.c > > @@ -211,6 +211,11 @@ static signed long jiffies_scan_wait; > > static int kmemleak_stack_scan = 1; > > /* protects the memory scanning, parameters and debug/kmemleak file access */ > > static DEFINE_MUTEX(scan_mutex); > > +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF > > +/* setting kmemleak=on, will set this var, skipping the disable */ > > +static int kmemleak_skip_disable; > > +#endif > > Can we have kmemleak_skip_disable always and remove the #ifdef? > > > @@ -1600,10 +1605,22 @@ static int kmemleak_boot_config(char *str) > > { > > if (!str) > > return -EINVAL; > > + > > if (strcmp(str, "off") == 0) > > +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF > > + return -EINVAL; > > +#else > > kmemleak_disable(); > > - else if (strcmp(str, "on") != 0) > > +#endif > > + else if (strcmp(str, "on") == 0) > > +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF > > + kmemleak_skip_disable = 1; > > +#else > > + return -EINVAL; > > +#endif > > + else > > return -EINVAL; > > + > > return 0; > > } > > Here I would remove all these #ifdef's. It doesn't really matter if you > pass kmemleak=off/on on the command line and kmemleak was already > disable/enabled. If we remove the #ifdef for the kmemleak_skip_disable, > we can always set the variable here. > > > @@ -1616,6 +1633,13 @@ void __init kmemleak_init(void) > > int i; > > unsigned long flags; > > > > +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF > > + if (!kmemleak_skip_disable) { > > + kmemleak_disable(); > > + return; > > + } > > +#endif > > That's the only place where we could keep the #ifdef. > > Thanks. > > -- > Catalin > Ok. re-vised patch is below. thanks, -Jason Introduce a new DEBUG_KMEMLEAK_DEFAULT_OFF config parameter that allows kmemleak to be disabled by default, but enabled on the command line via: kmemleak=on. Although a reboot is required to turn it on, its still useful to not require a re-compile. Signed-off-by: Jason Baron <jbaron@redhat.com> --- lib/Kconfig.debug | 7 +++++++ mm/kmemleak.c | 14 +++++++++++++- 2 files changed, 20 insertions(+), 1 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index e722e9d..95ab402 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -400,6 +400,13 @@ config DEBUG_KMEMLEAK_TEST If unsure, say N. +config DEBUG_KMEMLEAK_DEFAULT_OFF + bool "Default kmemleak to off" + depends on DEBUG_KMEMLEAK + help + Say Y here to disable kmemleak by default. It can then be enabled + on the command line via kmemleak=on. + config DEBUG_PREEMPT bool "Debug preemptible kernel" depends on DEBUG_KERNEL && PREEMPT && TRACE_IRQFLAGS_SUPPORT diff --git a/mm/kmemleak.c b/mm/kmemleak.c index 2c0d032..c7ff733 100644 --- a/mm/kmemleak.c +++ b/mm/kmemleak.c @@ -211,6 +211,9 @@ static signed long jiffies_scan_wait; static int kmemleak_stack_scan = 1; /* protects the memory scanning, parameters and debug/kmemleak file access */ static DEFINE_MUTEX(scan_mutex); +/* setting kmemleak=on, will set this var, skipping the disable */ +static int kmemleak_skip_disable; + /* * Early object allocation/freeing logging. Kmemleak is initialized after the @@ -1602,7 +1605,9 @@ static int kmemleak_boot_config(char *str) return -EINVAL; if (strcmp(str, "off") == 0) kmemleak_disable(); - else if (strcmp(str, "on") != 0) + else if (strcmp(str, "on") == 0) + kmemleak_skip_disable = 1; + else return -EINVAL; return 0; } @@ -1616,6 +1621,13 @@ void __init kmemleak_init(void) int i; unsigned long flags; +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF + if (!kmemleak_skip_disable) { + kmemleak_disable(); + return; + } +#endif + jiffies_min_age = msecs_to_jiffies(MSECS_MIN_AGE); jiffies_scan_wait = msecs_to_jiffies(SECS_SCAN_WAIT * 1000); -- 1.7.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] introduce a default off mode for kmemleak 2010-07-13 21:55 ` Jason Baron @ 2010-07-14 9:42 ` Catalin Marinas 0 siblings, 0 replies; 4+ messages in thread From: Catalin Marinas @ 2010-07-14 9:42 UTC (permalink / raw) To: Jason Baron; +Cc: linux-kernel On Tue, 2010-07-13 at 22:55 +0100, Jason Baron wrote: > Introduce a new DEBUG_KMEMLEAK_DEFAULT_OFF config parameter that allows > kmemleak to be disabled by default, but enabled on the command line > via: kmemleak=on. Although a reboot is required to turn it on, its still > useful to not require a re-compile. > > Signed-off-by: Jason Baron <jbaron@redhat.com> Thanks, it looks fine to me. I'll merge it in my tree and send Linus a pull request during the merging window (unless you prefer to push it yourself, in which case I ack it). -- Catalin ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-07-14 9:42 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-13 20:31 [PATCH] introduce a default off mode for kmemleak Jason Baron 2010-07-13 21:02 ` Catalin Marinas 2010-07-13 21:55 ` Jason Baron 2010-07-14 9:42 ` Catalin Marinas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox