diff mbox series

[RFC] mm: sort kmemleak object via backtrace

Message ID 1664264570-3716-1-git-send-email-zhaoyang.huang@unisoc.com (mailing list archive)
State New
Headers show
Series [RFC] mm: sort kmemleak object via backtrace | expand

Commit Message

zhaoyang.huang Sept. 27, 2022, 7:42 a.m. UTC
From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>

Kmemleak objects are reported each which could produce lot of redundant
backtrace informations. introduce a set of method to establish a hash
tree to sort the objects according to backtrace.

results:
[  579.075111]c6 [ T5491] kmemleak: unreferenced object 0xffffff80badd9e00 (size 128):
[  579.082734]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
[  579.096837]c6 [ T5491] kmemleak: unreferenced object 0xffffff80badd9d00 (size 128):
[  579.104435]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
[  579.118563]c6 [ T5491] kmemleak: unreferenced object 0xffffff80baddce80 (size 128):
[  579.126201]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
[  579.140303]c6 [ T5491] kmemleak: unreferenced object 0xffffff80baddcb00 (size 128):
[  579.147906]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
[  579.162032]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae74a80 (size 128):
[  579.169661]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
[  579.183775]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae74100 (size 128):
[  579.191374]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892471
[  579.205486]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae75880 (size 128):
[  579.213127]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892471
[  579.227743]c6 [ T5491] kmemleak:   backtrace:
[  579.232109]c6 [ T5491] kmemleak:     [<0000000066492d96>] __kmalloc_track_caller+0x1d4/0x3e0
[  579.240506]c6 [ T5491] kmemleak:     [<00000000e5400df8>] kstrdup_const+0x6c/0xa4
[  579.247930]c6 [ T5491] kmemleak:     [<00000000d7843951>] __kernfs_new_node+0x5c/0x1dc
[  579.255830]c6 [ T5491] kmemleak:     [<0000000073b5a7bd>] kernfs_new_node+0x60/0xc4
[  579.263436]c6 [ T5491] kmemleak:     [<000000002c7a48d5>] __kernfs_create_file+0x60/0xfc
[  579.271485]c6 [ T5491] kmemleak:     [<00000000260ae4a1>] cgroup_addrm_files+0x244/0x4b0
[  579.279534]c6 [ T5491] kmemleak:     [<00000000ec6bce51>] css_populate_dir+0xb4/0x13c
[  579.287324]c6 [ T5491] kmemleak:     [<000000005913d698>] cgroup_mkdir+0x1e0/0x31c
[  579.294859]c6 [ T5491] kmemleak:     [<0000000052605ead>] kernfs_iop_mkdir.llvm.8836999160598622324+0xb0/0x168
[  579.304817]c6 [ T5491] kmemleak:     [<0000000009665bc4>] vfs_mkdir+0xec/0x170
[  579.311990]c6 [ T5491] kmemleak:     [<000000003c9c94c1>] do_mkdirat+0xa4/0x168
[  579.319279]c6 [ T5491] kmemleak:     [<000000005dd5be19>] __arm64_sys_mkdirat+0x28/0x38
[  579.327242]c6 [ T5491] kmemleak:     [<000000005a0b9381>] el0_svc_common+0xb4/0x188
[  579.334868]c6 [ T5491] kmemleak:     [<0000000063586a51>] el0_svc_handler+0x2c/0x3c
[  579.342472]c6 [ T5491] kmemleak:     [<00000000edfd67aa>] el0_svc+0x8/0x100

Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
---
 mm/kmemleak.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+), 1 deletion(-)

Comments

Zhaoyang Huang Sept. 27, 2022, 7:53 a.m. UTC | #1
I am also working on introduce these set of method on SLAB_STORE_USER
which could save a certain amount of memory for slub debug

