2009-06-25 22:16:59

by Sergey Senozhatsky

[permalink] [raw]
Subject: kmemleak suggestion (long message)

Hello.

Currently kmemleak prints info about all objects. I guess sometimes kmemleak gives you more than you actually need.
syslog:
...
kmemleak: unreferenced object 0xf702fad0 (size 152):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
kmemleak: [<c123b0c4>] idr_pre_get+0x64/0x90
kmemleak: [<c123b115>] ida_pre_get+0x25/0xe0
kmemleak: [<c10edf3f>] set_anon_super+0x2f/0xf0
kmemleak: [<c10eea50>] sget+0x2d0/0x390
kmemleak: [<c10ef0f1>] get_sb_single+0x41/0xd0
kmemleak: [<c114a3f8>] sysfs_get_sb+0x28/0x40
kmemleak: [<c10eeea1>] vfs_kern_mount+0x71/0x150
kmemleak: [<c10eefa1>] kern_mount_data+0x21/0x40
kmemleak: [<c15cb2f5>] sysfs_init+0x67/0xcc
kmemleak: [<c15ca0f0>] mnt_init+0x9d/0x182
kmemleak: [<c15c9ca6>] vfs_caches_init+0x10d/0x130
kmemleak: [<c15aca26>] start_kernel+0x2f4/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
kmemleak: unreferenced object 0xf702fa20 (size 152):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
kmemleak: [<c123b0c4>] idr_pre_get+0x64/0x90
kmemleak: [<c123b115>] ida_pre_get+0x25/0xe0
kmemleak: [<c10edf3f>] set_anon_super+0x2f/0xf0
kmemleak: [<c10eea50>] sget+0x2d0/0x390
kmemleak: [<c10ef0f1>] get_sb_single+0x41/0xd0
kmemleak: [<c114a3f8>] sysfs_get_sb+0x28/0x40
kmemleak: [<c10eeea1>] vfs_kern_mount+0x71/0x150
kmemleak: [<c10eefa1>] kern_mount_data+0x21/0x40
kmemleak: [<c15cb2f5>] sysfs_init+0x67/0xcc
kmemleak: [<c15ca0f0>] mnt_init+0x9d/0x182
kmemleak: [<c15c9ca6>] vfs_caches_init+0x10d/0x130
kmemleak: [<c15aca26>] start_kernel+0x2f4/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
...
kmemleak: unreferenced object 0xf7028140 (size 1024):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e584d>] __kmalloc+0x16d/0x210
kmemleak: [<c10e5bcd>] alloc_arraycache+0x2d/0x70
kmemleak: [<c10e5e46>] do_tune_cpucache+0x236/0x3e0
kmemleak: [<c10e6192>] enable_cpucache+0x42/0x110
kmemleak: [<c13ec2a3>] setup_cpu_cache+0x183/0x2a0
kmemleak: [<c10e65f5>] kmem_cache_create+0x395/0x5a0
kmemleak: [<c15cf0d1>] radix_tree_init+0x33/0x87
kmemleak: [<c15aca2b>] start_kernel+0x2f9/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
kmemleak: unreferenced object 0xf70057b0 (size 128):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e584d>] __kmalloc+0x16d/0x210
kmemleak: [<c113b729>] __proc_create+0x99/0x120
kmemleak: [<c113c143>] proc_symlink+0x33/0xb0
kmemleak: [<c15cac41>] proc_root_init+0x60/0xbe
kmemleak: [<c15aca3a>] start_kernel+0x308/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
....
kmemleak: unreferenced object 0xf702fce0 (size 152):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
kmemleak: [<c123b0c4>] idr_pre_get+0x64/0x90
kmemleak: [<c123b115>] ida_pre_get+0x25/0xe0
kmemleak: [<c113bcaf>] proc_register+0x2f/0x1e0
kmemleak: [<c113c17e>] proc_symlink+0x6e/0xb0
kmemleak: [<c15cac41>] proc_root_init+0x60/0xbe
kmemleak: [<c15aca3a>] start_kernel+0x308/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
....x10

