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 <[email protected]>
---
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
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 <[email protected]>
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
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 <[email protected]>
>
> 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 <[email protected]>
---
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
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 <[email protected]>
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