On Tue, Sep 27, 2022 at 3:46 PM zhaoyang.huang
<zhaoyang.huang@unisoc.com> wrote:
>
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>
> Kmemleak objects are reported each which could produce lot of redundant
> backtrace informations. introduce a set of method to establish a hash
> tree to sort the objects according to backtrace.
>
> results:
> [  579.075111]c6 [ T5491] kmemleak: unreferenced object 0xffffff80badd9e00 (size 128):
> [  579.082734]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.096837]c6 [ T5491] kmemleak: unreferenced object 0xffffff80badd9d00 (size 128):
> [  579.104435]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.118563]c6 [ T5491] kmemleak: unreferenced object 0xffffff80baddce80 (size 128):
> [  579.126201]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.140303]c6 [ T5491] kmemleak: unreferenced object 0xffffff80baddcb00 (size 128):
> [  579.147906]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.162032]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae74a80 (size 128):
> [  579.169661]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.183775]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae74100 (size 128):
> [  579.191374]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892471
> [  579.205486]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae75880 (size 128):
> [  579.213127]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892471
> [  579.227743]c6 [ T5491] kmemleak:   backtrace:
> [  579.232109]c6 [ T5491] kmemleak:     [<0000000066492d96>] __kmalloc_track_caller+0x1d4/0x3e0
> [  579.240506]c6 [ T5491] kmemleak:     [<00000000e5400df8>] kstrdup_const+0x6c/0xa4
> [  579.247930]c6 [ T5491] kmemleak:     [<00000000d7843951>] __kernfs_new_node+0x5c/0x1dc
> [  579.255830]c6 [ T5491] kmemleak:     [<0000000073b5a7bd>] kernfs_new_node+0x60/0xc4
> [  579.263436]c6 [ T5491] kmemleak:     [<000000002c7a48d5>] __kernfs_create_file+0x60/0xfc
> [  579.271485]c6 [ T5491] kmemleak:     [<00000000260ae4a1>] cgroup_addrm_files+0x244/0x4b0
> [  579.279534]c6 [ T5491] kmemleak:     [<00000000ec6bce51>] css_populate_dir+0xb4/0x13c
> [  579.287324]c6 [ T5491] kmemleak:     [<000000005913d698>] cgroup_mkdir+0x1e0/0x31c
> [  579.294859]c6 [ T5491] kmemleak:     [<0000000052605ead>] kernfs_iop_mkdir.llvm.8836999160598622324+0xb0/0x168
> [  579.304817]c6 [ T5491] kmemleak:     [<0000000009665bc4>] vfs_mkdir+0xec/0x170
> [  579.311990]c6 [ T5491] kmemleak:     [<000000003c9c94c1>] do_mkdirat+0xa4/0x168
> [  579.319279]c6 [ T5491] kmemleak:     [<000000005dd5be19>] __arm64_sys_mkdirat+0x28/0x38
> [  579.327242]c6 [ T5491] kmemleak:     [<000000005a0b9381>] el0_svc_common+0xb4/0x188
> [  579.334868]c6 [ T5491] kmemleak:     [<0000000063586a51>] el0_svc_handler+0x2c/0x3c
> [  579.342472]c6 [ T5491] kmemleak:     [<00000000edfd67aa>] el0_svc+0x8/0x100
>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> ---
>  mm/kmemleak.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 1eddc01..34134e5 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -102,6 +102,8 @@
>  #include <linux/kfence.h>
>  #include <linux/kmemleak.h>
>  #include <linux/memory_hotplug.h>
> +#include <linux/jhash.h>
> +#include <linux/random.h>
>
>  /*
>   * Kmemleak configuration and common defines.
> @@ -143,6 +145,7 @@ struct kmemleak_object {
>         unsigned int flags;             /* object status flags */
>         struct list_head object_list;
>         struct list_head gray_list;
> +       struct list_head report_list;
>         struct rb_node rb_node;
>         struct rcu_head rcu;            /* object_list lockless traversal */
>         /* object usage count; object freed when use_count == 0 */
> @@ -161,11 +164,18 @@ struct kmemleak_object {
>         struct hlist_head area_list;
>         unsigned long trace[MAX_TRACE];
>         unsigned int trace_len;
> +       u32 trace_hash;
>         unsigned long jiffies;          /* creation timestamp */
>         pid_t pid;                      /* pid of the current task */
>         char comm[TASK_COMM_LEN];       /* executable name */
>  };
>
> +struct kmemleak_report {
> +       u32 trace_hash;
> +       struct list_head report_list_head;
> +       struct rb_node rb_node_report;
> +       struct list_head *report_pos;
> +};
>  /* flag representing the memory block allocation status */
>  #define OBJECT_ALLOCATED       (1 << 0)
>  /* flag set after the first reporting of an unreference object */
> @@ -195,10 +205,13 @@ struct kmemleak_object {
>  static struct kmemleak_object mem_pool[CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE];
>  static int mem_pool_free_count = ARRAY_SIZE(mem_pool);
>  static LIST_HEAD(mem_pool_free_list);
> +static struct kmemleak_report kr_pool[CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE];
> +static int kr_pool_free_count = ARRAY_SIZE(kr_pool);
>  /* search tree for object boundaries */
>  static struct rb_root object_tree_root = RB_ROOT;
>  /* search tree for object (with OBJECT_PHYS flag) boundaries */
>  static struct rb_root object_phys_tree_root = RB_ROOT;
> +static struct rb_root object_report_tree_root = RB_ROOT;
>  /* protecting the access to object_list, object_tree_root (or object_phys_tree_root) */
>  static DEFINE_RAW_SPINLOCK(kmemleak_lock);
>
> @@ -241,6 +254,7 @@ struct kmemleak_object {
>
>  static void kmemleak_disable(void);
>
> +static u32 trace_seed;
>  /*
>   * Print a warning and dump the stack trace.
>   */
> @@ -423,6 +437,46 @@ static struct kmemleak_object *lookup_object(unsigned long ptr, int alias)
>         return __lookup_object(ptr, alias, false);
>  }
>
> +void print_unreferenced_object_list(struct seq_file *seq)
> +{
> +       unsigned long flags;
> +       struct kmemleak_object *object;
> +       struct kmemleak_report *object_report;
> +       int i;
> +       struct rb_node *rb;
> +       bool reported_flag = false;
> +
> +       rcu_read_lock();
> +       raw_spin_lock_irqsave(&kmemleak_lock, flags);
> +       for (rb = rb_first(&object_report_tree_root); rb; rb = rb_next(rb)) {
> +               object_report = rb_entry(rb, struct kmemleak_report, rb_node_report);
> +               if (object_report && !list_empty(&object_report->report_list_head)) {
> +                       list_for_each_entry_rcu(object, &object_report->report_list_head, report_list) {
> +                               raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
> +                               raw_spin_lock_irqsave(&kmemleak_lock, flags);
> +                               if (unreferenced_object(object) &&
> +                                               (object->flags & OBJECT_REPORTED)) {
> +                                       reported_flag = true;
> +                                       warn_or_seq_printf(seq, "unreferenced object 0x%08lx (size %zu):\n",
> +                                                       object->pointer, object->size);
> +                                       warn_or_seq_printf(seq, "  comm \"%s\", pid %d, jiffies %lu\n",
> +                                                       object->comm, object->pid, object->jiffies);
> +                               }
> +                       }
> +                       if (reported_flag) {
> +                               warn_or_seq_printf(seq, "  backtrace:\n");
> +                               object = list_prev_entry(object, report_list);
> +                               for (i = 0; i < object->trace_len; i++) {
> +                                       void *ptr = (void *)object->trace[i];
> +                                       warn_or_seq_printf(seq, "    [<%p>] %pS\n", ptr, ptr);
> +                               }
> +                               reported_flag = false;
> +                       }
> +               }
> +       }
> +       raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
> +       rcu_read_unlock();
> +}
>  /*
>   * Increment the object use_count. Return 1 if successful or 0 otherwise. Note
>   * that once an object's use_count reached 0, the RCU freeing was already
> @@ -568,6 +622,7 @@ static void __remove_object(struct kmemleak_object *object)
>                                    &object_phys_tree_root :
>                                    &object_tree_root);
>         list_del_rcu(&object->object_list);
> +       list_del_rcu(&object->report_list);
>  }
>
>  /*
> @@ -610,9 +665,11 @@ static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
>  {
>         unsigned long flags;
>         struct kmemleak_object *object, *parent;
> +       struct kmemleak_report *report, *report_parent;
>         struct rb_node **link, *rb_parent;
>         unsigned long untagged_ptr;
>         unsigned long untagged_objp;
> +       u32 trace_hash;
>
>         object = mem_pool_alloc(gfp);
>         if (!object) {
> @@ -623,6 +680,7 @@ static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
>
>         INIT_LIST_HEAD(&object->object_list);
>         INIT_LIST_HEAD(&object->gray_list);
> +       INIT_LIST_HEAD(&object->report_list);
>         INIT_HLIST_HEAD(&object->area_list);
>         raw_spin_lock_init(&object->lock);
>         atomic_set(&object->use_count, 1);
> @@ -655,6 +713,7 @@ static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
>
>         /* kernel backtrace */
>         object->trace_len = __save_stack_trace(object->trace);
> +       object->trace_hash = jhash2((const u32 *)object->trace, object->trace_len / sizeof(u32), trace_seed);
>
>         raw_spin_lock_irqsave(&kmemleak_lock, flags);
>
> @@ -694,8 +753,42 @@ static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
>         rb_link_node(&object->rb_node, rb_parent, link);
>         rb_insert_color(&object->rb_node, is_phys ? &object_phys_tree_root :
>                                           &object_tree_root);
> -
>         list_add_tail_rcu(&object->object_list, &object_list);
> +
> +       raw_spin_lock_irqsave(&kmemleak_lock, flags);
> +       link = &object_report_tree_root.rb_node;
> +       rb_parent = NULL;
> +       while (*link) {
> +               raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
> +               raw_spin_lock_irqsave(&kmemleak_lock, flags);
> +               rb_parent = *link;
> +               report_parent = rb_entry(rb_parent, struct kmemleak_report, rb_node_report);
> +               trace_hash = report_parent->trace_hash;
> +               if (object->trace_hash < trace_hash)
> +                       link = &report_parent->rb_node_report.rb_left;
> +               else if (trace_hash < object->trace_hash)
> +                       link = &report_parent->rb_node_report.rb_right;
> +               else {
> +                       list_add_tail_rcu(&object->report_list, &report_parent->report_list_head);
> +                       goto out;
> +               }
> +       }
> +       report = kr_pool_free_count ? &kr_pool[--kr_pool_free_count] : NULL;
> +       if (!report)
> +               goto out;
> +       report->trace_hash = object->trace_hash;
> +       /*
> +        * report is the 1st node represent this trace_hash, init its list_head and
> +        * insert it to object_report_tree_root
> +        */
> +       INIT_LIST_HEAD(&report->report_list_head);
> +       /* record 1st object to the list*/
> +       list_add_tail_rcu(&object->report_list, &report->report_list_head);
> +       /* initialize the report_pos as report_list_head*/
> +       report->report_pos = &report->report_list_head;
> +       /* add the node to rb tree*/
> +       rb_link_node(&report->rb_node_report, rb_parent, link);
> +       rb_insert_color(&report->rb_node_report, &object_report_tree_root);
>  out:
>         raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
>         return object;
> @@ -1947,6 +2040,8 @@ static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
>                 kmemleak_scan();
>         else if (strncmp(buf, "dump=", 5) == 0)
>                 ret = dump_str_object_info(buf + 5);
> +       else if (strncmp(buf, "report", 6) == 0)
> +               print_unreferenced_object_list(NULL);
>         else
>                 ret = -EINVAL;
>
> @@ -2080,6 +2175,7 @@ void __init kmemleak_init(void)
>                 create_object((unsigned long)__start_ro_after_init,
>                               __end_ro_after_init - __start_ro_after_init,
>                               KMEMLEAK_GREY, GFP_ATOMIC);
> +       get_random_bytes(&trace_seed, 4);
>  }
>
>  /*
> --
> 1.9.1
>
Yujie Liu Oct. 7, 2022, 1:23 p.m. UTC | #2
Greeting,

FYI, we noticed the following commit (built with gcc-11):

commit: 0662227fdfe7b22c8a88e4ef64eedf549770cb59 ("[RFC PATCH] mm: sort kmemleak object via backtrace")
url: https://github.com/intel-lab-lkp/linux/commits/zhaoyang-huang/mm-sort-kmemleak-object-via-backtrace/20220927-154444
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git a1375562c0a87f0fa2eaf3e8ce15824696d4170a
patch link: https://lore.kernel.org/linux-mm/1664264570-3716-1-git-send-email-zhaoyang.huang@unisoc.com

in testcase: boot

on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


[    0.016452][    T0] WARNING: possible recursive locking detected
[    0.017183][    T0] 6.0.0-rc7-00035-g0662227fdfe7 #1 Not tainted
[    0.017895][    T0] --------------------------------------------
[    0.018581][    T0] swapper/0 is trying to acquire lock:
[ 0.019237][ T0] 82d83e70 (kmemleak_lock){....}-{2:2}, at: __create_object+0x2de/0x630 
[    0.020339][    T0]
[    0.020339][    T0] but task is already holding lock:
[ 0.021223][ T0] 82d83e70 (kmemleak_lock){....}-{2:2}, at: __create_object+0x1db/0x630 
[    0.022321][    T0]
[    0.022321][    T0] other info that might help us debug this:
[    0.023277][    T0]  Possible unsafe locking scenario:
[    0.023277][    T0]
[    0.024129][    T0]        CPU0
[    0.024496][    T0]        ----
[    0.024866][    T0]   lock(kmemleak_lock);
[    0.025390][    T0]   lock(kmemleak_lock);
[    0.025887][    T0]
[    0.025887][    T0]  *** DEADLOCK ***
[    0.025887][    T0]
[    0.026877][    T0]  May be due to missing lock nesting notation
[    0.026877][    T0]
[    0.027818][    T0] 1 lock held by swapper/0:
[ 0.028327][ T0] #0: 82d83e70 (kmemleak_lock){....}-{2:2}, at: __create_object+0x1db/0x630 
[    0.029455][    T0]
[    0.029455][    T0] stack backtrace:
[    0.030161][    T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.0.0-rc7-00035-g0662227fdfe7 #1 ce0142f1816d214291543120a71fbfb037192f5b
[    0.031630][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-4 04/01/2014
[    0.032866][    T0] Call Trace:
[ 0.033238][ T0] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 4)) 
[ 0.033766][ T0] dump_stack (lib/dump_stack.c:114) 
[ 0.034218][ T0] validate_chain.cold (kernel/locking/lockdep.c:2988 kernel/locking/lockdep.c:3031 kernel/locking/lockdep.c:3816) 
[ 0.034830][ T0] __lock_acquire (kernel/locking/lockdep.c:5053) 
[ 0.035418][ T0] lock_acquire (kernel/locking/lockdep.c:466 kernel/locking/lockdep.c:5668) 
[ 0.035940][ T0] ? __create_object+0x2de/0x630 
[ 0.036607][ T0] ? startup_32_smp (arch/x86/kernel/head_32.S:329) 
[ 0.037188][ T0] _raw_spin_lock_irqsave (include/linux/spinlock_api_smp.h:111 kernel/locking/spinlock.c:162) 
[ 0.037845][ T0] ? __create_object+0x2de/0x630 
[ 0.038508][ T0] __create_object+0x2de/0x630 
[ 0.039147][ T0] kmemleak_alloc_phys (mm/kmemleak.c:808 mm/kmemleak.c:1285) 
[ 0.039728][ T0] memblock_alloc_range_nid (mm/memblock.c:1427) 
[ 0.040367][ T0] memblock_phys_alloc_range (mm/memblock.c:1451 (discriminator 3)) 
[ 0.040997][ T0] reserve_real_mode (arch/x86/realmode/init.c:58 (discriminator 3)) 
[ 0.041545][ T0] setup_arch (arch/x86/kernel/setup.c:1180) 
[ 0.042064][ T0] start_kernel (init/main.c:277 (discriminator 3) init/main.c:475 (discriminator 3) init/main.c:953 (discriminator 3)) 
[ 0.042588][ T0] i386_start_kernel (arch/x86/kernel/head32.c:57) 
[ 0.043158][ T0] startup_32_smp (arch/x86/kernel/head_32.S:329) 
BUG: kernel hang in boot stage


If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <yujie.liu@intel.com>
| Link: https://lore.kernel.org/r/202210072108.2d1b9006-yujie.liu@intel.com


To reproduce:

        # build kernel
	cd linux
	cp config-6.0.0-rc7-00035-g0662227fdfe7 .config
	make HOSTCC=gcc-11 CC=gcc-11 ARCH=i386 olddefconfig prepare modules_prepare bzImage modules
	make HOSTCC=gcc-11 CC=gcc-11 ARCH=i386 INSTALL_MOD_PATH=<mod-install-dir> modules_install
	cd <mod-install-dir>
	find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz


        git clone https://github.com/intel/lkp-tests.git
        cd lkp-tests
        bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email

        # if come across any failure that blocks the test,
        # please remove ~/.lkp and /lkp dir to run from a clean state.
Catalin Marinas Oct. 28, 2022, 1:56 p.m. UTC | #3
On Tue, Sep 27, 2022 at 03:42:50PM +0800, zhaoyang.huang wrote:
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> 
> Kmemleak objects are reported each which could produce lot of redundant
> backtrace informations. introduce a set of method to establish a hash
> tree to sort the objects according to backtrace.
> 
> results:
> [  579.075111]c6 [ T5491] kmemleak: unreferenced object 0xffffff80badd9e00 (size 128):
> [  579.082734]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.096837]c6 [ T5491] kmemleak: unreferenced object 0xffffff80badd9d00 (size 128):
> [  579.104435]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.118563]c6 [ T5491] kmemleak: unreferenced object 0xffffff80baddce80 (size 128):
> [  579.126201]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.140303]c6 [ T5491] kmemleak: unreferenced object 0xffffff80baddcb00 (size 128):
> [  579.147906]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.162032]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae74a80 (size 128):
> [  579.169661]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892470
> [  579.183775]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae74100 (size 128):
> [  579.191374]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892471
> [  579.205486]c6 [ T5491] kmemleak: unreferenced object 0xffffff80bae75880 (size 128):
> [  579.213127]c6 [ T5491] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892471
> [  579.227743]c6 [ T5491] kmemleak:   backtrace:
> [  579.232109]c6 [ T5491] kmemleak:     [<0000000066492d96>] __kmalloc_track_caller+0x1d4/0x3e0
> [  579.240506]c6 [ T5491] kmemleak:     [<00000000e5400df8>] kstrdup_const+0x6c/0xa4
> [  579.247930]c6 [ T5491] kmemleak:     [<00000000d7843951>] __kernfs_new_node+0x5c/0x1dc
> [  579.255830]c6 [ T5491] kmemleak:     [<0000000073b5a7bd>] kernfs_new_node+0x60/0xc4
> [  579.263436]c6 [ T5491] kmemleak:     [<000000002c7a48d5>] __kernfs_create_file+0x60/0xfc
> [  579.271485]c6 [ T5491] kmemleak:     [<00000000260ae4a1>] cgroup_addrm_files+0x244/0x4b0
> [  579.279534]c6 [ T5491] kmemleak:     [<00000000ec6bce51>] css_populate_dir+0xb4/0x13c
> [  579.287324]c6 [ T5491] kmemleak:     [<000000005913d698>] cgroup_mkdir+0x1e0/0x31c
> [  579.294859]c6 [ T5491] kmemleak:     [<0000000052605ead>] kernfs_iop_mkdir.llvm.8836999160598622324+0xb0/0x168
> [  579.304817]c6 [ T5491] kmemleak:     [<0000000009665bc4>] vfs_mkdir+0xec/0x170
> [  579.311990]c6 [ T5491] kmemleak:     [<000000003c9c94c1>] do_mkdirat+0xa4/0x168
> [  579.319279]c6 [ T5491] kmemleak:     [<000000005dd5be19>] __arm64_sys_mkdirat+0x28/0x38
> [  579.327242]c6 [ T5491] kmemleak:     [<000000005a0b9381>] el0_svc_common+0xb4/0x188
> [  579.334868]c6 [ T5491] kmemleak:     [<0000000063586a51>] el0_svc_handler+0x2c/0x3c
> [  579.342472]c6 [ T5491] kmemleak:     [<00000000edfd67aa>] el0_svc+0x8/0x100

I'm not convinced it's worth the complexity. Yes, it looks nicer, but if
you have so many leaks they'd not go unnoticed and get fixed relatively
quickly.

One thing I liked about the kmemleak traces is that they are shown in
the order they were allocated. On many occasions, one leak (or false
positive) leaks to another, so it's easier to track them down if you
start with the first. Is the order preserved with your patch?
diff mbox series

Patch

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 1eddc01..34134e5 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -102,6 +102,8 @@ 
 #include <linux/kfence.h>
 #include <linux/kmemleak.h>
 #include <linux/memory_hotplug.h>
+#include <linux/jhash.h>
+#include <linux/random.h>
 
 /*
  * Kmemleak configuration and common defines.
@@ -143,6 +145,7 @@  struct kmemleak_object {
 	unsigned int flags;		/* object status flags */
 	struct list_head object_list;
 	struct list_head gray_list;
