Muchun Song <[email protected]> writes: > vmemmap_populate_compound_pages() uses addr_pfn to determine the PFN > offset within a compound page and to decide whether the current > vmemmap slot should be populated as a head page mapping or should reuse > a tail page mapping. > > However, addr_pfn is advanced manually in parallel with addr. The loop > itself progresses in vmemmap address space, so each PAGE_SIZE step in > addr covers PAGE_SIZE / sizeof(struct page) struct page slots. Since > addr_pfn is compared against nr_pages in data-PFN units, it should > advance by the same number of PFNs. The existing manual increments do > not match that and therefore do not reliably track the PFN > corresponding to the current addr. > > As a result, pfn_offset can be computed from the wrong PFN and the code > can make the head/tail decision for the wrong compound-page position. > > Fix this by deriving addr_pfn directly from the current vmemmap address > instead of carrying it as loop state. > > Fixes: f2b79c0d7968 ("powerpc/book3s64/radix: add support for vmemmap > optimization for radix") > Signed-off-by: Muchun Song <[email protected]> > Acked-by: Oscar Salvador <[email protected]>
Thanks for fixing it. I guess this was not caught because section size on powerpc is 16MB and with 64K pagesize we have 256 pfns to map. The vmemmap size required for this is 256*sizeof(struct page) = 16KB which is < 64K (pagesize). So basically we never loop in vmemmap_populate_compound_page(), because next = addr+PAGE_SIZE will be > end after the 1st iteration itself. But I agree this is a bug which needs fixing and it can be easily caught with 4K pagesize, where we have 4096 pfns to map within a 16MB section. The change looks good to me. Can we please add stable tag too? Cc: [email protected] Also, feel free to add: Reviewed-by: Ritesh Harjani (IBM) <[email protected]>
