On 12/20/2017 11:28 PM, Michal Hocko wrote: > On Wed 20-12-17 14:43:03, Mike Kravetz wrote: >> On 12/20/2017 01:53 AM, Michal Hocko wrote: >>> On Wed 20-12-17 05:33:36, Naoya Horiguchi wrote: >>>> I have one comment on the code path from mbind(2). >>>> The callback passed to migrate_pages() in do_mbind() (i.e. new_page()) >>>> calls alloc_huge_page_noerr() which currently doesn't call >>>> SetPageHugeTemporary(), >>>> so hugetlb migration fails when h->surplus_huge_page >= >>>> h->nr_overcommit_huge_pages. >>> >>> Yes, I am aware of that. I should have been more explicit in the >>> changelog. Sorry about that and thanks for pointing it out explicitly. >>> To be honest I wasn't really sure what to do about this. The code path >>> is really complex and it made my head spin. I fail to see why we have to >>> call alloc_huge_page and mess with reservations at all. >> >> Oops! I missed that in my review. >> >> Since alloc_huge_page was called with avoid_reserve == 1, it should not >> do anything with reserve counts. One potential issue with the existing >> code is cgroup accounting done by alloc_huge_page. When the new target >> page is allocated, it is charged against the cgroup even though the original >> page is still accounted for. If we are 'at the cgroup limit', the migration >> may fail because of this. > > Yeah, the existing code seems just broken. I strongly suspect that the > allocation API for hugetlb was so complicated that this was just a > natural result of a confusion with some follow up changes on top. > >> I like your new code below as it explicitly takes reserve and cgroup >> accounting out of the picture for migration. Let me think about it >> for another day before providing a Reviewed-by. > > Thanks a lot!
You can add, Reviewed-by: Mike Kravetz <mike.krav...@oracle.com> I had some concerns about transferring huge page state during migration not specific to this patch, so I did a bunch of testing. -- Mike Kravetz
