Re: [PATCH v4] mm/mempolicy,hugetlb: Checking hstate for hugetlbfs page in vma_migratable

[Michal Hocko <mhocko@kernel.org>]

On Mon 20-01-20 23:37:25, Li Xinhai wrote:
[...]
> Changelog is updated as below, thanks for comments:
> ---
> mm/mempolicy: Checking hugepage migration is supported by arch in vma_migratable
> 
> 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
> - mbind
> - move_pages
> 
> For hugetlb mapping, whether 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,
> which express less accurate semantics of vma_migratable(). (note that
> current code in vma_migratable don't cause failure or bug because
> unmap_and_move_huge_page() will catch unsupported hugepage and handle it
> properly)
> 
> This patch checks the two factors for impoveing code logic and
> robustness. It will enable early bail out of hugepage migration procedure,
> but because currently all architecture supporting hugepage migration is able
> to support all page size, we would not see performance gain with this patch
> applied.

This looks definitely better than the original one. I hope it is more
clear to you what I meant by a better description for the justification.
I would just add that the no code should use
CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION directly and use
arch_hugetlb_migration_supported instead. This will be the case after
this patch.

Please keep in mind that changelogs are really important and growing in
importance as the code gets more complicated over time. It is much more
easier to see what the patch does because reading diffs and the code is
easy but the lack of motivation is what people usually fighting with.
-- 
Michal Hocko
SUSE Labs