On 5 Apr 2018, at 15:04, Michal Hocko wrote: > On Thu 05-04-18 13:58:43, Zi Yan wrote: >> On 5 Apr 2018, at 12:03, Michal Hocko wrote: >> >>> On Thu 05-04-18 18:55:51, Kirill A. Shutemov wrote: >>>> On Thu, Apr 05, 2018 at 05:05:47PM +0200, Michal Hocko wrote: >>>>> On Thu 05-04-18 16:40:45, Kirill A. Shutemov wrote: >>>>>> On Thu, Apr 05, 2018 at 02:48:30PM +0200, Michal Hocko wrote: >>>>> [...] >>>>>>> RIght, I confused the two. What is the proper layer to fix that then? >>>>>>> rmap_walk_file? >>>>>> >>>>>> Maybe something like this? Totally untested. >>>>> >>>>> This looks way too complex. Why cannot we simply split THP page cache >>>>> during migration? >>>> >>>> This way we unify the codepath for archictures that don't support THP >>>> migration and shmem THP. >>> >>> But why? There shouldn't be really nothing to prevent THP (anon or >>> shemem) to be migratable. If we cannot migrate it at once we can always >>> split it. So why should we add another thp specific handling all over >>> the place? >> >> Then, it would be much easier if your "unclutter thp migration" patches is >> merged, >> plus the patch below: > > Good point. Except I would prefer a less convoluted condition > >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 60531108021a..b4087aa890f5 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1138,7 +1138,9 @@ static ICE_noinline int unmap_and_move(new_page_t >> get_new_page, >> int rc = MIGRATEPAGE_SUCCESS; >> struct page *newpage; >> >> - if (!thp_migration_supported() && PageTransHuge(page)) >> + if ((!thp_migration_supported() || >> + (thp_migration_supported() && !PageAnon(page))) && >> + PageTransHuge(page)) >> return -ENOMEM; > > What about this? > diff --git a/mm/migrate.c b/mm/migrate.c > index 5d0dc7b85f90..cd02e2bdf37c 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1138,7 +1138,11 @@ static ICE_noinline int unmap_and_move(new_page_t > get_new_page, > int rc = MIGRATEPAGE_SUCCESS; > struct page *newpage; > > - if (!thp_migration_supported() && PageTransHuge(page)) > + /* > + * THP pagecache or generally non-migrateable THP need to be split > + * up before migration > + */ > + if (PageTransHuge(page) && (!thp_migration_supported() || > !PageAnon(page))) > return -ENOMEM; > > newpage = get_new_page(page, private);
I think it works and is better than mine. Reviewed-by: Zi Yan <zi....@cs.rutgers.edu> -- Best Regards Yan Zi
signature.asc
Description: OpenPGP digital signature
