On 2020-01-16 at 16:07 HORIGUCHI NAOYA(堀口 直也) wrote: >Hi everyone, > >Thank you all for finding and digging the issue. > >> Summary >> ======= >> It 'looks' like the statement "MPOL_MF_STRICT is ignored on huge page >> mappings." is left over from the original mbind implementation. When >> the huge page migration support was added, I can not be sure if ignoring >> MPOL_MF_STRICT for huge pages during the verify/isolation phase was >> intentional. It seems like it was as the return value from >> isolate_huge_page() is ignored. > >This summary is totally correct. I've simply missed considering MPOL_MF_STRICT >flag when implementing hugetlb migration. As you pointed out, the discrepacy >between the manpage and the code is also due to the lack of updates on the >"MPOL_MF_STRICT is ignored on huge page mappings." statement. > >On Wed, Jan 15, 2020 at 01:59:14PM -0800, Yang Shi wrote: >> >> On 1/15/20 1:45 PM, Mike Kravetz wrote: >> > On 1/15/20 1:30 PM, Yang Shi wrote: >> > > On 1/15/20 1:07 PM, Mike Kravetz wrote: >> > > > What should we do? >> > > > ================== >> > > > 1) Nothing more than optimizations by Li Xinhai. Behavior that could be >> > > > seen as conflicting with man page has existed since v3.12 and I am >> > > > not aware of any complaints. >> > > > 2) In addition to optimizations by Li Xinhai, modify code to truly ignore >> > > > MPOL_MF_STRICT for huge page mappings. This would be fairly easy to do >> > > > after a failure of migrate_pages(). We could simply traverse the list >> > > > of pages that were not migrated looking for any non-hugetlb page. >> > > I don't think we can do this easily since migrate_pages() would put the migration failed hugetlb pages back to hugepage_activelist so there should not any hugetlb page reside on the pagelist regardless of failure if I read the code correctly. > >Although this behavior seems to me not prevent from finding non-hugetlb >pages in migration list, this is a difference in migration behavior between >normal pages and hugepages that might be better to be optimized. >Maybe hugepages failed to migrate should remain in migration list after >migrate_pages() returns and the should be put back via putback_movable_pages(). > >> > > >> > You are correct. I made an assumption without actually looking at the code. :( >> > >> > > Other than that traversing page list to look for a certain type page doesn't sound scalable to me. >> > > >> > > > 3) Remove the statement "MPOL_MF_STRICT is ignored on huge page mappings." >> > > > and modify code accordingly. >> > > > >> > > > My suggestion would be for 1 or 2. Thoughts? >> > > By rethinking the history (thanks again for digging into it), it sounds #3 should be more reasonable. It sounds like the behavior was changed since hugetlb migration was added but the man page was not updated to reflect the change. >> > > >> > Let's hope Naoya comments. My only concern with #3 is that we will be changing >> > behavior. I do not think many people (if any) depend on existing behavior. >> > However, you can never be sure. >> >> Yes, this would change the bahavior, but I don't see why we have to treat >> hugetlb specially nowadays with migration supported. > >(Option #1 is good for short term solution, but eventually) I agree with option #3. >We have no reason to handle hugetlb differently about MPOL_MF_STRICT flag. Thanks. Same thoughts for option #3, but it seems better not change current behavior. Add more about current behavior of code: - In unmap&move phase, there is no different behavior of handling hugepage and non-hugepage, that is when STRICT is set, report -EIO if any page can't move, when STRICT is not set, don't report when failed to move; - In isolation phase, STRICT is effective for non-hugepge, that means set STRICT alone will cause -EIO if found misplaced pages, and set STRICT with MOVE* will cause -EIO if failed to isolate pages; for hugepage, STRICT is ignored, it don't detect misplaced pages nor report -EIO if isolation failed. This patch don't change any part of current behavior, only avoids walking page table, where currently do nothing if STRICT is set alone. > >Thanks, >Naoya Horiguchi
