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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090401125506.GA6406@csn.ul.ie>
Date:	Wed, 1 Apr 2009 13:55:07 +0100
From:	Mel Gorman <mel@....ul.ie>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	linux-kernel@...r.kernel.org, mm-commits@...r.kernel.org,
	alexn@....su.se, akpm@...ux-foundation.org, alexn@...ia.com,
	apw@...dowen.org, cl@...ux-foundation.org, haveblue@...ibm.com,
	kamezawa.hiroyu@...fujitu.com,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Steven Rostedt <rostedt@...dmis.org>,
	Fr?d?ric Weisbecker <fweisbec@...il.com>
Subject: Re: + page-owner-tracking.patch added to -mm tree

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/