On Wed, Mar 27, 2019 at 09:44:32AM +0100, Michal Hocko wrote:
> On Tue 26-03-19 20:59:48, Qian Cai wrote:
> [...]
> > Unless there is a brave soul to reimplement the kmemleak to embed it's
> > metadata into the tracked memory itself in a foreseeable future,

Revisiting the kmemleak memory scanning code, that's not actually
possible without some long periods with kmemleak_lock held. The scanner
relies on the kmemleak_object (refcounted) being around even when the
actual memory block has been freed.

> > this
> > provides a good balance between enabling kmemleak in a low-memory
> > situation and not introducing too much hackiness into the existing
> > code for now. Another approach is to fail back the original allocation
> > once kmemleak_alloc() failed, but there are too many call sites to
> > deal with which makes it error-prone.
> 
> As long as there is an implicit __GFP_NOFAIL then kmemleak is simply
> broken no matter what other gfp flags you play with. Has anybody looked
> at some sort of preallocation where gfpflags_allow_blocking context
> allocate objects into a pool that non-sleeping allocations can eat from?

Quick attempt below and it needs some more testing (pretty random pick
of the EMERGENCY_POOL_SIZE value). Also, with __GFP_NOFAIL removed, are
the other flags safe or we should trim them further?

---------------8<-------------------------------
>From dc4194539f8191bb754901cea74c86e7960886f8 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.mari...@arm.com>
Date: Wed, 27 Mar 2019 17:20:57 +0000
Subject: [PATCH] mm: kmemleak: Add an emergency allocation pool for kmemleak
 objects

This patch adds an emergency pool for struct kmemleak_object in case the
normal kmem_cache_alloc() fails under the gfp constraints passed by the
slab allocation caller. The patch also removes __GFP_NOFAIL which does
not play well with other gfp flags (introduced by commit d9570ee3bd1d,
"kmemleak: allow to coexist with fault injection").

Suggested-by: Michal Hocko <mho...@kernel.org>
Signed-off-by: Catalin Marinas <catalin.mari...@arm.com>
---
 mm/kmemleak.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 6c318f5ac234..366a680cff7c 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -127,7 +127,7 @@
 /* GFP bitmask for kmemleak internal allocations */
 #define gfp_kmemleak_mask(gfp) (((gfp) & (GFP_KERNEL | GFP_ATOMIC)) | \
                                 __GFP_NORETRY | __GFP_NOMEMALLOC | \
-                                __GFP_NOWARN | __GFP_NOFAIL)
+                                __GFP_NOWARN)
 
 /* scanning area inside a memory block */
 struct kmemleak_scan_area {
@@ -191,11 +191,16 @@ struct kmemleak_object {
 #define HEX_ASCII              1
 /* max number of lines to be printed */
 #define HEX_MAX_LINES          2
+/* minimum emergency pool size */
+#define EMERGENCY_POOL_SIZE    (NR_CPUS * 4)
 
 /* the list of all allocated objects */
 static LIST_HEAD(object_list);
 /* the list of gray-colored objects (see color_gray comment below) */
 static LIST_HEAD(gray_list);
+/* emergency pool allocation */
+static LIST_HEAD(emergency_list);
+static int emergency_pool_size;
 /* search tree for object boundaries */
 static struct rb_root object_tree_root = RB_ROOT;
 /* rw_lock protecting the access to object_list and object_tree_root */
@@ -467,6 +472,43 @@ static int get_object(struct kmemleak_object *object)
        return atomic_inc_not_zero(&object->use_count);
 }
 
+/*
+ * Emergency pool allocation and freeing. kmemleak_lock must not be held.
+ */
+static struct kmemleak_object *emergency_alloc(void)
+{
+       unsigned long flags;
+       struct kmemleak_object *object;
+
+       write_lock_irqsave(&kmemleak_lock, flags);
+       object = list_first_entry_or_null(&emergency_list, typeof(*object), 
object_list);
+       if (object) {
+               list_del(&object->object_list);
+               emergency_pool_size--;
+       }
+       write_unlock_irqrestore(&kmemleak_lock, flags);
+
+       return object;
+}
+
+/*
+ * Return true if object added to the emergency pool, false otherwise.
+ */
+static bool emergency_free(struct kmemleak_object *object)
+{
+       unsigned long flags;
+
+       if (emergency_pool_size >= EMERGENCY_POOL_SIZE)
+               return false;
+
+       write_lock_irqsave(&kmemleak_lock, flags);
+       list_add(&object->object_list, &emergency_list);
+       emergency_pool_size++;
+       write_unlock_irqrestore(&kmemleak_lock, flags);
+
+       return true;
+}
+
 /*
  * RCU callback to free a kmemleak_object.
  */
@@ -485,7 +527,8 @@ static void free_object_rcu(struct rcu_head *rcu)
                hlist_del(&area->node);
                kmem_cache_free(scan_area_cache, area);
        }
-       kmem_cache_free(object_cache, object);
+       if (!emergency_free(object))
+               kmem_cache_free(object_cache, object);
 }
 
 /*
@@ -577,6 +620,8 @@ static struct kmemleak_object *create_object(unsigned long 
ptr, size_t size,
        unsigned long untagged_ptr;
 
        object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
+       if (!object)
+               object = emergency_alloc();
        if (!object) {
                pr_warn("Cannot allocate a kmemleak_object structure\n");
                kmemleak_disable();
@@ -2127,6 +2172,16 @@ void __init kmemleak_init(void)
                        kmemleak_warning = 0;
                }
        }
+
+       /* populate the emergency allocation pool */
+       while (emergency_pool_size < EMERGENCY_POOL_SIZE) {
+               struct kmemleak_object *object;
+
+               object = kmem_cache_alloc(object_cache, GFP_KERNEL);
+               if (!object)
+                       break;
+               emergency_free(object);
+       }
 }
 
 /*

Reply via email to