Re: Fragmented physical memory on powerpc/32

[Christophe Leroy]

Le 14/09/2022 à 11:32, Mike Rapoport a écrit :
> On Tue, Sep 13, 2022 at 02:36:13PM +0200, Christophe Leroy wrote:
>>
>>
>> Le 13/09/2022 à 08:11, Christophe Leroy a écrit :
>>>
>>>
>>> Le 12/09/2022 à 23:16, Pali Rohár a écrit :
>>>>>
>>>>> My guess would be that something went wrong in the linear map
>>>>> setup, but it
>>>>> won't hurt running with "memblock=debug" added to the kernel
>>>>> command line
>>>>> to see if there is anything suspicious there.
>>>>
>>>> Here is boot log on serial console with memblock=debug command line:
>>>>
>>> ...
>>>>
>>>> Do you need something more for debug?
>>>
>>> Can you send me the 'vmlinux' used to generate the above Oops so that I
>>> can see exactly where we are in function mem_init().
>>>
>>> And could you also try without CONFIG_HIGHMEM just in case.
>>>
>>
>> I looked at the vmlinux you sent me, the problem is in the loop for highmem
>> in mem_init(). It crashes in the call to free_highmem_page()
>>
>> #ifdef CONFIG_HIGHMEM
>> 	{
>> 		unsigned long pfn, highmem_mapnr;
>>
>> 		highmem_mapnr = lowmem_end_addr >> PAGE_SHIFT;
>> 		for (pfn = highmem_mapnr; pfn < max_mapnr; ++pfn) {
>> 			phys_addr_t paddr = (phys_addr_t)pfn << PAGE_SHIFT;
>> 			struct page *page = pfn_to_page(pfn);
>> 			if (!memblock_is_reserved(paddr))
>> 				free_highmem_page(page);
>> 		}
>> 	}
>> #endif /* CONFIG_HIGHMEM */
>>
>>
>> As far as I can see in the memblock debug lines, the holes don't seem to be
>> marked as reserved by memblock. So it is above valid ? Other architectures
>> seem to do differently.
>>
>> Can you try by replacing !memblock_is_reserved(paddr) by
>> memblock_is_memory(paddr) ?
> 
> The holes should not be marked as reserved, we just need to loop over the
> memory ranges rather than over pfns. Then the holes will be taken into
> account.
> 
> I believe arm and xtensa got this right:
> 
> (from arch/arm/mm/init.c)
> 
> static void __init free_highpages(void)
> {
> #ifdef CONFIG_HIGHMEM
> 	unsigned long max_low = max_low_pfn;
> 	phys_addr_t range_start, range_end;
> 	u64 i;
> 
> 	/* set highmem page free */
> 	for_each_free_mem_range(i, NUMA_NO_NODE, MEMBLOCK_NONE,
> 				&range_start, &range_end, NULL) {
> 		unsigned long start = PFN_UP(range_start);
> 		unsigned long end = PFN_DOWN(range_end);
> 
> 		/* Ignore complete lowmem entries */
> 		if (end <= max_low)
> 			continue;
> 
> 		/* Truncate partial highmem entries */
> 		if (start < max_low)
> 			start = max_low;
> 
> 		for (; start < end; start++)
> 			free_highmem_page(pfn_to_page(start));
> 	}
> #endif
> }
> 


And what about the way MIPS does it ?

static inline void __init mem_init_free_highmem(void)
{
#ifdef CONFIG_HIGHMEM
	unsigned long tmp;

	if (cpu_has_dc_aliases)
		return;

	for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
		struct page *page = pfn_to_page(tmp);

		if (!memblock_is_memory(PFN_PHYS(tmp)))
			SetPageReserved(page);
		else
			free_highmem_page(page);
	}
#endif
}


Christophe