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
