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/20200117111629898234212@xxxxxxxxx/ 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 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. --- >>>-- >>>Michal Hocko >>>SUSE Labs
