Re: [PATCH 1/2] mm, pageblock: make sure pageblock won't exceed mem_sectioin

[David Hildenbrand <david@redhat.com>]

On 05.12.18 23:31, Wei Yang wrote:
> On Wed, Dec 05, 2018 at 03:37:33PM +0000, Mel Gorman wrote:
>> On Wed, Dec 05, 2018 at 12:08:20PM +0000, Wei Yang wrote:
>>> On Wed, Dec 05, 2018 at 11:15:13AM +0000, Mel Gorman wrote:
>>>> On Wed, Dec 05, 2018 at 05:19:04PM +0800, Wei Yang wrote:
>>>>> When SPARSEMEM is used, there is an indication that pageblock is not
>>>>> allowed to exceed one mem_section. Current code doesn't have this
>>>>> constrain explicitly.
>>>>>
>>>>> This patch adds this to make sure it won't.
>>>>>
>>>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>>>
>>>> Is this even possible? This would imply that the section size is smaller
>>>> than max order which would be quite a crazy selection for a sparesemem
>>>> section size. A lot of assumptions on the validity of PFNs within a
>>>> max-order boundary would be broken with such a section size. I'd be
>>>> surprised if such a setup could even boot, let alone run.
>>>
>>> pageblock_order has two definitions.
>>>
>>>     #define pageblock_order        HUGETLB_PAGE_ORDER
>>>
>>>     #define pageblock_order        (MAX_ORDER-1)
>>>
>>> If CONFIG_HUGETLB_PAGE is not enabled, pageblock_order is related to
>>> MAX_ORDER, which ensures it is smaller than section size.
>>>
>>> If CONFIG_HUGETLB_PAGE is enabled, pageblock_order is not related to
>>> MAX_ORDER. I don't see HUGETLB_PAGE_ORDER is ensured to be less than
>>> section size. Maybe I missed it?
>>>
>>
>> HUGETLB_PAGE_ORDER is less than MAX_ORDER on the basis that normal huge
>> pages (not gigantic) pages are served from the buddy allocator which is
>> limited by MAX_ORDER.
>>
> 
> Maybe I am lost here, I got one possible definition on x86.
> 
> #define pageblock_order		HUGETLB_PAGE_ORDER
> #define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
> #define HPAGE_SHIFT		PMD_SHIFT
> #define PMD_SHIFT	PUD_SHIFT

PMD_SHIFT is usually 21

arch/x86/include/asm/pgtable-3level_types.h:#define PMD_SHIFT   21
arch/x86/include/asm/pgtable_64_types.h:#define PMD_SHIFT       21

Unless CONFIG_PGTABLE_LEVELS <= 2

Then include/asm-generic/pgtable-nopmd.h will be used in
arch/x86/include/asm/pgtable_types.h
	#define PMD_SHIFT	PUD_SHIFT

In that case, also include/asm-generic/pgtable-nopmd.h is uses
	#define PUD_SHIFT	P4D_SHIFT

... include/asm-generic/pgtable-nop4d.h
	#define P4D_SHIFT	PGDIR_SHIFT


And that would be
arch/x86/include/asm/pgtable-2level_types.h:#define PGDIR_SHIFT 22

If I am not wrong.

So we would have pageblock_order = (22 - 12) = 10


> #define PUD_SHIFT	30
> 
> This leads to pageblock_order = (30 - 12) = 18 > MAX_ORDER  ?
> 
> What you mentioned sounds reasonable. A huge page should be less than
> MAX_ORDER, otherwise page allocator couldn't handle it. But I don't see
> the connection between MAX_ORDER and HUGETLB_PAGE_ORDER. Do we need to
> add a check on this? Or it already has similar contrain in code, but I
> missed it?
> 
>> -- 
>> Mel Gorman
>> SUSE Labs
> 


-- 

Thanks,

David / dhildenb