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
Daniel J Blueman wrote:
> 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,
Is it just potential leaks that we lose or can this cause false positives?
On 28 July 2010 17:49, Pekka Enberg <[email protected]> wrote:
> Daniel J Blueman wrote:
>>
>> 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,
>
> Is it just potential leaks that we lose or can this cause false positives?
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
On 28 July 2010 17:49, Pekka Enberg <[email protected]> wrote:
> Daniel J Blueman wrote:
>>
>> 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,
>
> Is it just potential leaks that we lose or can this cause false positives?
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
On 29 July 2010 12:34, Catalin Marinas <[email protected]> wrote:
> On 28 July 2010 17:49, Pekka Enberg <[email protected]> wrote:
>> Daniel J Blueman wrote:
>>>
>>> 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,
>>
>> Is it just potential leaks that we lose or can this cause false positives?
>
> 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.
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
Daniel J Blueman wrote:
>> 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.
>
> 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.
If it's just kmemleak_init() we're talking about, slab caches are up at
that point so you can just use kmalloc().
On 29 July 2010 16:12, Pekka Enberg <[email protected]> wrote:
> Daniel J Blueman wrote:
>>>
>>> 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.
>>
>> 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.
>
> If it's just kmemleak_init() we're talking about, slab caches are up at that
> point so you can just use kmalloc().
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
On Thu, Jul 29, 2010 at 9:03 PM, Daniel J Blueman
<[email protected]> wrote:
>> If it's just kmemleak_init() we're talking about, slab caches are up at that
>> point so you can just use kmalloc().
>
> 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.
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()?
On 29 July 2010 19:50, Pekka Enberg <[email protected]> wrote:
> On Thu, Jul 29, 2010 at 9:03 PM, Daniel J Blueman
> <[email protected]> wrote:
>>> If it's just kmemleak_init() we're talking about, slab caches are up at that
>>> point so you can just use kmalloc().
>>
>> 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.
>
> 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()?
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
Catalin Marinas wrote:
>> 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()?
>
> 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?
No idea. I wonder if kmemleak can dump out the call-sites for the
overflow case somehow to see what's going on?
Pekka