Suppose, I'm monitoring application which never calls "[<c123b0c4>] idr_pre_get" or
"[<c12cf49b>] tty_ldisc_try_get". It would be nice to "temporarily turn-off unimportant" output.
I suggest to give ability to "blacklist" function(s). I did it via printed by kmemleak function address %p
(like [<c10e963b>]) since it's impossible to assume what objects will be reported in next YYY seconds
("unreferenced object 0xf702fce0"). (It's possible to "blacklist" according to function name
(like idr_pre_get) but I think %p is quite enough).

For example, to avoid some notifications I just copy-paste address from printed stack:
echo "block=c123b0c4" > /sys/kernel/debug/kmemleak

//debug output
kmemleak: Added to blacklist: <c123b0c4>

syslog:
kmemleak: unreferenced object 0xf7049a10 (size 1024):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e584d>] __kmalloc+0x16d/0x210
kmemleak: [<c10e5bcd>] alloc_arraycache+0x2d/0x70
kmemleak: [<c10e5e46>] do_tune_cpucache+0x236/0x3e0
kmemleak: [<c10e6192>] enable_cpucache+0x42/0x110
kmemleak: [<c13ec2a3>] setup_cpu_cache+0x183/0x2a0
kmemleak: [<c10e65f5>] kmem_cache_create+0x395/0x5a0
kmemleak: [<c15c2a6b>] signals_init+0x34/0x4c
kmemleak: [<c15aca30>] start_kernel+0x2fe/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
kmemleak: unreferenced object 0xf70495f8 (size 1024):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e584d>] __kmalloc+0x16d/0x210
kmemleak: [<c10e5bcd>] alloc_arraycache+0x2d/0x70
kmemleak: [<c10e5e46>] do_tune_cpucache+0x236/0x3e0
kmemleak: [<c10e6192>] enable_cpucache+0x42/0x110
kmemleak: [<c13ec2a3>] setup_cpu_cache+0x183/0x2a0
kmemleak: [<c10e65f5>] kmem_cache_create+0x395/0x5a0
kmemleak: [<c15cabc9>] proc_init_inodecache+0x31/0x49
kmemleak: [<c15cabf7>] proc_root_init+0x16/0xbe
kmemleak: [<c15aca3a>] start_kernel+0x308/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
kmemleak: unreferenced object 0xf70057b0 (size 128):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e584d>] __kmalloc+0x16d/0x210
kmemleak: [<c113b729>] __proc_create+0x99/0x120
kmemleak: [<c113c143>] proc_symlink+0x33/0xb0
kmemleak: [<c15cac41>] proc_root_init+0x60/0xbe
kmemleak: [<c15aca3a>] start_kernel+0x308/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
...
//Debug output showing that we have blocked objects.
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
kmemleak: Function blacklisted <c123b0c4>
...
kmemleak: unreferenced object 0xf7005680 (size 128):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e584d>] __kmalloc+0x16d/0x210
kmemleak: [<c113b729>] __proc_create+0x99/0x120
kmemleak: [<c113c143>] proc_symlink+0x33/0xb0
kmemleak: [<c15cb03a>] proc_net_init+0x22/0x3f
kmemleak: [<c15cac46>] proc_root_init+0x65/0xbe
kmemleak: [<c15aca3a>] start_kernel+0x308/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff

To unblock:
echo "unblock=c123b0c4" > /sys/kernel/debug/kmemleak

//Debug output
kmemleak: Removed from blacklist <c123b0c4>

