Date:   Thu, 22 Sep 2022 13:25:51 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Doug Berger <opendmb@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Muchun Song <songmuchun@...edance.com>,
        Oscar Salvador <osalvador@...e.de>,
        Michal Hocko <mhocko@...e.com>,
        David Hildenbrand <david@...hat.com>,
        Florian Fainelli <f.fainelli@...il.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] mm/hugetlb: hugepage migration enhancements

On 09/21/22 15:36, Doug Berger wrote:
> This patch set was included as patches [04/21-06/21] in my
> previous patch set to introduce Designated Movable Blocks [1].
> They were included there for context, but they have value on
> their own and are being resubmitted here for consideration on
> their own merits.
> 
> The alloc_contig_range() function attempts to isolate the free
> pages within the range and migrate the data from non-free pages
> within the range to allow those pages to become isolated free
> pages. If all of the pages in the range are on isolated free
> page lists they can be allocated to the caller.
> 
> When free hugepages are encountered in the range an attempt is
> made to allocate a new compound page to become a replacement
> hugepage and to dissolve the free hugepage so that its pages
> within isolated pageblocks can be added to the isolated free
> page lists. Hugepages that are not free and encountered within
> the range must be migrated out of the range and dissolved to
> allow the underlying pages to be added to the isolated free
> page lists.
> 
> Moving the data from hugepages within the range and freeing the
> hugepages is not sufficient since the allocation of migration
> target hugepages is allowed to come from free hugepages that may
> contain isolated pageblocks and freed hugepages will not be on
> isolated free page lists so the alloc_contig_range() will fail.

Thanks for looking into this!  I am adding Oscar, Michal and David on
Cc: as they have looked at similar issues in the past.

Before jumping into the details of your changes, I just want to make one
observation.  When migrating (or dissolving) hugetlb pages that are in a
range operated on by alloc_contig_range(), we should not change the number
of hugetlb pages available as noted here.  This includes the number of
total huge pages and the number of huge pages on the node.  Therefore,
we should allocate another huge page from buddy either as the migration
target or to take the place of the dissolved page.

For free huge pages, we do this via alloc_and_dissolve_huge_page.  IIUC,
there are no issues with this code path?

As noted above, for pages to be migrated we first try to use an existing
free huge page as the target.  Quite some time ago, Michal added code to
allocate a new page from buddy as the target if no free huge pages were
available.  This change also included a special flag to dissolve the
source huge page when it is freed.  It seems like this is the exact
behavior we want here?  I wonder if it might be easier just to use this
existing code?
-- 
Mike Kravetz

> To address these shortcommings the HPG_dissolve hugepage flag is
> introduced to tag hugepages that must be dissolved when they are
> freed so that their underlying pages can be moved to the page
> allocator's free lists. This prevents hugepages that have had
> their data migrated to new hugepages from being made available
> to subsequent hugepage allocations and allows the isolated free
> page test of alloc_contig_range() to succeed.
> 
> Dissolved hugepages must be replaced with new hugepages to keep
> the hugetlbfs balanced. To support blocking allocation a new
> workqueue in introduced that is analogous to the workqueue
> introduced to support the hugetlb vmemmap optimization. This new
> workqueue allows the allocation and dissolution of the hugepage
> to be offloaded to a separate context from the freeing of the
> hugepage. The sync_hugetlb_dissolve() function is introduced to
> allow outstanding dissolution of hugepages to complete before
> the isolated free page check is made by alloc_contig_range().
> 
> In addition, a check is added to hugepage allocation to prevent
> free hugepages within an isolated pageblock range from being
> used to satisfy migration target allocations preventing circular
> migrations.
> 
> [1] https://lore.kernel.org/linux-mm/20220913195508.3511038-1-opendmb@gmail.com/
> 
> Doug Berger (3):
>   mm/hugetlb: refactor alloc_and_dissolve_huge_page
>   mm/hugetlb: allow migrated hugepage to dissolve when freed
>   mm/hugetlb: add hugepage isolation support
> 
>  include/linux/hugetlb.h |   5 ++
>  mm/hugetlb.c            | 161 +++++++++++++++++++++++++++++++---------
>  mm/migrate.c            |   1 +
>  mm/page_alloc.c         |   1 +
>  4 files changed, 131 insertions(+), 37 deletions(-)
> 
> 
> base-commit: b90cb1053190353cc30f0fef0ef1f378ccc063c5
> -- 
> 2.25.1
> 

Powered by blists - more mailing lists