linux-kernel - Re: [PATCH 18/35] Do not disable interrupts in free_page

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: <20090316162936.GI24293@csn.ul.ie>
Date:	Mon, 16 Mar 2009 16:29:36 +0000
From:	Mel Gorman <mel@....ul.ie>
To:	Christoph Lameter <cl@...ux-foundation.org>
Cc:	Linux Memory Management List <linux-mm@...ck.org>,
	Pekka Enberg <penberg@...helsinki.fi>,
	Rik van Riel <riel@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Nick Piggin <npiggin@...e.de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Lin Ming <ming.m.lin@...el.com>,
	Zhang Yanmin <yanmin_zhang@...ux.intel.com>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 18/35] Do not disable interrupts in free_page_mlock()

On Mon, Mar 16, 2009 at 12:05:53PM -0400, Christoph Lameter wrote:
> On Mon, 16 Mar 2009, Mel Gorman wrote:
> 
> > @@ -570,6 +570,8 @@ static void __free_pages_ok(struct page *page, unsigned int order)
> >  	kernel_map_pages(page, 1 << order, 0);
> >
> >  	local_irq_save(flags);
> > +	if (clearMlocked)
> > +		free_page_mlock(page);
> >  	__count_vm_events(PGFREE, 1 << order);
> >  	free_one_page(page_zone(page), page, order,
> >  					get_pageblock_migratetype(page));
> 
> Add an unlikely(clearMblocked) here?
> 

I wasn't sure at the time of writing how likely the case really is but it
makes sense that mlocked() pages are rarely freed. On reflection though,
it makes sense to mark this unlikely().

> > @@ -1036,6 +1039,9 @@ static void free_hot_cold_page(struct page *page, int cold)
> >  	pcp = &zone_pcp(zone, get_cpu())->pcp;
> >  	local_irq_save(flags);
> >  	__count_vm_event(PGFREE);
> > +	if (clearMlocked)
> > +		free_page_mlock(page);
> > +
> >  	if (cold)
> >  		list_add_tail(&page->lru, &pcp->list);
> >  	else
> >
> 
> Same here also make sure tha the __count_vm_events(PGFREE) comes after the
> free_pages_mlock() to preserve symmetry with __free_pages_ok() and maybe
> allow the compiler to do CSE between two invocations of
> __count_vm_events().
> 

Whatever about the latter reasoning about CSE, the symmetry makes sense.
I've made the change. Thanks

-- 
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/