On 2020-01-18 at 11:11 Li Xinhai wrote: >On 2020-01-17 at 11:16 Li Xinhai wrote: >>On 2020-01-16 at 23:38 Li Xinhai wrote: >>>On 2020-01-16 at 23:18 Michal Hocko wrote: >>>>On Thu 16-01-20 21:50:34, Li Xinhai wrote: >>>>> On 2020-01-16 at 17:56 Michal Hocko wrote: >>>>> >On Thu 16-01-20 04:11:25, Li Xinhai wrote: >>>>> >> Checking hstate at early phase when isolating page, instead of during >>>>> >> unmap and move phase, to avoid useless isolation. >>>>> > >>>>> >Could you be more specific what you mean by isolation and why does it >>>>> >matter? The patch description should really explain _why_ the change is >>>>> >needed or desirable. >>>>> >>>>> The changelog can be improved: >>>>> >>>>> vma_migratable() is called to check if pages in vma can be migrated >>>>> before go ahead to isolate, unmap and move pages. For hugetlb pages, >>>>> hugepage_migration_supported(struct hstate *h) is one factor which >>>>> decide if migration is supported. In current code, this function is called >>>>> from unmap_and_move_huge_page(), after isolating page has >>>>> completed. >>>>> This patch checks hstate from vma_migratable() and avoids isolating pages >>>>> which are not supported. >>>> >>>>This still explains what but not why this is relevant. If by isolating >>>>pages you mean isolate_lru_page then this really a noop for hugetlb >>>>pages. Or do I still misread your changelog? >>> >>>I mean isolate_huge_page will queue pages for moving, and >>>unmap_and_move_huge_page will call >>>hugepage_migration_supported then refuse moving. >>> >> >>Forgot to mention that this patch has no relevant with this one >>https://patchwork.kernel.org/patch/11331639/, ; >> >>Code change at here is common for avoids walking page table and >>isolate hugepage in case architecture or page size are not supported >>for migration. Comments from code are copied here: >> >>static int unmap_and_move_huge_page(...) >>{ >> /* >> * Migratability of hugepages depends on architectures and their size. >> * This check is necessary because some callers of hugepage migration >> * like soft offline and memory hotremove don't walk through page >> * tables or check whether the hugepage is pmd-based or not before >> * kicking migration. >> */ >> if (!hugepage_migration_supported(page_hstate(hpage))) { >> putback_active_hugepage(hpage); >> return -ENOSYS; >> } >>} >> >>For current code change, we are able to know the 'hstate' because we have 'vma', so >>do early check instead of later. >> > https://lore.kernel.org/linux-mm/1579147885-23511-1-git-send-email-lixinhai.lxh@xxxxxxxxx/ Add context information >Revise with more details on changelog for reason why this patch >is need. Thanks for your comments. > >--- >vma_migratable() is called to check if pages in vma can be migrated >before go ahead to further actions. Currently it is used in below code >path: >- task_numa_work (kernel\sched\fair.c) >- mbind (mm\mempolicy.c) >- move_pages (mm\migrate.c) > >For hugetlb mapping, vma is migratable or not is determined by: >- CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION >- arch_hugetlb_migration_supported - arch_hugetlb_migration_supported [1] > >Issue: current code only checks for CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION. > >This patch checks the two factors to impove code logic. Besides, caller >of vma_migratable can take action properly in case architecture don't >support migration, e.g., mbind don't go further to try isolating/moving >pages, but currently it do continue because vma_migratable say yes. > >No adding for Fix tag, since vma_migratable was implemented before >arch_hugetlb_migration_supported, it is up to the caller to use it. [1] This interface is introduced by commit e693de186414 (mm/hugetlb: enable arch specific huge page size support for migration), which allow improvement of this path. >--- > > >>>>-- >>>>Michal Hocko >>>>SUSE Labs
