Re: + page-owner-tracking.patch added to -mm tree

[Mel Gorman]

On Wed, Apr 01, 2009 at 01:15:40PM +0200, Ingo Molnar wrote:
> > <VAST AMOUNTS OF SNIPPAGE>
> >
> > +static inline void __stack_trace(struct page *page, unsigned long *stack,
> > +			unsigned long bp)
> > +{
> > +	int i = 0;
> > +	unsigned long addr;
> > +	struct thread_info *tinfo = (struct thread_info *)
> > +		((unsigned long)stack & (~(THREAD_SIZE - 1)));
> > +
> > +	memset(page->trace, 0, sizeof(long) * 8);
> > +
> > +#ifdef CONFIG_FRAME_POINTER
> > +	if (bp) {
> > +		while (valid_stack_ptr(tinfo, (void *)bp)) {
> > +			addr = *(unsigned long *)(bp + sizeof(long));
> > +			page->trace[i] = addr;
> > +			if (++i >= 8)
> > +				break;
> > +			bp = *(unsigned long *)bp;
> > +		}
> > +		return;
> > +	}
> > +#endif /* CONFIG_FRAME_POINTER */
> > +	while (valid_stack_ptr(tinfo, stack)) {
> > +		addr = *stack++;
> > +		if (__kernel_text_address(addr)) {
> > +			page->trace[i] = addr;
> > +			if (++i >= 8)
> > +				break;
> > +		}
> > +	}
> > +}
> 
> Uhm, this is not acceptable and broken, we have generic stacktrace 
> saving facilities for this precise purpose. It has other problems 
> too.
> 
> The purpose of the patch seems genuinely useful and i support the 
> concept, but the current design is limiting (we could do much 
> better) and the implementation is awful. Please.
> 
> Has this patch been reviewed by or Cc:-ed to anyone versed in kernel 
> instrumentation details, before it was applied to -mm? Those folks 
> hang around the tracing tree usually so they are easy to find. :)
> 

This patch is ancient, predating most of the instrumentation stuff by years. It
was dropped from -mm a few months ago because of changes in proc and this is
a rebase because it came up as being potentially useful pinning down memory
leaks when they occur.

I'm not sure when exactly it was introduced to -mm, but I see references
going back as far as 2.6.12-rc1 so it's no surprise this is now extremly
odd looking. However, there is no plan to merge this to mainline making the
effort of redoing it from scratch a questionable expenditure of time.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/