Re: [External] Re: [PATCH v15 4/8] mm: hugetlb: alloc the vmemmap pages associated with each HugeTLB page
From: Michal Hocko <mhocko@suse.com>
Date: 2021-02-15 10:33:58
Also in:
linux-fsdevel, linux-mm, lkml
On Mon 15-02-21 18:05:06, Muchun Song wrote:
On Fri, Feb 12, 2021 at 11:32 PM Michal Hocko [off-list ref] wrote:
[...]
quoted
quoted
+int alloc_huge_page_vmemmap(struct hstate *h, struct page *head) +{ + int ret; + unsigned long vmemmap_addr = (unsigned long)head; + unsigned long vmemmap_end, vmemmap_reuse; + + if (!free_vmemmap_pages_per_hpage(h)) + return 0; + + vmemmap_addr += RESERVE_VMEMMAP_SIZE; + vmemmap_end = vmemmap_addr + free_vmemmap_pages_size_per_hpage(h); + vmemmap_reuse = vmemmap_addr - PAGE_SIZE; + + /* + * The pages which the vmemmap virtual address range [@vmemmap_addr, + * @vmemmap_end) are mapped to are freed to the buddy allocator, and + * the range is mapped to the page which @vmemmap_reuse is mapped to. + * When a HugeTLB page is freed to the buddy allocator, previously + * discarded vmemmap pages must be allocated and remapping. + */ + ret = vmemmap_remap_alloc(vmemmap_addr, vmemmap_end, vmemmap_reuse, + GFP_ATOMIC | __GFP_NOWARN | __GFP_THISNODE);I do not think that this is a good allocation mode. GFP_ATOMIC is a non sleeping allocation and a medium memory pressure might cause it to fail prematurely. I do not think this is really an atomic context which couldn't afford memory reclaim. I also do not think we want to grantBecause alloc_huge_page_vmemmap is called under hugetlb_lock now. So using GFP_ATOMIC indeed makes the code more simpler.
You can have a preallocated list of pages prior taking the lock. Moreover do we want to manipulate vmemmaps from under spinlock in general. I have to say I have missed that detail when reviewing. Need to think more.
From the document of the kernel, I learned that __GFP_NOMEMALLOC can be used to explicitly forbid access to emergency reserves. So if we do not want to use the reserve memory. How about replacing it to GFP_ATOMIC | __GFP_NOMEMALLOC | __GFP_NOWARN | __GFP_THISNODE
The whole point of GFP_ATOMIC is to grant access to memory reserves so the above is quite dubious. If you do not want access to memory reserves then use GFP_NOWAIT instead. But failures are much more easier to happen then. NOMEMALLOC is meant to be used from paths which are allowed to consume memory reserves - e.g. when invoked from the memory reclaim path. -- Michal Hocko SUSE Labs