Re: [PATCH] mm: Warn once when a page is freed with PG_mlocked set V2

[Johannes Weiner <hannes@cmpxchg.org>]

Hello Christoph,

On Wed, Jul 15, 2009 at 10:31:54AM -0400, Christoph Lameter wrote:
> On Wed, 15 Jul 2009, Mel Gorman wrote:
> 
> > -static inline int free_pages_check(struct page *page)
> > +static inline int free_pages_check(struct page *page, int wasMlocked)
> >  {
> > +	WARN_ONCE(wasMlocked, KERN_WARNING
> > +		"Page flag mlocked set for process %s at pfn:%05lx\n"
> > +		"page:%p flags:0x%lX\n",
> > +		current->comm, page_to_pfn(page),
> > +		page, page->flags|__PG_MLOCKED);
> > +
> >  	if (unlikely(page_mapcount(page) |
> 
> There is already a free_page_mlocked() that is only called if the mlock
> bit is set. Move it into there to avoid having to run two checks in the
> hot codee path?
> 
> Also __free_pages_ok() now has a TestClearMlocked in the hot code path.
> Would it be possible to get rid of the unconditional use of an atomic
> operation? Just check the bit and clear it later in free_page_mlocked()?

That was initially done, but free_pages_check() checks for that flag
and did bad_page() on those mlocked ones.  Now, one idea was to not
check the mlocked flag at all in free_pages_check() as we handle it
differently anyway.  But I think we might still want to check for it
in tail-pages of higher order blocks.

And if you move that warning after free_pages_check(), the interesting
bits for the warning have been wiped already.

But we can get rid of the locked test-and-clear despite all the other
issues, patch below.

	Hannes