linux-kernel - Re: [PATCH 2/4] mm: more intensive memory corruption debug

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]

Date:	Mon, 14 Nov 2011 11:29:23 +0100
From:	Stanislaw Gruszka <sgruszka@...hat.com>
To:	Mel Gorman <mgorman@...e.de>
Cc:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Andrea Arcangeli <aarcange@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Christoph Lameter <cl@...ux-foundation.org>
Subject: Re: [PATCH 2/4] mm: more intensive memory corruption debug

On Fri, Nov 11, 2011 at 02:29:53PM +0000, Mel Gorman wrote:
> >  	if (PageBuddy(buddy) && page_order(buddy) == order) {
> >  		VM_BUG_ON(page_count(buddy) != 0);
> >  		return 1;
> > @@ -518,9 +562,15 @@ static inline void __free_one_page(struct page *page,
> >  			break;
> >  
> >  		/* Our buddy is free, merge with it and move up one order. */
> > -		list_del(&buddy->lru);
> > -		zone->free_area[order].nr_free--;
> > -		rmv_page_order(buddy);
> > +		if (page_is_corrupt_dbg(buddy)) {
> > +			clear_page_corrupt_dbg(buddy);
> > +			set_page_private(page, 0);
> > +			__mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
> 
> Why are the buddies not merged?
I believe they are merged, but I'll double check.

> >  static inline void expand(struct zone *zone, struct page *page,
> > -	int low, int high, struct free_area *area,
> > +	unsigned int low, unsigned int high, struct free_area *area,
> >  	int migratetype)
> >  {
> >  	unsigned long size = 1 << high;
> > @@ -746,9 +796,16 @@ static inline void expand(struct zone *zone, struct page *page,
> >  		high--;
> >  		size >>= 1;
> >  		VM_BUG_ON(bad_range(zone, &page[size]));
> > -		list_add(&page[size].lru, &area->free_list[migratetype]);
> > -		area->nr_free++;
> > -		set_page_order(&page[size], high);
> > +		if (high < corrupt_dbg()) {
> > +			INIT_LIST_HEAD(&page[size].lru);
> > +			set_page_corrupt_dbg(&page[size]);
> > +			set_page_private(&page[size], high);
> > +			__mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << high));
> > +		} else {
> 
> Because high is a signed integer, I don't think this would necessarily
> optimised away at compile time when DEBUG_PAGEALLOC is not set adding a
> new branch to a heavily executed fast path.
> 
> For the fast paths, you should not add new branches if you can. Move the
> debugging code to inline functions that only exist when DEBUG_PAGEALLOC
> is set so there is no additional overhead in the !CONFIG_DEBUG_PAGEALLOC
> case.

I changed "high" type from int to unsigned int in the patch, and checked
that this branch is removed by compiler in !CONFIG_DEBUG_PAGEALLOC case.
But perhaps having this inside preprocessor checks is cleaner, so I'll do
that.

Thanks for the comments, I'll rework and repost.
Stanislaw
--
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/