Messages in this thread | | | Date | Thu, 22 Sep 2022 16:14:27 -0700 | Subject | Re: [PATCH 0/3] mm/hugetlb: hugepage migration enhancements | From | 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
| |