Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page()

[KOSAKI Motohiro <kosaki.motohiro@gmail.com>]

(3/27/13 9:00 AM), Michal Hocko wrote:
> On Tue 26-03-13 16:35:35, Naoya Horiguchi wrote:
> [...]
>> The differences is that migrate_huge_page() has one hugepage as an argument,
>> and migrate_pages() has a pagelist with multiple hugepages.
>> I already told this before and I'm not sure it's enough to answer the question,
>> so I explain another point about why this patch do like it.
> 
> OK, I am blind. It is
> +       list_move(&hpage->lru, &pagelist);
> +       ret = migrate_pages(&pagelist, new_page, MPOL_MF_MOVE_ALL,
> +                               MIGRATE_SYNC, MR_MEMORY_FAILURE);
> 
> which moves it from active_list and so you have to put it back.
> 
>> I think that we must do putback_*pages() for source pages whether migration
>> succeeds or not.
>> But when we call migrate_pages() with a pagelist,
>> the caller can't access to the successfully migrated source pages
>> after migrate_pages() returns, because they are no longer on the pagelist.
>> So putback of the successfully migrated source pages should be done *in*
>> unmap_and_move() and/or unmap_and_move_huge_page().
> 
> If the migration succeeds then the page becomes unused and free after
> its last reference drops. So I do not see any reason to put it back to
> active list and free it right afterwards.
> On the other hand unmap_and_move does the same thing (although page
> reference counting is a bit more complicated in that case) so it would
> be good to keep in sync with regular pages case.

Even if pages are isolated from lists, there are several page count increasing
path. So, putback_pages() close a race when page count != 1.

I'm not sure, but I guess follow_hugepage() can make the same race.


--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>