Re: [PATCH 0/3] mm/hugetlb: hugepage migration enhancements

[Doug Berger]

On 9/22/2022 1:25 PM, Mike Kravetz wrote:
> 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.
Thanks for helping to get the right eyeballs looking at this.

> 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?
Yes, I did not observe any issue with the the free hugepage handling 
except that your recent improvements with freezing allocated pages 
(thanks for those) will make merging patch 1 more difficult ;).

> 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?
Yes, the temporary page flag can be made to work here and I did 
experiment with it, but it is dependent on the current design decisions.
I decided to move to the approach suggested here because I believe it 
could conceivably scale if support for migrating gigantic pages is 
desired in the future. The circular dependency on alloc_contig_range 
will likely keep that from happening, but that was my thinking.

Thanks for taking the time,
-Doug