2005-09-05 19:50:17

by Alexander Nyberg

[permalink] [raw]
Subject: Some debugging patches on top of -mm

These are debugging patches on-top of -mm that makes it possible
for those arches that want to be able to to save caller traces
of who allocates pages and slab objects.

Any arch that wants to use this could make a next_stack_func function
that goes through the stack starting at *prev_addr and finds the next
function return address. 'count' is for when we can use the frame
pointer (CONFIG_FRAME_POINTER) to get accurate backtraces.

For x86 it goes like:

unsigned long *next_stack_func(unsigned long *prev_addr, int count)
{
struct thread_info *tinfo = current_thread_info();

if (!prev_addr)
return NULL;

#ifdef CONFIG_FRAME_POINTER
/* In this case 'prev_addr' is a pointer to the last return
* function found on the stack */
if (count == 0) {
unsigned long ebp;
unsigned long *func_ptr;

asm ("movl %%ebp, %0" : "=r" (ebp) : );
/* We don't want the obvious caller to show up */
ebp = *(unsigned long *) ebp;
func_ptr = (unsigned long *)(ebp + 4);
if (valid_stack_ptr(tinfo, func_ptr))
return func_ptr;
} else {
unsigned long *func_ptr;
unsigned long ebp = (unsigned long) prev_addr;

ebp -= 4;

ebp = *(unsigned long *) ebp;
func_ptr = (unsigned long *) ((unsigned long)ebp + 4);
if (valid_stack_ptr(tinfo, func_ptr))
return func_ptr;
}
#else
while (prev_addr++) {
if (!valid_stack_ptr(tinfo, prev_addr))
break;
if (__kernel_text_address(*prev_addr))
return prev_addr;
}
#endif
return NULL;
}


1) A "generic" next_stack_func() for arches that want to have these
debugging facilities

2) Saving more slab object call traces via DBG_DEBUGWORDS. Now uses
next_stack_func(). This still prints to the console, oh well...
(I have not made SLAB_DEBUG conditional on x86 so it won't compile on
non-x86 arches with these patches currently...)

3) Simplification of the page-owner-leak-detector to use next_stack_func()
so that any arch that wants it can use it.



Attachments:
(No filename) (2.24 kB)
generic_stack_trace.patch (3.49 kB)
slab_user_mult.patch (5.92 kB)
simplify_page_owner.patch (1.71 kB)
Download all attachments

2005-11-12 11:13:38

by Paul Jackson

[permalink] [raw]
Subject: Re: Some debugging patches on top of -mm

Alexander,

This patch, known as page-owner-tracking-leak-detector.patch has
apparently been sitting in Andrew's *-mm for the last two months.

I just noticed it now, when reading mm/page_alloc.c.

I'd like to know if the #ifdef's and CONFIG_PAGE_OWNER specific code
can be removed from page_alloc.c, and put in a header file. Ideally,
you patch would add just one line to the __alloc_pages() code - a call
to set_page_owner() that either became no code (a static inline empty
function) or a call to your code, if this feature was CONFIG enabled.

The *.c files are where all the logic comes together, and it is vital
to the long term readability of these files that we avoid #ifdef's in
these files. Any one feature can be ifdef'd in, with seeming little
harm to the readability of the code (especially in the eyes of the
author of that particular bit of ifdef'd code ;). But imagine what
a deity-awful mess these files would be if we had all been doing that
over the years with our various favorite features.

I am not sure which header file - quite possibly in a new header
file just for this feature (unless others have the good sense to
recommend better.) Static inline code that is only called from
one place should work fine in a header file, at least technically.

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401

2005-11-12 20:26:49

by Alexander Nyberg

[permalink] [raw]
Subject: Re: Some debugging patches on top of -mm

> This patch, known as page-owner-tracking-leak-detector.patch has
> apparently been sitting in Andrew's *-mm for the last two months.
>
> I just noticed it now, when reading mm/page_alloc.c.
>
> I'd like to know if the #ifdef's and CONFIG_PAGE_OWNER specific code
> can be removed from page_alloc.c, and put in a header file. Ideally,
> you patch would add just one line to the __alloc_pages() code - a call
> to set_page_owner() that either became no code (a static inline empty
> function) or a call to your code, if this feature was CONFIG enabled.
>
> The *.c files are where all the logic comes together, and it is vital
> to the long term readability of these files that we avoid #ifdef's in
> these files. Any one feature can be ifdef'd in, with seeming little
> harm to the readability of the code (especially in the eyes of the
> author of that particular bit of ifdef'd code ;). But imagine what
> a deity-awful mess these files would be if we had all been doing that
> over the years with our various favorite features.
>

Yes, it was a quick hack when akpm said that he'd like a page tracking
mechanism. I said myself that I thought it was too ugly to go into
mainline. Later I tried to make it some kind of generic framework that
all arches could use but there was no interest.

If you wish to make it nicer/mergeable it's all yours.

2005-11-12 20:39:20

by Paul Jackson

[permalink] [raw]
Subject: Re: Some debugging patches on top of -mm

> If you wish to make it nicer/mergeable it's all yours.

Nah - I just wanted those ugly ifdef's out of page_alloc.c.

I sure don't want to own it.

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401