Are there any more comments here? It seems the initial reaction wasn't all that bad and so I would like to post this with RFC dropped. On Fri 08-12-17 17:15:56, Michal Hocko wrote: > On Thu 07-12-17 15:34:01, Michal Hocko wrote: > > On Thu 07-12-17 22:10:47, Zi Yan wrote: > [...] > > > I agree with you that we should try to migrate all tail pages if the THP > > > needs to be split. But this might not be compatible with "getting > > > migration results" in unmap_and_move(), since a caller of > > > migrate_pages() may want to know the status of each page in the > > > migration list via int **result in get_new_page() (e.g. > > > new_page_node()). The caller has no idea whether a THP in its migration > > > list will be split or not, thus, storing migration results might be > > > quite tricky if tail pages are added into the migration list. > > > > Ouch. I wasn't aware of this "beauty". I will try to wrap my head around > > this code and think about what to do about it. Thanks for point me to > > it. > > OK, so was staring at this yesterday and concluded that the current > implementation of do_move_page_to_node_array is unfixable to work with > split_thp_page_list in migrate_pages. So I've reimplemented it to not > use the quite ugly fixed sized batching. Instead I am using dynamic > batching based on the same node request. See the patch 1 for more > details about implementation. This will allow us to remove the quite > ugly 'int **reason' from the allocation callback as well. This is patch > 2 and patch 3 is finally the thp migration code. > > Diffstat is quite supportive for this cleanup. > include/linux/migrate.h | 7 +- > include/linux/page-isolation.h | 3 +- > mm/compaction.c | 3 +- > mm/huge_memory.c | 6 + > mm/internal.h | 1 + > mm/memory_hotplug.c | 5 +- > mm/mempolicy.c | 40 +---- > mm/migrate.c | 350 ++++++++++++++++++----------------------- > mm/page_isolation.c | 3 +- > 9 files changed, 177 insertions(+), 241 deletions(-) > > Does anybody see any issues with this approach? > > On a side note: > There are some semantic issues I have encountered on the way but I am > not addressing them here. E.g. trying to move THP tail pages will result > in either success or EBUSY (the later one more likely once we isolate > head from the LRU list). Hugetlb reports EACCESS on tail pages. > Some errors are reported via status parameter but migration failures are > not even though the original `reason' argument suggests there was an > intention to do so. From a quick look into git history this never > worked. I have tried to keep the semantic unchanged. > > Then there is a relatively minor thing that the page isolation might > fail because of pages not being on the LRU - e.g. because they are > sitting on the per-cpu LRU caches. Easily fixable. -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
