Re: + mm-hugetlb-make-alloc_gigantic_page-available-for-general-use.patch added to -mm tree

[Michal Hocko <mhocko@kernel.org>]

On Mon 14-10-19 18:23:00, Anshuman Khandual wrote:
> 
> 
> On 10/14/2019 05:47 PM, Michal Hocko wrote:
> > On Fri 11-10-19 13:29:32, Andrew Morton wrote:
> >> alloc_gigantic_page() implements an allocation method where it scans over
> >> various zones looking for a large contiguous memory block which could not
> >> have been allocated through the buddy allocator.  A subsequent patch which
> >> tests arch page table helpers needs such a method to allocate PUD_SIZE
> >> sized memory block.  In the future such methods might have other use cases
> >> as well.  So alloc_gigantic_page() has been split carving out actual
> >> memory allocation method and made available via new
> >> alloc_gigantic_page_order().
> > 
> > You are exporting a helper used for hugetlb internally. Is this really
> 
> Right, because the helper i.e alloc_gigantic_page() is generic enough which
> scans over various zones to allocate a page order which could not be allocated
> through the buddy. Only thing which is HugeTLB specific in there, is struct
> hstate from where the order is derived with huge_page_order(). Otherwise it
> is very generic.
> 
> > what is needed? I haven't followed this patchset but don't you simply
> 
> Originally I had just implemented similar allocator inside the test itself
> but then figured that alloc_gigantic_page() can be factored out to create
> a generic enough allocator helper.
> 
> > need a generic 1GB allocator? If yes then you should be looking at
> 
> The test needs a PUD_SIZE allocator.
> 
> > alloc_contig_range.
> 
> IIUC alloc_contig_range() requires (start pfn, end pfn) for the region to be
> allocated. But before that all applicable zones need to be scanned to figure
> out any available and suitable pfn range for alloc_contig_range() to try. In
> this case pfn_range_valid_gigantic() check seemed reasonable while scanning
> the zones.
> 
> If pfn_range_valid_gigantic() is good enough or could be made more generic,
> then the new factored alloc_gigantic_page_order() could be made a helper in
> mm/page_alloc.c

OK, thanks for the clarification. This all means that this patch is not
the right approach. If you need a more generic alloc_contig_range then
add it to page_alloc.c and make it completely independent on the hugetlb
config and the code. Hugetlb allocator can reuse that helper.
-- 
Michal Hocko
SUSE Labs