Re: [PATCH v2 0/6] mm/memory_hotplug: Consider all zones when removing memory

[David Hildenbrand <david@redhat.com>]

On 26.08.19 18:01, Aneesh Kumar K.V wrote:
> On 8/26/19 9:13 PM, David Hildenbrand wrote:
>> On 26.08.19 16:53, Aneesh Kumar K.V wrote:
>>> David Hildenbrand <david@redhat.com> writes:
>>>
>>>>
> 
> ....
> 
>>>
>>> I did report a variant of the issue at
>>>
>>> https://lore.kernel.org/linux-mm/20190514025354.9108-1-aneesh.kumar@linux.ibm.com/
>>>
>>> This patch series still doesn't handle the fact that struct page backing
>>> the start_pfn might not be initialized. ie, it results in crash like
>>> below
>>
>> Okay, that's a related but different issue I think.
>>
>> I can see that current shrink_zone_span() might read-access the
>> uninitialized struct page of a PFN if
>>
>> 1. The zone has holes and we check for "zone all holes". If we get
>> pfn_valid(pfn), we check if "page_zone(pfn_to_page(pfn)) != zone".
>>
>> 2. Via find_smallest_section_pfn() / find_biggest_section_pfn() find a
>> spanned pfn_valid(). We check
>> - pfn_to_nid(start_pfn) != nid
>> - zone != page_zone(pfn_to_page(start_pfn)
>>
>> So we don't actually use the zone/nid, only use it to sanity check. That
>> might result in false-positives (not that bad).
>>
>> It all boils down to shrink_zone_span() not working only on active
>> memory, for which the PFN is not only valid but also initialized
>> (something for which we need a new section flag I assume).
>>
>> Which access triggers the issue you describe? pfn_to_nid()?
>>
>>>
>>>      pc: c0000000004bc1ec: shrink_zone_span+0x1bc/0x290
>>>      lr: c0000000004bc1e8: shrink_zone_span+0x1b8/0x290
>>>      sp: c0000000dac7f910
>>>     msr: 800000000282b033
>>>    current = 0xc0000000da2fa000
>>>    paca    = 0xc00000000fffb300   irqmask: 0x03   irq_happened: 0x01
>>>      pid   = 1224, comm = ndctl
>>> kernel BUG at /home/kvaneesh/src/linux/include/linux/mm.h:1088!
>>> Linux version 5.3.0-rc6-17495-gc7727d815970-dirty (kvaneesh@ltc-boston123) (gcc version 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04.1)) #183 SMP Mon Aug 26 09:37:32 CDT 2019
>>> enter ? for help
>>
>> Which exact kernel BUG are you hitting here? (my tree doesn't seem t
>> have any BUG statement around  include/linux/mm.h:1088). 
> 
> 
> 
> This is against upstream linus with your patches applied.

I'm

> 
> 
> static inline int page_to_nid(const struct page *page)
> {
> 	struct page *p = (struct page *)page;
> 
> 	return (PF_POISONED_CHECK(p)->flags >> NODES_PGSHIFT) & NODES_MASK;
> }
> 
> 
> #define PF_POISONED_CHECK(page) ({					\
> 		VM_BUG_ON_PGFLAGS(PagePoisoned(page), page);		\
> 		page; })
> #
> 
> 
> It is the node id access.

A right. A temporary hack would be to assume in these functions
(shrink_zone_span() and friends) that we might have invalid NIDs /
zonenumbers and simply skip these. After all we're only using them for
finding zone boundaries. Not what we ultimately want, but I think until
we have a proper SECTION_ACTIVE, it might take a while.

-- 

Thanks,

David / dhildenb