Re: [RFC PATCH] mm, page_alloc: avoid page_to_pfn() in move_freepages()

[Qian Cai <cai@lca.pw>]

> On Nov 27, 2019, at 9:39 AM, Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> 
> 
> 
> On 2019/11/27 22:13, Michal Hocko wrote:
>> On Wed 27-11-19 21:13:00, Kefeng Wang wrote:
>>> 
>>> 
>>> On 2019/11/27 19:47, Michal Hocko wrote:
>>>> On Wed 27-11-19 18:28:00, Kefeng Wang wrote:
>>>>> The start_pfn and end_pfn are already available in move_freepages_block(),
>>>>> pfn_valid_within() should validate pfn first before touching the page,
>>>>> or we might access an unitialized page with CONFIG_HOLES_IN_ZONE configs.
>>>>> 
>>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>>> Cc: Michal Hocko <mhocko@suse.com>
>>>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>>> ---
>>>>> 
>>>>> Here is an oops in 4.4(arm64 enabled CONFIG_HOLES_IN_ZONE),
>>>> 
>>>> Is this reproducible with the current upstream kernel? There were large
>>>> changes in this aread since 4.4
>>> 
>>> Our inner tester found this oops twice, but couldn't be reproduced for now,
>>> even in 4.4 kernel, still trying...
>>> 
>>> But the page_to_pfn() shouldn't be used in move_freepages(), right? ; )
>> 
>> Well, I do agree that going back and forth between page and pfn is ugly.
>> So this as a cleanup makes sense to me. But you are trying to fix a bug
>> and that bug should be explained. NULL ptr dereference sounds like a
>> memmap is not allocated for the particular pfn and this is a bit
>> unexpected even with holes, at least on x86, maybe arm64 allows that.
>> But the changelog should be clear about all this rather than paper over
>> a deeper problem potentially. Please also make sure to involve arm64
>> people.
> 
> I'm still trying to reproduce it on 4.4 and 5.4, add Catalin, Will Mark,
> could you give some advice on it, thanks.
> 
> https://lore.kernel.org/linux-mm/54064878-ea85-247a-3382-b96ddf97c667@huawei.com/T/#m87c545730a0a00c45e042937593c59f6552d1246
> 
> note:
> We backport numa patches into 4.4, so the CONFIG_HOLES_IN_ZONE is enabled.
> 
> # CONFIG_NUMA is not set
> CONFIG_HOLES_IN_ZONE=y
> 
> CONFIG_SPARSEMEM_MANUAL=y
> CONFIG_SPARSEMEM=y
> CONFIG_HAVE_MEMORY_PRESENT=y
> CONFIG_SPARSEMEM_EXTREME=y
> CONFIG_SPARSEMEM_VMEMMAP_ENABLE=y
> CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER=y
> # CONFIG_SPARSEMEM_VMEMMAP is not set

See the commit b13bc35193d9 (“mm/hotplug: invalid PFNs from pfn_to_online_page()”)
where it lists a lot of code churn in that space that might indicate there are many commits
missing in your kernel.