Re: [2.6.35-rc6 patch] increase kmemleak robustness at boot

Previous thread: [PATCH 06/10] ALSA: msnd: check request_region() return value by Kulikov Vasiliy on Wednesday, July 28, 2010 - 9:40 am. (1 message)

Next thread: [PATCH 07/10] ALSA: sb: check get_user() return value by Kulikov Vasiliy on Wednesday, July 28, 2010 - 9:40 am. (2 messages)
From: Daniel J Blueman
Date: Wednesday, July 28, 2010 - 9:40 am

Hi Catalin,

I've consistently been experiencing kmemleak exhaust it's 400-entry
early-boot buffer and disabling itself; there have been reports of
this also, and I'm finding this on x86-64 with various debug options
enabled.

If we issue a warning and allow the buffer to wrap, we don't need to
hit the kill-switch. While we lose track of some early potential
leaks, it's better than no functionality.

Let me know if it's acceptable, and many thanks for such an excellent tool,
  Daniel

---

Allow the early-boot buffer to wrap, rather than disabling kmemleak
and losing the functionality.

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 2c0d032..93bf8a3 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -786,13 +786,6 @@ static void __init log_early(int op_type, const
void *ptr, size_t size,
 	unsigned long flags;
 	struct early_log *log;

-	if (crt_early_log >= ARRAY_SIZE(early_log)) {
-		pr_warning("Early log buffer exceeded, "
-			   "please increase DEBUG_KMEMLEAK_EARLY_LOG_SIZE\n");
-		kmemleak_disable();
-		return;
-	}
-
 	/*
 	 * There is no need for locking since the kernel is still in UP mode
 	 * at this stage. Disabling the IRQs is enough.
@@ -805,7 +798,13 @@ static void __init log_early(int op_type, const
void *ptr, size_t size,
 	log->min_count = min_count;
 	if (op_type == KMEMLEAK_ALLOC)
 		log->trace_len = __save_stack_trace(log->trace);
+
 	crt_early_log++;
+	if (crt_early_log >= ARRAY_SIZE(early_log)) {
+		pr_warning("Early log buffer exhausted - wrapping\n");
+		crt_early_log = 0;
+	}
+	
 	local_irq_restore(flags);
 }

-- 
Daniel J Blueman
--

From: Pekka Enberg
Date: Wednesday, July 28, 2010 - 9:49 am

Is it just potential leaks that we lose or can this cause false positives?
--

From: Daniel J Blueman
Date: Wednesday, July 28, 2010 - 11:39 am

I don't get any false positives having had the buffer wrap a number of
times at early-boot; not to say this can't cause any.

It seems that some kernel debug options are doing heavy early-boot
allocations, so getting any false-positives would likely be a triple
edge case.

Thanks,
  Daniel
-- 
Daniel J Blueman
--

From: Catalin Marinas
Date: Thursday, July 29, 2010 - 4:34 am

I wouldn't go this route, it's a great source of false positives.
Given that it's not always easy to investigate a memory leak, adding
more false positives would just make people turn the tool off. There
are several things in place like crc checking and maybe that's why
Daniel doesn't get false positives but that's not always the case.

I would rather change the static early alloc buffer with something
like bootmem allocation (the recursiveness should be bound, kmemleak
tracks bootmem allocations as well). But I'm on holiday until middle
of August and not able to do any tests in this area.

-- 
Catalin
--

From: Daniel J Blueman
Date: Thursday, July 29, 2010 - 5:48 am

Indeed, moving to dynamic early allocation is all the more better. For
now, I'll increase the early allocation to 15200 elements, as the
400-entry buffer wraps 38.

Thanks again,
  Daniel
-- 
Daniel J Blueman
--

From: Pekka Enberg
Date: Thursday, July 29, 2010 - 8:12 am

If it's just kmemleak_init() we're talking about, slab caches are up at 
that point so you can just use kmalloc().
--

From: Daniel J Blueman
Date: Thursday, July 29, 2010 - 11:03 am

The slab allocator isn't up at this point. With
CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE set to 16K elements, an extra 3MB
of __initdata memory is used, but freed afterwards, and it works
great.

Thanks,
  Daniel
-- 
Daniel J Blueman
--

From: Pekka Enberg
Date: Thursday, July 29, 2010 - 11:50 am

On Thu, Jul 29, 2010 at 9:03 PM, Daniel J Blueman

Right. I guess the required earlylog buffer size would be smaller if
we initialized kmemleak earlier. Can we do that in mm_init() after
kmem_cache_init()?
--

From: Catalin Marinas
Date: Thursday, July 29, 2010 - 1:39 pm

Kmemleak uses kmem_cache_alloc() internally so we could initialise it
as soon as kmem_cache_init() was called. But it's really strange the
amount of early allocations that Daniel is getting. I've been happy so
far with 400, usually with standard Ubuntu-like configs and some
debugging turned on. Any idea what's causing these allocations?

-- 
Catalin
--

From: Pekka Enberg
Date: Saturday, July 31, 2010 - 2:42 am

No idea. I wonder if kmemleak can dump out the call-sites for the 
overflow case somehow to see what's going on?

			Pekka
--

Previous thread: [PATCH 06/10] ALSA: msnd: check request_region() return value by Kulikov Vasiliy on Wednesday, July 28, 2010 - 9:40 am. (1 message)

Next thread: [PATCH 07/10] ALSA: sb: check get_user() return value by Kulikov Vasiliy on Wednesday, July 28, 2010 - 9:40 am. (2 messages)