On Tue, Aug 28, 2018 at 11:26:22AM +0100, Catalin Marinas wrote:
> On Tue, Aug 28, 2018 at 12:14:12PM +0200, Vincent Whitchurch wrote:
> > On Mon, Aug 27, 2018 at 03:16:41PM -0700, Andrew Morton wrote:
> > > On Mon, 27 Aug 2018 10:38:21 +0200 Vincent Whitchurch
> > > <vincent.whitchu...@axis.com> wrote:
> > > > +config DEBUG_KMEMLEAK_WARN
> > > > + bool "Print kmemleak object warnings to log buffer"
> > > > + depends on DEBUG_KMEMLEAK
> > > > + help
> > > > + Say Y here to make kmemleak print information about
> > > > unreferenced
> > > > + objects (including stacktraces) as warnings to the kernel log
> > > > buffer.
> > > > + Otherwise this information is only available by reading the
> > > > kmemleak
> > > > + debugfs file.
> > >
> > > Why add the config option? Why not simply make the change for all
> > > configs?
> >
> > No particular reason other than preserving the current behaviour for
> > existing users. I can remove the config option if Catalin is fine with
> > it.
>
> IIRC, in the early kmemleak days, people complained about it being to
> noisy (the false positives rate was also much higher), so the default
> behaviour was changed to monitor (almost) quietly with the details
> available via debugfs. I'd like to keep this default behaviour but we
> could have a "verbose" command via both debugfs and kernel parameter (as
> we do with "off" and "on"). Would this work for you?
Either a config option or a parameter are usable for me. How about
something like this? It can be enabled with kmemleak.verbose=1 or "echo
1 > /sys/module/kmemleak/parameters/verbose":
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9a3fc905b8bd..ab1b599202bc 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -593,15 +593,6 @@ config DEBUG_KMEMLEAK_DEFAULT_OFF
Say Y here to disable kmemleak by default. It can then be enabled
on the command line via kmemleak=on.
-config DEBUG_KMEMLEAK_WARN
- bool "Print kmemleak object warnings to log buffer"
- depends on DEBUG_KMEMLEAK
- help
- Say Y here to make kmemleak print information about unreferenced
- objects (including stacktraces) as warnings to the kernel log buffer.
- Otherwise this information is only available by reading the kmemleak
- debugfs file.
-
config DEBUG_STACK_USAGE
bool "Stack utilization instrumentation"
depends on DEBUG_KERNEL && !IA64
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 22662715a3dc..c91d43738596 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -86,6 +86,7 @@
#include <linux/seq_file.h>
#include <linux/cpumask.h>
#include <linux/spinlock.h>
+#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/rcupdate.h>
#include <linux/stacktrace.h>
@@ -236,6 +237,9 @@ static int kmemleak_skip_disable;
/* If there are leaks that can be reported */
static bool kmemleak_found_leaks;
+static bool kmemleak_verbose;
+module_param_named(verbose, kmemleak_verbose, bool, 0600);
+
/*
* Early object allocation/freeing logging. Kmemleak is initialized after the
* kernel allocator. However, both the kernel allocator and kmemleak may
@@ -1618,9 +1622,10 @@ static void kmemleak_scan(void)
if (unreferenced_object(object) &&
!(object->flags & OBJECT_REPORTED)) {
object->flags |= OBJECT_REPORTED;
-#ifdef CONFIG_DEBUG_KMEMLEAK_WARN
- print_unreferenced(NULL, object);
-#endif
+
+ if (kmemleak_verbose)
+ print_unreferenced(NULL, object);
+
new_leaks++;
}
spin_unlock_irqrestore(&object->lock, flags);
