Messages in this thread | | | Date | Fri, 05 Apr 2013 17:11:40 -0400 | From | KOSAKI Motohiro <> | Subject | Re: [PATCH 03/10] soft-offline: use migrate_pages() instead of migrate_huge_page() |
| |
(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.
| |