+	struct list_head report_list;
 	struct rb_node rb_node;
 	struct rcu_head rcu;		/* object_list lockless traversal */
 	/* object usage count; object freed when use_count == 0 */
@@ -161,11 +164,18 @@  struct kmemleak_object {
 	struct hlist_head area_list;
 	unsigned long trace[MAX_TRACE];
 	unsigned int trace_len;
+	u32 trace_hash;
 	unsigned long jiffies;		/* creation timestamp */
 	pid_t pid;			/* pid of the current task */
 	char comm[TASK_COMM_LEN];	/* executable name */
 };
 
+struct kmemleak_report {
+	u32 trace_hash;
+	struct list_head report_list_head;
+	struct rb_node rb_node_report;
+	struct list_head *report_pos;
+};
 /* flag representing the memory block allocation status */
 #define OBJECT_ALLOCATED	(1 << 0)
 /* flag set after the first reporting of an unreference object */
@@ -195,10 +205,13 @@  struct kmemleak_object {
 static struct kmemleak_object mem_pool[CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE];
 static int mem_pool_free_count = ARRAY_SIZE(mem_pool);
 static LIST_HEAD(mem_pool_free_list);
+static struct kmemleak_report kr_pool[CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE];
+static int kr_pool_free_count = ARRAY_SIZE(kr_pool);
 /* search tree for object boundaries */
 static struct rb_root object_tree_root = RB_ROOT;
 /* search tree for object (with OBJECT_PHYS flag) boundaries */
 static struct rb_root object_phys_tree_root = RB_ROOT;
+static struct rb_root object_report_tree_root = RB_ROOT;
 /* protecting the access to object_list, object_tree_root (or object_phys_tree_root) */
 static DEFINE_RAW_SPINLOCK(kmemleak_lock);
 
@@ -241,6 +254,7 @@  struct kmemleak_object {
 
 static void kmemleak_disable(void);
 
+static u32 trace_seed;
 /*
  * Print a warning and dump the stack trace.
  */
@@ -423,6 +437,46 @@  static struct kmemleak_object *lookup_object(unsigned long ptr, int alias)
 	return __lookup_object(ptr, alias, false);
 }
 
+void print_unreferenced_object_list(struct seq_file *seq)
+{
+	unsigned long flags;
+	struct kmemleak_object *object;
+	struct kmemleak_report *object_report;
+	int i;
+	struct rb_node *rb;
+	bool reported_flag = false;
+
+	rcu_read_lock();
+	raw_spin_lock_irqsave(&kmemleak_lock, flags);
+	for (rb = rb_first(&object_report_tree_root); rb; rb = rb_next(rb)) {
+		object_report = rb_entry(rb, struct kmemleak_report, rb_node_report);
+		if (object_report && !list_empty(&object_report->report_list_head)) {
+			list_for_each_entry_rcu(object, &object_report->report_list_head, report_list) {
+				raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
+				raw_spin_lock_irqsave(&kmemleak_lock, flags);
+				if (unreferenced_object(object) &&
+						(object->flags & OBJECT_REPORTED)) {
+					reported_flag = true;
+					warn_or_seq_printf(seq, "unreferenced object 0x%08lx (size %zu):\n",
+							object->pointer, object->size);
+					warn_or_seq_printf(seq, "  comm \"%s\", pid %d, jiffies %lu\n",
+							object->comm, object->pid, object->jiffies);
+				}
+			}
+			if (reported_flag) {
+				warn_or_seq_printf(seq, "  backtrace:\n");
+				object = list_prev_entry(object, report_list);
+				for (i = 0; i < object->trace_len; i++) {
+					void *ptr = (void *)object->trace[i];
+					warn_or_seq_printf(seq, "    [<%p>] %pS\n", ptr, ptr);
+				}
+				reported_flag = false;
+			}
+		}
+	}
+	raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
+	rcu_read_unlock();
+}
 /*
  * Increment the object use_count. Return 1 if successful or 0 otherwise. Note
  * that once an object's use_count reached 0, the RCU freeing was already
@@ -568,6 +622,7 @@  static void __remove_object(struct kmemleak_object *object)
 				   &object_phys_tree_root :
 				   &object_tree_root);
 	list_del_rcu(&object->object_list);
+	list_del_rcu(&object->report_list);
 }
 
 /*
@@ -610,9 +665,11 @@  static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
 {
 	unsigned long flags;
 	struct kmemleak_object *object, *parent;
+	struct kmemleak_report *report, *report_parent;
 	struct rb_node **link, *rb_parent;
 	unsigned long untagged_ptr;
 	unsigned long untagged_objp;
+	u32 trace_hash;
 
 	object = mem_pool_alloc(gfp);
 	if (!object) {
@@ -623,6 +680,7 @@  static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
 
 	INIT_LIST_HEAD(&object->object_list);
 	INIT_LIST_HEAD(&object->gray_list);
+	INIT_LIST_HEAD(&object->report_list);
 	INIT_HLIST_HEAD(&object->area_list);
 	raw_spin_lock_init(&object->lock);
 	atomic_set(&object->use_count, 1);
@@ -655,6 +713,7 @@  static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
 
 	/* kernel backtrace */
 	object->trace_len = __save_stack_trace(object->trace);
+	object->trace_hash = jhash2((const u32 *)object->trace, object->trace_len / sizeof(u32), trace_seed);
 
 	raw_spin_lock_irqsave(&kmemleak_lock, flags);
 
@@ -694,8 +753,42 @@  static struct kmemleak_object *__create_object(unsigned long ptr, size_t size,
 	rb_link_node(&object->rb_node, rb_parent, link);
 	rb_insert_color(&object->rb_node, is_phys ? &object_phys_tree_root :
 					  &object_tree_root);
-
 	list_add_tail_rcu(&object->object_list, &object_list);
+
+	raw_spin_lock_irqsave(&kmemleak_lock, flags);
+	link = &object_report_tree_root.rb_node;
+	rb_parent = NULL;
+	while (*link) {
+		raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
+		raw_spin_lock_irqsave(&kmemleak_lock, flags);
+		rb_parent = *link;
+		report_parent = rb_entry(rb_parent, struct kmemleak_report, rb_node_report);
+		trace_hash = report_parent->trace_hash;
+		if (object->trace_hash < trace_hash)
+			link = &report_parent->rb_node_report.rb_left;
+		else if (trace_hash < object->trace_hash)
+			link = &report_parent->rb_node_report.rb_right;
+		else {
+			list_add_tail_rcu(&object->report_list, &report_parent->report_list_head);
+			goto out;
+		}
+	}
+	report = kr_pool_free_count ? &kr_pool[--kr_pool_free_count] : NULL;
+	if (!report)
+		goto out;
+	report->trace_hash = object->trace_hash;
+	/*
+	 * report is the 1st node represent this trace_hash, init its list_head and
+	 * insert it to object_report_tree_root
+	 */
+	INIT_LIST_HEAD(&report->report_list_head);
+	/* record 1st object to the list*/
+	list_add_tail_rcu(&object->report_list, &report->report_list_head);
+	/* initialize the report_pos as report_list_head*/
+	report->report_pos = &report->report_list_head;
+	/* add the node to rb tree*/
+	rb_link_node(&report->rb_node_report, rb_parent, link);
+	rb_insert_color(&report->rb_node_report, &object_report_tree_root);
 out:
 	raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
 	return object;
@@ -1947,6 +2040,8 @@  static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
 		kmemleak_scan();
 	else if (strncmp(buf, "dump=", 5) == 0)
 		ret = dump_str_object_info(buf + 5);
+	else if (strncmp(buf, "report", 6) == 0)
+		print_unreferenced_object_list(NULL);
 	else
 		ret = -EINVAL;
 
@@ -2080,6 +2175,7 @@  void __init kmemleak_init(void)
 		create_object((unsigned long)__start_ro_after_init,
 			      __end_ro_after_init - __start_ro_after_init,
 			      KMEMLEAK_GREY, GFP_ATOMIC);
+	get_random_bytes(&trace_seed, 4);
 }
 
 /*