Messages in this thread Patch in this message | | | Subject | Re: [PATCH] kmemleak: Reduce verbosity when memory allocation fails | From | Catalin Marinas <> | Date | Tue, 11 Jan 2011 12:44:01 +0000 |
| |
On Mon, 2011-01-10 at 23:31 +0000, David Rientjes wrote: > On Mon, 10 Jan 2011, Catalin Marinas wrote: > > > It would be a shame if the allocation were __GFP_NORETRY and kmemleak > > > ended up looping forever because it suppresses the bit for a single page, > > > it uses __GFP_NOMEMALLOC and kmemleak ends up allocating from memory > > > reserves, or it uses __GFP_HARDWALL and kmemleak is allocating metadata in > > > a different cpuset. > > > > > > I'm not sure why you're not just masking __GFP_NOFAIL and __GFP_REPEAT and > > > then failing gracefully? (And __GFP_ZERO and __GFP_COMP, too, of course.) [...] > > If the user calling the kernel alloc function cannot get memory, > > kmemleak won't be called anyway. > > I'm talking about when the allocation is successful and the metadata > allocation is not, such as what Toralf reported. If you pass > __GFP_NOFAIL, it's going to loop forever which is certainly not what we'd > want: we'd rather just disable kmemleak and continue doing work.
I agree with this but kmemleak doesn't pass any __GFP_NOFAIL flag (it's masked out). The reason it only keeps (GFP_KERNEL | GFP_ATOMIC) from the caller flags is to know whether the kmemleak metadata allocation must be atomic or not. All the other flags a user may pass like __GFP_NOFAIL are masked out.
Of course, kmemleak can pass additional flags for allocating its metadata, but I see this as unrelated to what the user passed (as long as the atomicity is preserved).
Now, if the user passes __GFP_NOFAIL, do we want such flag for the kmemleak allocation? IMHO we don't need this (hence it is masked out). We just allow the kmemleak allocation to fail.
> If you > pass __GFP_NOMEMALLOC, then kmemleak can allocate from memory reserves in > the reclaim path which is not allowed. And if you don't pass > __GFP_HARDWALL, then these allocations can break memory isolation by > allowing the metadata to be allocated from different cpusets.
As I said above, we can pass additional flags like __GFP_NOMEMALLOC. But I think these should be hard-coded for kmemleak allocations irrespective of the k*alloc user gfp flags.
As for the __GFP_HARDWALL, I don't think it matters much. Kmemleak doesn't have per-CPU data anyway and the tree for storing the metada is a global one. When scanning the memory, it does it on a single CPU no matter where the object was allocated from. > > In other words, I'm pretty sure you don't want to be masking these options > off of the caller allocation when passed. It makes no sense for the user > to do a GFP_KERNEL | __GFP_NORETRY allocation and then have kmemleak loop > forever.
__GFP_NORETRY is another flag we could force on kmemleak metadata allocations. See below:
kmemleak: Allow kmemleak metadata allocations to fail
From: Catalin Marinas <catalin.marinas@arm.com>
This patch adds __GFP_NORETRY and __GFP_NOMEMALLOC flags to the kmemleak metadata allocations so that it has a smaller effect on the users of the kernel slab allocator. Since kmemleak allocations can now fail more often, this patch also reduces the verbosity by passing __GFP_NOWARN and not dumping the stack trace when a kmemleak allocation fails.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Reported-by: Toralf Förster <toralf.foerster@gmx.de> Cc: Pekka Enberg <penberg@cs.helsinki.fi> Cc: David Rientjes <rientjes@google.com> Cc: Ted Ts'o <tytso@mit.edu> --- mm/kmemleak.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c index bd9bc21..84225f3 100644 --- a/mm/kmemleak.c +++ b/mm/kmemleak.c @@ -113,7 +113,9 @@ #define BYTES_PER_POINTER sizeof(void *) /* GFP bitmask for kmemleak internal allocations */ -#define GFP_KMEMLEAK_MASK (GFP_KERNEL | GFP_ATOMIC) +#define gfp_kmemleak_mask(gfp) (((gfp) & (GFP_KERNEL | GFP_ATOMIC)) | \ + __GFP_NORETRY | __GFP_NOMEMALLOC | \ + __GFP_NOWARN) /* scanning area inside a memory block */ struct kmemleak_scan_area { @@ -511,9 +513,10 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size, struct kmemleak_object *object; struct prio_tree_node *node; - object = kmem_cache_alloc(object_cache, gfp & GFP_KMEMLEAK_MASK); + object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp)); if (!object) { - kmemleak_stop("Cannot allocate a kmemleak_object structure\n"); + pr_warning("Cannot allocate a kmemleak_object structure\n"); + kmemleak_disable(); return NULL; } @@ -734,9 +737,9 @@ static void add_scan_area(unsigned long ptr, size_t size, gfp_t gfp) return; } - area = kmem_cache_alloc(scan_area_cache, gfp & GFP_KMEMLEAK_MASK); + area = kmem_cache_alloc(scan_area_cache, gfp_kmemleak_mask(gfp)); if (!area) { - kmemleak_warn("Cannot allocate a scan area\n"); + pr_warning("Cannot allocate a scan area\n"); goto out; }
-- Catalin
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |