[PATCH 0/2] Unlock 1GB-hugetlb on x86_64

[PATCH 0/2] Unlock 1GB-hugetlb on x86_64

[Oscar Salvador]

RFC -> V1:
	- Split up the patch
	- Added Michal's Acked-by

The RFC version of this patch was discussed here [1], and it did not find any
objection.
I decided to split up the former patch because one of the changes enables
offlining operation for 1GB-hugetlb pages, while the other change is a mere
cleanup.

Patch1 contains all the information regarding 1GB-hugetlb pages change.

[1] https://lore.kernel.org/linux-mm/20190221094212.16906-1-osalvador@suse.de/

Oscar Salvador (2):
  mm,memory_hotplug: Unlock 1GB-hugetlb on x86_64
  mm,memory_hotplug: Drop redundant hugepage_migration_supported check

 mm/memory_hotplug.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

-- 
2.13.7

[PATCH 1/2] mm,memory_hotplug: Unlock 1GB-hugetlb on x86_64

[Oscar Salvador]

On x86_64, 1GB-hugetlb pages could never be offlined due to the fact
that hugepage_migration_supported() returned false for PUD_SHIFT.
So whenever we wanted to offline a memblock containing a gigantic
hugetlb page, we never got beyond has_unmovable_pages() check.
This changed with [1], where now we also return true for PUD_SHIFT.

After that patch, the check in has_unmovable_pages() and scan_movable_pages()
returned true, but we still had a final barrier in do_migrate_range():

if (compound_order(head) > PFN_SECTION_SHIFT) {
	ret = -EBUSY;
	break;
}

This is not really nice, and we do not really need it.
It is perfectly possible to migrate a gigantic page as long as another node has
a spare gigantic page for us.
In alloc_huge_page_nodemask(), we calculate the __real__ number of free pages,
and if any, we try to dequeue one from another node.

This all works fine when we do have another node with a spare gigantic page,
but if that is not the case, alloc_huge_page_nodemask() ends up calling
alloc_migrate_huge_page() which bails out if the wanted page is gigantic.
That is mainly because finding a 1GB (or even 16GB on powerpc) contiguous
memory is quite unlikely when the system has been running for a while.

In that situation, we will keep looping forever because scan_movable_pages()
will give us the same page and we will fail again because there is no node
where we can dequeue a gigantic page from.
This is not nice, and it has been raised that we might want to treat -ENOMEM
as a fatal error in do_migrate_range(), but this has to be checked further.

Anyway, I would tend say that this is the administrator's job, to make sure
that the system can keep up with the memory to be offlined, so that would mean
that if we want to use gigantic pages, make sure that the other nodes have at
least enough gigantic pages to keep up in case we need to offline memory.

Just for the sake of completeness, this is one of the tests done:

 # echo 1 > /sys/devices/system/node/node1/hugepages/hugepages-1048576kB/nr_hugepages
 # echo 1 > /sys/devices/system/node/node2/hugepages/hugepages-1048576kB/nr_hugepages

 # cat /sys/devices/system/node/node1/hugepages/hugepages-1048576kB/nr_hugepages
   1
 # cat /sys/devices/system/node/node1/hugepages/hugepages-1048576kB/free_hugepages
   1

 # cat /sys/devices/system/node/node2/hugepages/hugepages-1048576kB/nr_hugepages
   1
 # cat /sys/devices/system/node/node2/hugepages/hugepages-1048576kB/free_hugepages
   1

 (hugetlb1gb is a program that maps 1GB region using MAP_HUGE_1GB)

 # numactl -m 1 ./hugetlb1gb
 # cat /sys/devices/system/node/node1/hugepages/hugepages-1048576kB/free_hugepages
   0
 # cat /sys/devices/system/node/node2/hugepages/hugepages-1048576kB/free_hugepages
   1

 # offline node1 memory
 # cat /sys/devices/system/node/node2/hugepages/hugepages-1048576kB/free_hugepages
   0

[1] https://lore.kernel.org/patchwork/patch/998796/

Signed-off-by: Oscar Salvador <osalvador@suse.de>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 mm/memory_hotplug.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index a9d5787044e1..0f479c710615 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1387,10 +1387,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 
 		if (PageHuge(page)) {
 			struct page *head = compound_head(page);
-			if (compound_order(head) > PFN_SECTION_SHIFT) {
-				ret = -EBUSY;
-				break;
-			}
 			pfn = page_to_pfn(head) + (1<<compound_order(head)) - 1;
 			isolate_huge_page(head, &source);
 			continue;
-- 
2.13.7

[PATCH 2/2] mm,memory_hotplug: Drop redundant hugepage_migration_supported check

[Oscar Salvador]

has_unmovable_pages() does alreay check whether the hugetlb page supports
migration, so all non-migrateable hugetlb pages should have been caught there.
Let us drop the check from scan_movable_pages() as is redundant.

Signed-off-by: Oscar Salvador <osalvador@suse.de>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 mm/memory_hotplug.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 0f479c710615..2dfd9a0b0832 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1346,8 +1346,7 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
 		if (!PageHuge(page))
 			continue;
 		head = compound_head(page);
-		if (hugepage_migration_supported(page_hstate(head)) &&
-		    page_huge_active(head))
+		if (page_huge_active(head))
 			return pfn;
 		skip = (1 << compound_order(head)) - (page - head);
 		pfn += skip - 1;
-- 
2.13.7

Re: [PATCH 2/2] mm,memory_hotplug: Drop redundant hugepage_migration_supported check

[David Hildenbrand]

On 04.03.19 09:51, Oscar Salvador wrote:
> has_unmovable_pages() does alreay check whether the hugetlb page supports
> migration, so all non-migrateable hugetlb pages should have been caught there.
> Let us drop the check from scan_movable_pages() as is redundant.
> 
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> Acked-by: Michal Hocko <mhocko@suse.com>
> ---
>  mm/memory_hotplug.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 0f479c710615..2dfd9a0b0832 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1346,8 +1346,7 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end)
>  		if (!PageHuge(page))
>  			continue;
>  		head = compound_head(page);
> -		if (hugepage_migration_supported(page_hstate(head)) &&
> -		    page_huge_active(head))
> +		if (page_huge_active(head))
>  			return pfn;
>  		skip = (1 << compound_order(head)) - (page - head);
>  		pfn += skip - 1;
> 

Yes, it would actually be a BUG once we reach that point and we suddenly
have !hugepage_migration_supported() in my opinion.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [PATCH 0/2] Unlock 1GB-hugetlb on x86_64

[Oscar Salvador]

On Mon, Mar 04, 2019 at 09:51:45AM +0100, Oscar Salvador wrote:
> RFC -> V1:
> 	- Split up the patch
> 	- Added Michal's Acked-by
> 
> The RFC version of this patch was discussed here [1], and it did not find any
> objection.
> I decided to split up the former patch because one of the changes enables
> offlining operation for 1GB-hugetlb pages, while the other change is a mere
> cleanup.
> 
> Patch1 contains all the information regarding 1GB-hugetlb pages change.
> 
> [1] https://lore.kernel.org/linux-mm/20190221094212.16906-1-osalvador@suse.de/
> 
> Oscar Salvador (2):
>   mm,memory_hotplug: Unlock 1GB-hugetlb on x86_64
>   mm,memory_hotplug: Drop redundant hugepage_migration_supported check
> 
>  mm/memory_hotplug.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)

Andrew, now that the merge window is closed, do you want me to re-base and
re-send this, or will you pick it up as is?

Thanks

-- 
Oscar Salvador
SUSE L3