syslog:
...
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
kmemleak: [<c123b0c4>] idr_pre_get+0x64/0x90
kmemleak: [<c123b115>] ida_pre_get+0x25/0xe0
kmemleak: [<c113bcaf>] proc_register+0x2f/0x1e0
kmemleak: [<c113c17e>] proc_symlink+0x6e/0xb0
kmemleak: [<c15cac41>] proc_root_init+0x60/0xbe
kmemleak: [<c15aca3a>] start_kernel+0x308/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
kmemleak: unreferenced object 0xf7037810 (size 152):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
kmemleak: [<c123b0c4>] idr_pre_get+0x64/0x90
kmemleak: [<c123b115>] ida_pre_get+0x25/0xe0
kmemleak: [<c113bcaf>] proc_register+0x2f/0x1e0
kmemleak: [<c113c17e>] proc_symlink+0x6e/0xb0
kmemleak: [<c15cac41>] proc_root_init+0x60/0xbe
kmemleak: [<c15aca3a>] start_kernel+0x308/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
kmemleak: unreferenced object 0xf7037760 (size 152):
kmemleak: comm "swapper", pid 0, jiffies 4294877296
kmemleak: backtrace:
kmemleak: [<c10e963b>] kmemleak_alloc+0x11b/0x2b0
kmemleak: [<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
kmemleak: [<c123b0c4>] idr_pre_get+0x64/0x90
kmemleak: [<c123b115>] ida_pre_get+0x25/0xe0
kmemleak: [<c113bcaf>] proc_register+0x2f/0x1e0
kmemleak: [<c113c17e>] proc_symlink+0x6e/0xb0
kmemleak: [<c15cac41>] proc_root_init+0x60/0xbe
kmemleak: [<c15aca3a>] start_kernel+0x308/0x343
kmemleak: [<c15ac088>] __init_begin+0x88/0xa1
kmemleak: [<ffffffff>] 0xffffffff
...

As you can see, I'm not blocking monitoring of any object. I just suppress output of objects with "unwanted"
addresses in stack.

Other useful feature (to my mind) - block according to pid. (Or block "if pid != <the_one_I_want>").

"block=XXXX"/"unblock=XXXX" is _very_ general. I should use block-address/block-function/filter-by-address, etc.
Here is code I wrote today (it's just a concept. Not 'beta' or something like this.).
It's against kmemleak.c without CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE support.

If you like the basic idea - I'll continue my work.
Any comments are highly appreciable.

---
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index c96f2c8..7a20898 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -153,6 +153,14 @@ struct kmemleak_object {
char comm[TASK_COMM_LEN]; /* executable name */
};

+/*
+ * Structure holding the data for each unwanted address (blacklisted function).
+ */
+struct kmemleak_function {
+ unsigned int pointer;
+ struct list_head function_list;
+};
+
/* flag representing the memory block allocation status */
#define OBJECT_ALLOCATED (1 << 0)
/* flag set after the first reporting of an unreference object */
@@ -164,10 +172,14 @@ struct kmemleak_object {
static LIST_HEAD(object_list);
/* the list of gray-colored objects (see color_gray comment below) */
static LIST_HEAD(gray_list);
+/* the list of blacklisted functions */
+static LIST_HEAD(function_list);
/* prio search tree for object boundaries */
static struct prio_tree_root object_tree_root;
/* rw_lock protecting the access to object_list and prio_tree_root */
static DEFINE_RWLOCK(kmemleak_lock);
+/* spinlock protecting the access to function_list*/
+static DEFINE_RWLOCK(function_list_lock);

/* allocation caches for kmemleak internal data */
static struct kmem_cache *object_cache;
@@ -258,6 +270,102 @@ static void kmemleak_disable(void);
kmemleak_disable(); \
} while (0)

+
+static int lookup_function(unsigned long pointer) {
+ struct kmemleak_function *function;
+ unsigned long flags;
+
+ /*Find out if given address (pointer) is in blacklist.
+ *
+ */
+ read_lock_irqsave(&function_list_lock, flags);
+ list_for_each_entry(function, &function_list, function_list) {
+ if( function->pointer == pointer) {
+ pr_info("Function already blacklisted <%p>\n", (void*)pointer);
+ return 1;
+ }
+ }
+ read_unlock_irqrestore(&function_list_lock, flags);
+
+ return 0;
+}
+
+
+static void create_function(unsigned long pointer) {
+ unsigned long flags;
+ struct kmemleak_function *function;
+
+ /*prevent multi-blacklisted*/
+ if( lookup_function(pointer) )
+ return;
+
+ /*Is it ok? Should I allocate in cache (like objects)?*/
+ function = kmalloc(sizeof(struct kmemleak_function), GFP_KERNEL & GFP_KMEMLEAK_MASK);
+ if( !function ) {
+ kmemleak_warn("Cannot allocate a kmemleak_function structure\n");
+ return;
+ }
+
+ function->pointer = pointer;
+ INIT_LIST_HEAD(&function->function_list);
+
+ write_lock_irqsave(&function_list_lock, flags);
+ list_add(&function->function_list, &function_list);
+ write_unlock_irqrestore(&function_list_lock, flags);
+
+ pr_info("Added to blacklist: <%p>\n", (void*)pointer);
+}
+
+
+static void delete_function(unsigned long pointer) {
+ unsigned long flags;
+ struct kmemleak_function *function;
+
+ write_lock_irqsave(&function_list_lock, flags);
+
+ list_for_each_entry(function, &function_list, function_list) {
+ if( function->pointer == pointer ) {
+ list_del(&function->function_list);
+ /*Again. Is it ok?*/
+ kfree(function);
+ write_unlock_irqrestore(&function_list_lock, flags);
+
+ pr_info("Removed from blacklist <%p>\n", (void*)pointer);
+ return;
+ }
+ }
+
+ write_unlock_irqrestore(&function_list_lock, flags);
+}
+
+
+static int match_function(const unsigned long *trace , int trace_len) {
+ int i;
+ unsigned long flags;
+ struct kmemleak_function *function;
+
+ read_lock_irqsave(&function_list_lock, flags);
+ /*Most objects usually have more than 10 pointers in stack.
+ *I don't think that blacklisted count normally will be more that 10 functions.
+ *So it's better to inner-loop on blacklist.
+ */
+ list_for_each_entry(function, &function_list, function_list) {
+ for(i = 0; i < trace_len; i++) {
+ if( function->pointer == trace[i]) {
+ read_unlock_irqrestore(&function_list_lock, flags);
+ pr_info("Function blacklisted <%p>\n", (void*)trace[i]);
+ return 1;
+ }
+ }
+ }
+
+ read_unlock_irqrestore(&function_list_lock, flags);
+
+ return 0;
+}
+
+
+
/*
* Object colors, encoded with count and min_count:
* - white - orphan object, not enough references to it (count < min_count)
@@ -321,6 +429,15 @@ static void print_unreferenced(struct seq_file *seq,
struct kmemleak_object *object)
{
int i;
+ void *ptr;
+
+ /*The basic idea is to stop printing object's stack with blacklisted
+ *function(s). As soon as we whitelist (unblock) function, we'll get this info
+ *again.
+ */
+ if( match_function(object->trace, object->trace_len) )
+ return;
+

print_helper(seq, "unreferenced object 0x%08lx (size %zu):\n",
object->pointer, object->size);
@@ -329,7 +446,7 @@ static void print_unreferenced(struct seq_file *seq,
print_helper(seq, " backtrace:\n");

for (i = 0; i < object->trace_len; i++) {
- void *ptr = (void *)object->trace[i];
+ ptr = (void *)object->trace[i];
print_helper(seq, " [<%p>] %pS\n", ptr, ptr);
}
}
@@ -1303,6 +1420,33 @@ static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
start_scan_thread();
else if (strncmp(buf, "scan=off", 8) == 0)
stop_scan_thread();
+ else if (strncmp(buf, "block=", 6) == 0) {
+ unsigned long pointer;
+ int err;
+
+ /*or we can force user to pass xADDRESS (base = 0).
+ *basically, address will be given as simple copy-paste
+ *from print_XXX stack <%p> (I think so).
+ */
+ err = strict_strtoul(buf + 6, 16, &pointer);
+ if (err < 0)
+ return err;
+
+ create_function(pointer);
+ }
+ else if (strncmp(buf, "unblock=", 8) ==0 ) {
+ unsigned long pointer;
+ int err;
+
+ /*
+ *Read comment for block=XXX.
+ */
+ err = strict_strtoul(buf + 8, 16, &pointer);
+ if (err < 0)
+ return err;
+
+ delete_function(pointer);
+ }
else if (strncmp(buf, "scan=", 5) == 0) {
unsigned long secs;
int err;
@@ -1338,8 +1482,11 @@ static const struct file_operations kmemleak_fops = {
*/
static int kmemleak_cleanup_thread(void *arg)
{
+ unsigned long flags;
struct kmemleak_object *object;
+ struct kmemleak_function *function;

+
mutex_lock(&kmemleak_mutex);
stop_scan_thread();
mutex_unlock(&kmemleak_mutex);
@@ -1349,6 +1496,15 @@ static int kmemleak_cleanup_thread(void *arg)
list_for_each_entry_rcu(object, &object_list, object_list)
delete_object(object->pointer);
rcu_read_unlock();
+
+ write_lock_irqsave(&function_list_lock, flags);
+ list_for_each_entry(function, &function_list, function_list) {
+ list_del(&function->function_list);
+ kfree(function);
+ }
+
+ write_unlock_irqrestore(&function_list_lock, flags);
+
mutex_unlock(&scan_mutex);

return 0;


Thanks,
Sergey


2009-06-26 06:59:46

by Ingo Molnar

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)


* Sergey Senozhatsky <[email protected]> wrote:

> Hello.
>
> Currently kmemleak prints info about all objects. I guess
> sometimes kmemleak gives you more than you actually need.

It prints _a lot_ of info and spams the syslog. I lost crash info a
few days ago due to that: by the time i inspected a crashed machine
the tons of kmemleak output scrolled out the crash from the dmesg
buffer.

This is not acceptable.

Instead it should perhaps print _at most_ a single line every few
minutes, printing a summary about _how many_ leaked entries it
suspects, and should offer a /debug/mm/kmemleak style of file where
the entries can be read out from.

Ok?

Ingo

2009-06-26 07:07:53

by Pekka Enberg

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

Hi Ingo,

* Sergey Senozhatsky <[email protected]> wrote:
>> Currently kmemleak prints info about all objects. I guess
>> sometimes kmemleak gives you more than you actually need.

On Fri, Jun 26, 2009 at 9:59 AM, Ingo Molnar<[email protected]> wrote:
> It prints _a lot_ of info and spams the syslog. I lost crash info a
> few days ago due to that: by the time i inspected a crashed machine
> the tons of kmemleak output scrolled out the crash from the dmesg
> buffer.
>
> This is not acceptable.
>
> Instead it should perhaps print _at most_ a single line every few
> minutes, printing a summary about _how many_ leaked entries it
> suspects, and should offer a /debug/mm/kmemleak style of file where
> the entries can be read out from.

Yup, makes tons of sense.

Pekka

2009-06-26 07:47:24

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On (06/26/09 08:59), Ingo Molnar wrote:
> Date: Fri, 26 Jun 2009 08:59:23 +0200
> From: Ingo Molnar <[email protected]>
> To: Sergey Senozhatsky <[email protected]>
> Cc: Catalin Marinas <[email protected]>,
> Pekka Enberg <[email protected]>,
> "Paul E. McKenney" <[email protected]>,
> Andrew Morton <[email protected]>,
> [email protected], [email protected]
> Subject: Re: kmemleak suggestion (long message)
> User-Agent: Mutt/1.5.18 (2008-05-17)
>
>
> * Sergey Senozhatsky <[email protected]> wrote:
>
> > Hello.
> >
> > Currently kmemleak prints info about all objects. I guess
> > sometimes kmemleak gives you more than you actually need.
>
> It prints _a lot_ of info and spams the syslog. I lost crash info a
> few days ago due to that: by the time i inspected a crashed machine
> the tons of kmemleak output scrolled out the crash from the dmesg
> buffer.
>
> This is not acceptable.
>
Agreed.

> Instead it should perhaps print _at most_ a single line every few
> minutes, printing a summary about _how many_ leaked entries it
> suspects, and should offer a /debug/mm/kmemleak style of file where
> the entries can be read out from.
>
> Ok?
>
> Ingo
>
Agreed.

Sergey

2009-06-26 08:13:40

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On (06/26/09 10:07), Pekka Enberg wrote:
> > This is not acceptable.
> >
> > Instead it should perhaps print _at most_ a single line every few
> > minutes, printing a summary about _how many_ leaked entries it
> > suspects, and should offer a /debug/mm/kmemleak style of file where
> > the entries can be read out from.
>
> Yup, makes tons of sense.
>
> Pekka
>

Hello Pekka,
What do you about suggested ability to filter/block "unwanted" reports?
IMHO it makes sense.

Sergey

2009-06-26 08:18:04

by Pekka Enberg

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

Hi Sergey,

On (06/26/09 10:07), Pekka Enberg wrote:
> > > This is not acceptable.
> > >
> > > Instead it should perhaps print _at most_ a single line every few
> > > minutes, printing a summary about _how many_ leaked entries it
> > > suspects, and should offer a /debug/mm/kmemleak style of file where
> > > the entries can be read out from.
> >
> > Yup, makes tons of sense.

On Fri, 2009-06-26 at 11:14 +0300, Sergey Senozhatsky wrote:
> What do you about suggested ability to filter/block "unwanted" reports?
> IMHO it makes sense.

Well, the thing is, I am not sure it's needed if we implement Ingo's
suggestion. After all, syslog is no longer spammed very hard and you can
do all the filtering in userspace when you read /debug/mm/kmemleak file,
no?

Pekka

2009-06-26 08:26:17

by Catalin Marinas

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On Fri, 2009-06-26 at 08:59 +0200, Ingo Molnar wrote:
> * Sergey Senozhatsky <[email protected]> wrote:
> > Currently kmemleak prints info about all objects. I guess
> > sometimes kmemleak gives you more than you actually need.
>
> It prints _a lot_ of info and spams the syslog. I lost crash info a
> few days ago due to that: by the time i inspected a crashed machine
> the tons of kmemleak output scrolled out the crash from the dmesg
> buffer.
>
> This is not acceptable.
>
> Instead it should perhaps print _at most_ a single line every few
> minutes, printing a summary about _how many_ leaked entries it
> suspects, and should offer a /debug/mm/kmemleak style of file where
> the entries can be read out from.

I agree as well. It already provides the /sys/kernel/debug/kmemleak
which triggers a scan and shows possible leaks. That's easily fixable.

BTW, this was questioned in the past as well - do we still need the
automatic scanning from a kernel thread? Can a user cron job just read
the kmemleak file?

--
Catalin

2009-06-26 08:28:11

by Pekka Enberg

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On Fri, 2009-06-26 at 09:25 +0100, Catalin Marinas wrote:
> BTW, this was questioned in the past as well - do we still need the
> automatic scanning from a kernel thread? Can a user cron job just read
> the kmemleak file?

I think the kernel thread makes sense so that we get an early warning in
syslog. Ingo, what's your take on this from autoqa point of view?

Pekka

2009-06-26 08:41:52

by Catalin Marinas

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On Fri, 2009-06-26 at 11:27 +0300, Pekka Enberg wrote:
> On Fri, 2009-06-26 at 09:25 +0100, Catalin Marinas wrote:
> > BTW, this was questioned in the past as well - do we still need the
> > automatic scanning from a kernel thread? Can a user cron job just read
> > the kmemleak file?
>
> I think the kernel thread makes sense so that we get an early warning in
> syslog.

If we keep the automatic scanning I could also change the code so that
the debug/kmemleak file only shows what was found during the thread
scanning rather than trigger a new scan (this could be forced with
something like echo "scan=now" > debug/kmemleak).

--
Catalin

2009-06-26 08:43:04

by Ingo Molnar

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)


* Pekka Enberg <[email protected]> wrote:

> On Fri, 2009-06-26 at 09:25 +0100, Catalin Marinas wrote:
>
> > BTW, this was questioned in the past as well - do we still need
> > the automatic scanning from a kernel thread? Can a user cron job
> > just read the kmemleak file?
>
> I think the kernel thread makes sense so that we get an early
> warning in syslog. Ingo, what's your take on this from autoqa
> point of view?

it would be nice to have more relevant messages. Many of the
messages seem false positives, right? So it would be nice to
constrain kmemleak into a mode of operation that makes its
backtraces worth looking at. A message about suspected leaks is
definitely useful, it just shouldnt be printed too frequently.

Ingo

2009-06-26 08:49:30

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On (06/26/09 11:17), Pekka Enberg wrote:
> Well, the thing is, I am not sure it's needed if we implement Ingo's
> suggestion. After all, syslog is no longer spammed very hard and you can
> do all the filtering in userspace when you read /debug/mm/kmemleak file,
> no?
>
> Pekka
>

Well, we just move 'spam' out of syslog. Not dealing with 'spam' itself.
I'm not sure about 'filtering in userspace when you read'. Suppose I use
'tail -f /debug/mm/kmemleak'. How can I easy suppress printing of (for example):

[ 64.494396] kmemleak: unreferenced object 0xf63fee18 (size 32):
[ 64.494400] kmemleak: comm "init", pid 1, jiffies 4294879195
[ 64.494402] kmemleak: backtrace:
[ 64.494408] kmemleak: [<c10e92fb>] kmemleak_alloc+0x11b/0x2b0
[ 64.494412] kmemleak: [<c10e4b91>] kmem_cache_alloc+0x111/0x1c0
[ 64.494418] kmemleak: [<c12cf49b>] tty_ldisc_try_get+0x2b/0x130
[ 64.494422] kmemleak: [<c12cf7d7>] tty_ldisc_get+0x37/0x70
[ 64.494426] kmemleak: [<c12cfa54>] tty_ldisc_reinit+0x34/0x70
[ 64.494431] kmemleak: [<c12cfac5>] tty_ldisc_release+0x35/0x60
[ 64.494435] kmemleak: [<c12ca1fe>] tty_release_dev+0x33e/0x500
[ 64.494439] kmemleak: [<c12ca3e0>] tty_release+0x20/0x40
[ 64.494443] kmemleak: [<c10ed5fd>] __fput+0xed/0x200
[ 64.494446] kmemleak: [<c10ed732>] fput+0x22/0x40
[ 64.494450] kmemleak: [<c10e96a9>] filp_close+0x49/0x90
[ 64.494454] kmemleak: [<c10e9758>] sys_close+0x68/0xc0
[ 64.494459] kmemleak: [<c100324b>] sysenter_do_call+0x12/0x22
[ 64.494467] kmemleak: [<ffffffff>] 0xffffffff

Or any report with tty_ldisc_try_get (ppp generates tons of them).
echo "block=c12cf49b" > /sys/.../kmemleak looks good to me.

Sergey

2009-06-26 08:54:57

by Catalin Marinas

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On Fri, 2009-06-26 at 11:50 +0300, Sergey Senozhatsky wrote:
> On (06/26/09 11:17), Pekka Enberg wrote:
> > Well, the thing is, I am not sure it's needed if we implement Ingo's
> > suggestion. After all, syslog is no longer spammed very hard and you can
> > do all the filtering in userspace when you read /debug/mm/kmemleak file,
> > no?
>
> Well, we just move 'spam' out of syslog. Not dealing with 'spam' itself.
> I'm not sure about 'filtering in userspace when you read'. Suppose I use
> 'tail -f /debug/mm/kmemleak'. How can I easy suppress printing of (for example):

I don't have a strong opinion on this patch at the moment, I'll have a
look later today.

> Or any report with tty_ldisc_try_get (ppp generates tons of them).

BTW, that's a real leak IMHO (posted a patch yesterday in reply to the
initial report to Alan Cox).

--
Catalin

2009-06-26 09:11:58

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On (06/26/09 09:54), Catalin Marinas wrote:
> > Well, we just move 'spam' out of syslog. Not dealing with 'spam' itself.
> > I'm not sure about 'filtering in userspace when you read'. Suppose I use
> > 'tail -f /debug/mm/kmemleak'. How can I easy suppress printing of (for example):
>
> I don't have a strong opinion on this patch at the moment, I'll have a
> look later today.
>
Well, take a look please.
I find it (suggested mechanism) to be useful.

> > Or any report with tty_ldisc_try_get (ppp generates tons of them).
>
> BTW, that's a real leak IMHO (posted a patch yesterday in reply to the
> initial report to Alan Cox).
>
I'll try to find link to see it. Since tty_ldisc_try_get reports started
to 'annoy' me.

> --
> Catalin
>

Sergey

2009-06-26 09:19:22

by Alan

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On Fri, 26 Jun 2009 09:54:23 +0100
Catalin Marinas <[email protected]> wrote:

> On Fri, 2009-06-26 at 11:50 +0300, Sergey Senozhatsky wrote:
> > On (06/26/09 11:17), Pekka Enberg wrote:
> > > Well, the thing is, I am not sure it's needed if we implement Ingo's
> > > suggestion. After all, syslog is no longer spammed very hard and you can
> > > do all the filtering in userspace when you read /debug/mm/kmemleak file,
> > > no?
> >
> > Well, we just move 'spam' out of syslog. Not dealing with 'spam' itself.
> > I'm not sure about 'filtering in userspace when you read'. Suppose I use
> > 'tail -f /debug/mm/kmemleak'. How can I easy suppress printing of (for example):
>
> I don't have a strong opinion on this patch at the moment, I'll have a
> look later today.
>
> > Or any report with tty_ldisc_try_get (ppp generates tons of them).
>
> BTW, that's a real leak IMHO (posted a patch yesterday in reply to the
> initial report to Alan Cox).

