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@xxxxxxxxxx> writes: >>> >>>> > > .... > >>> >>> I did report a variant of the issue at >>> >>> https://lore.kernel.org/linux-mm/20190514025354.9108-1-aneesh.kumar@xxxxxxxxxxxxx/ >>> >>> 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
