Skip to content

Commit 85d3a31

Browse files
walken-googletorvalds
authored andcommitted
kmemleak: use rbtree instead of prio tree
kmemleak uses a tree where each node represents an allocated memory object in order to quickly find out what object a given address is part of. However, the objects don't overlap, so rbtrees are a better choice than prio tree for this use. They are both faster and have lower memory overhead. Tested by booting a kernel with kmemleak enabled, loading the kmemleak_test module, and looking for the expected messages. Signed-off-by: Michel Lespinasse <[email protected]> Cc: Rik van Riel <[email protected]> Cc: Hillf Danton <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Andrea Arcangeli <[email protected]> Cc: David Woodhouse <[email protected]> Acked-by: Catalin Marinas <[email protected]> Tested-by: Catalin Marinas <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 6b2dbba commit 85d3a31

File tree

1 file changed

+51
-49
lines changed

1 file changed

+51
-49
lines changed

mm/kmemleak.c

Lines changed: 51 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
* - kmemleak_lock (rwlock): protects the object_list modifications and
3030
* accesses to the object_tree_root. The object_list is the main list
3131
* holding the metadata (struct kmemleak_object) for the allocated memory
32-
* blocks. The object_tree_root is a priority search tree used to look-up
32+
* blocks. The object_tree_root is a red black tree used to look-up
3333
* metadata based on a pointer to the corresponding memory block. The
3434
* kmemleak_object structures are added to the object_list and
3535
* object_tree_root in the create_object() function called from the
@@ -71,7 +71,7 @@
7171
#include <linux/delay.h>
7272
#include <linux/export.h>
7373
#include <linux/kthread.h>
74-
#include <linux/prio_tree.h>
74+
#include <linux/rbtree.h>
7575
#include <linux/fs.h>
7676
#include <linux/debugfs.h>
7777
#include <linux/seq_file.h>
@@ -132,7 +132,7 @@ struct kmemleak_scan_area {
132132
* Structure holding the metadata for each allocated memory block.
133133
* Modifications to such objects should be made while holding the
134134
* object->lock. Insertions or deletions from object_list, gray_list or
135-
* tree_node are already protected by the corresponding locks or mutex (see
135+
* rb_node are already protected by the corresponding locks or mutex (see
136136
* the notes on locking above). These objects are reference-counted
137137
* (use_count) and freed using the RCU mechanism.
138138
*/
@@ -141,7 +141,7 @@ struct kmemleak_object {
141141
unsigned long flags; /* object status flags */
142142
struct list_head object_list;
143143
struct list_head gray_list;
144-
struct prio_tree_node tree_node;
144+
struct rb_node rb_node;
145145
struct rcu_head rcu; /* object_list lockless traversal */
146146
/* object usage count; object freed when use_count == 0 */
147147
atomic_t use_count;
@@ -182,9 +182,9 @@ struct kmemleak_object {
182182
static LIST_HEAD(object_list);
183183
/* the list of gray-colored objects (see color_gray comment below) */
184184
static LIST_HEAD(gray_list);
185-
/* prio search tree for object boundaries */
186-
static struct prio_tree_root object_tree_root;
187-
/* rw_lock protecting the access to object_list and prio_tree_root */
185+
/* search tree for object boundaries */
186+
static struct rb_root object_tree_root = RB_ROOT;
187+
/* rw_lock protecting the access to object_list and object_tree_root */
188188
static DEFINE_RWLOCK(kmemleak_lock);
189189

190190
/* allocation caches for kmemleak internal data */
@@ -380,7 +380,7 @@ static void dump_object_info(struct kmemleak_object *object)
380380
trace.entries = object->trace;
381381

382382
pr_notice("Object 0x%08lx (size %zu):\n",
383-
object->tree_node.start, object->size);
383+
object->pointer, object->size);
384384
pr_notice(" comm \"%s\", pid %d, jiffies %lu\n",
385385
object->comm, object->pid, object->jiffies);
386386
pr_notice(" min_count = %d\n", object->min_count);
@@ -392,32 +392,32 @@ static void dump_object_info(struct kmemleak_object *object)
392392
}
393393

394394
/*
395-
* Look-up a memory block metadata (kmemleak_object) in the priority search
395+
* Look-up a memory block metadata (kmemleak_object) in the object search
396396
* tree based on a pointer value. If alias is 0, only values pointing to the
397397
* beginning of the memory block are allowed. The kmemleak_lock must be held
398398
* when calling this function.
399399
*/
400400
static struct kmemleak_object *lookup_object(unsigned long ptr, int alias)
401401
{
402-
struct prio_tree_node *node;
403-
struct prio_tree_iter iter;
404-
struct kmemleak_object *object;
405-
406-
prio_tree_iter_init(&iter, &object_tree_root, ptr, ptr);
407-
node = prio_tree_next(&iter);
408-
if (node) {
409-
object = prio_tree_entry(node, struct kmemleak_object,
410-
tree_node);
411-
if (!alias && object->pointer != ptr) {
402+
struct rb_node *rb = object_tree_root.rb_node;
403+
404+
while (rb) {
405+
struct kmemleak_object *object =
406+
rb_entry(rb, struct kmemleak_object, rb_node);
407+
if (ptr < object->pointer)
408+
rb = object->rb_node.rb_left;
409+
else if (object->pointer + object->size <= ptr)
410+
rb = object->rb_node.rb_right;
411+
else if (object->pointer == ptr || alias)
412+
return object;
413+
else {
412414
kmemleak_warn("Found object by alias at 0x%08lx\n",
413415
ptr);
414416
dump_object_info(object);
415-
object = NULL;
417+
break;
416418
}
417-
} else
418-
object = NULL;
419-
420-
return object;
419+
}
420+
return NULL;
421421
}
422422

423423
/*
@@ -471,7 +471,7 @@ static void put_object(struct kmemleak_object *object)
471471
}
472472

473473
/*
474-
* Look up an object in the prio search tree and increase its use_count.
474+
* Look up an object in the object search tree and increase its use_count.
475475
*/
476476
static struct kmemleak_object *find_and_get_object(unsigned long ptr, int alias)
477477
{
@@ -516,8 +516,8 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
516516
int min_count, gfp_t gfp)
517517
{
518518
unsigned long flags;
519-
struct kmemleak_object *object;
520-
struct prio_tree_node *node;
519+
struct kmemleak_object *object, *parent;
520+
struct rb_node **link, *rb_parent;
521521

522522
object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
523523
if (!object) {
@@ -560,31 +560,34 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
560560
/* kernel backtrace */
561561
object->trace_len = __save_stack_trace(object->trace);
562562

563-
INIT_PRIO_TREE_NODE(&object->tree_node);
564-
object->tree_node.start = ptr;
565-
object->tree_node.last = ptr + size - 1;
566-
567563
write_lock_irqsave(&kmemleak_lock, flags);
568564

569565
min_addr = min(min_addr, ptr);
570566
max_addr = max(max_addr, ptr + size);
571-
node = prio_tree_insert(&object_tree_root, &object->tree_node);
572-
/*
573-
* The code calling the kernel does not yet have the pointer to the
574-
* memory block to be able to free it. However, we still hold the
575-
* kmemleak_lock here in case parts of the kernel started freeing
576-
* random memory blocks.
577-
*/
578-
if (node != &object->tree_node) {
579-
kmemleak_stop("Cannot insert 0x%lx into the object search tree "
580-
"(already existing)\n", ptr);
581-
object = lookup_object(ptr, 1);
582-
spin_lock(&object->lock);
583-
dump_object_info(object);
584-
spin_unlock(&object->lock);
585-
586-
goto out;
567+
link = &object_tree_root.rb_node;
568+
rb_parent = NULL;
569+
while (*link) {
570+
rb_parent = *link;
571+
parent = rb_entry(rb_parent, struct kmemleak_object, rb_node);
572+
if (ptr + size <= parent->pointer)
573+
link = &parent->rb_node.rb_left;
574+
else if (parent->pointer + parent->size <= ptr)
575+
link = &parent->rb_node.rb_right;
576+
else {
577+
kmemleak_stop("Cannot insert 0x%lx into the object "
578+
"search tree (overlaps existing)\n",
579+
ptr);
580+
kmem_cache_free(object_cache, object);
581+
object = parent;
582+
spin_lock(&object->lock);
583+
dump_object_info(object);
584+
spin_unlock(&object->lock);
585+
goto out;
586+
}
587587
}
588+
rb_link_node(&object->rb_node, rb_parent, link);
589+
rb_insert_color(&object->rb_node, &object_tree_root);
590+
588591
list_add_tail_rcu(&object->object_list, &object_list);
589592
out:
590593
write_unlock_irqrestore(&kmemleak_lock, flags);
@@ -600,7 +603,7 @@ static void __delete_object(struct kmemleak_object *object)
600603
unsigned long flags;
601604

602605
write_lock_irqsave(&kmemleak_lock, flags);
603-
prio_tree_remove(&object_tree_root, &object->tree_node);
606+
rb_erase(&object->rb_node, &object_tree_root);
604607
list_del_rcu(&object->object_list);
605608
write_unlock_irqrestore(&kmemleak_lock, flags);
606609

@@ -1766,7 +1769,6 @@ void __init kmemleak_init(void)
17661769

17671770
object_cache = KMEM_CACHE(kmemleak_object, SLAB_NOLEAKTRACE);
17681771
scan_area_cache = KMEM_CACHE(kmemleak_scan_area, SLAB_NOLEAKTRACE);
1769-
INIT_PRIO_TREE_ROOT(&object_tree_root);
17701772

17711773
if (crt_early_log >= ARRAY_SIZE(early_log))
17721774
pr_warning("Early log buffer exceeded (%d), please increase "

0 commit comments

Comments
 (0)