Re: [PATCH v3 03/19] powerpc/mm: Fix wrong addr_pfn tracking in compound vmemmap population

From: Muchun Song

Date: Wed Jun 03 2026 - 22:10:16 EST




> On Jun 3, 2026, at 22:36, Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx> wrote:
>
> Muchun Song <songmuchun@xxxxxxxxxxxxx> 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 <songmuchun@xxxxxxxxxxxxx>
>> Acked-by: Oscar Salvador <osalvador@xxxxxxx>
>
> 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: stable@xxxxxxxxxx

Yes. I'll add it next version.

>
> Also, feel free to add:
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>

Thanks.