On Wed 10-10-18 08:39:22, Anshuman Khandual wrote:
[...]
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 9df1d59..4bcbf1e 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -504,6 +504,16 @@ static inline bool hugepage_migration_supported(struct hstate *h)
> return arch_hugetlb_migration_supported(h);
> }
>
> +static inline bool hugepage_movable_supported(struct hstate *h)
> +{
> + if (!hugepage_migration_supported(h)) --> calls arch override restricting the set
> + return false;
> +
> + if (hstate_is_gigantic(h) --------> restricts the set further
> + return false;
> + return true;
> +}
> +
> static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
> struct mm_struct *mm, pte_t *pte)
> {
> @@ -600,6 +610,11 @@ static inline bool hugepage_migration_supported(struct hstate *h)
> return false;
> }
>
> +static inline bool hugepage_movable_supported(struct hstate *h)
> +{
> + return false;
> +}
> +
> static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
> struct mm_struct *mm, pte_t *pte)
> {
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 3c21775..a5a111d 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -919,7 +919,7 @@ static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask,
> /* Movability of hugepages depends on migration support. */
> static inline gfp_t htlb_alloc_mask(struct hstate *h)
> {
> - if (hugepage_migration_supported(h))
> + if (hugepage_movable_supported(h))
> return GFP_HIGHUSER_MOVABLE;
> else
> return GFP_HIGHUSER;
Exactly what I've had in mind. It would be great to have a comment in
hugepage_movable_supported to explain why we are not supporting giga
pages even though they are migrateable and why we need that distinction.
> The above patch is in addition to the following later patch in the series.
[...]
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 9c1b77f..9df1d59 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -479,18 +479,29 @@ static inline pgoff_t basepage_index(struct page *page)
> extern int dissolve_free_huge_page(struct page *page);
> extern int dissolve_free_huge_pages(unsigned long start_pfn,
> unsigned long end_pfn);
> -static inline bool hugepage_migration_supported(struct hstate *h)
> -{
> +
> #ifdef CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION
> +#ifndef arch_hugetlb_migration_supported
> +static inline bool arch_hugetlb_migration_supported(struct hstate *h)
> +{
> if ((huge_page_shift(h) == PMD_SHIFT) ||
> (huge_page_shift(h) == PUD_SHIFT) ||
> (huge_page_shift(h) == PGDIR_SHIFT))
> return true;
> else
> return false;
> +}
> +#endif
> #else
> +static inline bool arch_hugetlb_migration_supported(struct hstate *h)
> +{
> return false;
> +}
> #endif
> +
> +static inline bool hugepage_migration_supported(struct hstate *h)
> +{
> + return arch_hugetlb_migration_supported(h);
> }
Yes making hugepage_migration_supported to have an arch override is
definitely the right thing to do. Whether the above approach rather than
a weak symbol is better is a matter of taste and I do not feel strongly
about that.
--
Michal Hocko
SUSE Labs
