Re: [bug, 5.2.16] kswapd/compaction null pointer crash [was Re: xfs_inode not reclaimed/memory leak on 5.2.16]

[Vlastimil Babka <vbabka@suse.cz>]

On 10/7/19 3:28 PM, Vlastimil Babka wrote:
> On 10/1/19 9:40 PM, Florian Weimer wrote:
>> * Vlastimil Babka:
>>
>>
>> See below.  I don't have debuginfo for this build, and the binary does
>> not reproduce for some reason.  Due to the heavy inlining, it might be
>> quite hard to figure out what's going on.
> 
> Thanks, but I'm still not able to "decompile" that in my head.

While staring at the code, I think I found two probably unrelated bugs.
One is that pfn and page might be desynced when zone starts in the middle
of pageblock, as the max() is only applied to page and not pfn. But that
only effectively affects the later pfn_valid_within() checks, which should
be always true on x86.

The second is that "end of pageblock online and valid" should refer to
the last pfn of pageblock, not first pfn of next pageblocks. Otherwise we
might return false needlessly. Mel, what do you think?

--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -270,14 +270,15 @@ __reset_isolation_pfn(struct zone *zone, unsigned long pfn, bool check_source,
 
        /* Ensure the start of the pageblock or zone is online and valid */
        block_pfn = pageblock_start_pfn(pfn);
-       block_page = pfn_to_online_page(max(block_pfn, zone->zone_start_pfn));
+       block_pfn = max(block_pfn, zone->zone_start_pfn);
+       block_page = pfn_to_online_page(block_pfn);
        if (block_page) {
                page = block_page;
                pfn = block_pfn;
        }
 
        /* Ensure the end of the pageblock or zone is online and valid */
-       block_pfn += pageblock_nr_pages;
+       block_pfn = pageblock_end_pfn(pfn) - 1;
        block_pfn = min(block_pfn, zone_end_pfn(zone) - 1);
        end_page = pfn_to_online_page(block_pfn);
        if (!end_page)