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! -- Michal Hocko SUSE Labs
