Re: [PATCH 1/3] When migrate_pages returns 0, all pages must have been released

On Fri, Jan 21, 2011 at 06:36:18PM +0100, Andrea Arcangeli wrote:

On Fri, Jan 21, 2011 at 10:11:03AM -0600, Christoph Lameter wrote:

quoted

On Thu, 20 Jan 2011, Andrea Arcangeli wrote:

quoted

Which following putback_lru_page()?  You mean
putback_lru_page(newpage)? That is for the newly allocated page
(allocated at the very top, so always needed), it's not relevant to
the page_count(page) = 1. The page_count 1 is hold by the caller, so
it's leaking memory right now (for everything but compaction).

Ahh yes we removed the putback_lru_pages call from migrate_pages()
and broke the existing release logic. The caller has to call
putback_release_pages() as per commit

putback_lru_paeges

quoted

cf608ac19c95804dc2df43b1f4f9e068aa9034ab

That is the very commit that introduced the two bugs that I've fixed
by code review.

quoted

If that is still the case then we still have the double free.

The caller only calls putback_lru_pages if ret != 0 (the two cases you
refer to happen with ret = 0).

Even if caller unconditionally calls putback_lru_pages (kind of what
compaction did), it can't double free because migrate_pages already
unlinked the pages before calling putback_lru_page(page), so there's
no way to do a double free (however if the caller unconditionally
called putback_lru_pages there would be no memleak to fix, but it
doesn't).

quoted

Could we please document the calling conventions exactly in the source?
Right now it says that the caller should call putback_lru_pages().

The caller should call putback_lru_pages only if ret != 0. Minchan
this is your commit we're discussing can you check the commentary?

No problem.
I will send the patch.

Thanks Adnrea, Christoph.

Thanks!
Andrea

-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>