On Mon 15-02-21 19:51:26, Muchun Song wrote: > On Mon, Feb 15, 2021 at 6:33 PM Michal Hocko <[email protected]> wrote: > > > > On Mon 15-02-21 18:05:06, Muchun Song wrote: > > > On Fri, Feb 12, 2021 at 11:32 PM Michal Hocko <[email protected]> wrote: > > [...] > > > > > +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 grant > > > > > > Because 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. > > A discussion about this can refer to here: > > https://patchwork.kernel.org/project/linux-mm/patch/[email protected]/
I do not see any real response to the pre-allocation argument except that put_page can be called from an atomic context. Which might be true in general but it is not the case for hugetlb pages. hugetlb_lock would have to be irq safe otherwise. Also the whole operation can be scheduled to a kworker context for a stronger allocation context. -- Michal Hocko SUSE Labs
