Re: [PATCH 2/2] mm: hugetlb_vmemmap: allow alloc_vmemmap_page_list() ignore watermarks

[Muchun Song <muchun.song@linux.dev>]

> On Sep 5, 2023, at 11:13, Yuan Can <yuancan@huawei.com> wrote:
> 
> The alloc_vmemmap_page_list() is called when hugetlb get freed, more memory
> will be returned to buddy after it succeed, thus work with __GFP_MEMALLOC
> to allow it ignore watermarks.

From the kernel document about __GFP_MEMALLOC, it says:

* %__GFP_MEMALLOC allows access to all memory. This should only be used when
* the caller guarantees the allocation will allow more memory to be freed
* very shortly e.g. process exiting or swapping. Users either should
* be the MM or co-ordinating closely with the VM (e.g. swap over NFS).
* Users of this flag have to be extremely careful to not deplete the reserve

I think we may deplete the reserve memory if a 1GB page is freed. It'll
be even worse if recent patchset[1] is merged, because the vmemmap pages
will be freed batched meaning those memory will not be freed in a very
short time (the cover letter has some numbers). So NACK.

* completely and implement a throttling mechanism which controls the
* consumption of the reserve based on the amount of freed memory.
* Usage of a pre-allocated pool (e.g. mempool) should be always considered
* before using this flag.

[1] https://lore.kernel.org/linux-mm/20230825190436.55045-1-mike.kravetz@oracle.com/

> 
> Signed-off-by: Yuan Can <yuancan@huawei.com>
> ---
> mm/hugetlb_vmemmap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 0485e471d224..dc0b9247a1f9 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -386,7 +386,7 @@ static int vmemmap_remap_free(unsigned long start, unsigned long end,
> static int alloc_vmemmap_page_list(unsigned long start, unsigned long end,
> 				   struct list_head *list)
> {
> - 	gfp_t gfp_mask = GFP_KERNEL | __GFP_RETRY_MAYFAIL;
> + 	gfp_t gfp_mask = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_MEMALLOC;
> 	unsigned long nr_pages = (end - start) >> PAGE_SHIFT;
> 	int nid = page_to_nid((struct page *)start);
> 	struct page *page, *next;
> -- 
> 2.17.1
>