On September 14, 2022 10:43:52 AM GMT+01:00, Christophe Leroy <christophe.leroy@xxxxxxxxxx> wrote:
>
>
>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
>}
This iterates over all PFNs in the highmem range and skips those in holes.
for_each_free_mem_range() skips the holes altogether.
Largely, I think we need to move, say, arm's version to mm/ and use it everywhere, except, perhaps, x86.
>Christophe
--
Sincerely yours,
Mike