Yes its on my todo list - probably for early next week along with nailing
down a couple of warnings suggesting there is another race lurking
somewhere.

2009-06-26 16:13:20

by Catalin Marinas

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On Fri, 2009-06-26 at 11:50 +0300, Sergey Senozhatsky wrote:
> On (06/26/09 11:17), Pekka Enberg wrote:
> > Well, the thing is, I am not sure it's needed if we implement Ingo's
> > suggestion. After all, syslog is no longer spammed very hard and you can
> > do all the filtering in userspace when you read /debug/mm/kmemleak file,
> > no?
>
> Well, we just move 'spam' out of syslog. Not dealing with 'spam' itself.
> I'm not sure about 'filtering in userspace when you read'. Suppose I use
> 'tail -f /debug/mm/kmemleak'. How can I easy suppress printing of (for example):

I had a look at your patch and I tend to agree with Pekka. It really
adds too much complexity for something that could be easily done in user
space (could be more concise or even written in perl, awk, sed, python
etc.):

cat /sys/kernel/debug/kmemleak | tr "\n" "#" \
| sed -e "s/#unreferenced/\nunreferenced/g" \
| grep -v "tty_ldisc_try_get" | tr "#" "\n"

It would have made sense with the output in syslog but I just removed
this feature.

As for "tail -f", I'm not sure it would work anyway because of the way
the seqfile content is generated. New detected leaks aren't necessarily
appended to the kmemleak file. They are always listed in the order they
were allocated.

Thanks anyway.

--
Catalin

2009-06-26 22:46:35

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: kmemleak suggestion (long message)

On (06/26/09 17:12), Catalin Marinas wrote:
> I had a look at your patch and I tend to agree with Pekka. It really
> adds too much complexity for something that could be easily done in user
> space (could be more concise or even written in perl, awk, sed, python
> etc.):
>
> cat /sys/kernel/debug/kmemleak | tr "\n" "#" \
> | sed -e "s/#unreferenced/\nunreferenced/g" \
> | grep -v "tty_ldisc_try_get" | tr "#" "\n"
>
Well, it's hardly can be compared with
echo "block=ADDRESS_FROM_STACK" > /.../kmemleak

Frankly, I still found it useful (as you don't have to write in perl, awk, sed, python etc
to see 50 lines (you are interested in) out of 1000.
You just watching reports.)

(well, maybe not as useful as with syslog.)

Anyway, the decision is yours. And let it be so.
Thanks.

> Thanks anyway.
>
> --
> Catalin
>

Sergey