On 10/14/2019 06:30 PM, Michal Hocko wrote:
> 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.
Sure. Just to confirm again, the checks on pfns in pfn_range_valid_gigantic()
while looking for contiguous memory is generic enough in it's current form ?
static bool pfn_range_valid_gigantic(struct zone *z,
unsigned long start_pfn, unsigned long nr_pages)
{
unsigned long i, end_pfn = start_pfn + nr_pages;
struct page *page;
for (i = start_pfn; i < end_pfn; i++) {
if (!pfn_valid(i))
return false;
page = pfn_to_page(i);
if (page_zone(page) != z)
return false;
if (PageReserved(page))
return false;
if (page_count(page) > 0)
return false;
if (PageHuge(page))
return false;
}
return true;
}
