Re: [RFC] mm: Is flush_cache_page() in wp_page_reuse() really necessary?

["wuyifeng (C)" <wuyifeng10@huawei.com>]

Thanks for your reply and the pointers. I went through the kernel arch/ code
and looked into architectures that support PMD-level THP, such as arm64 and
x86_64 etc. I found that their flush_cache_page() implementations are all nop.

More precisely, for modern cache architectures (VIPT no-alias, PIPT),
flush_cache_page() is nop.

So the call in wp_page_reuse() looks more like a historical artifact, and it’s
hard to judge whether it was strictly necessary.

I really appreciate your clarifications!

Best regards,
Wuyifeng

在 2025/9/29 16:09, David Hildenbrand 写道:
> On 29.09.25 09:29, wuyifeng (C) wrote:
>> Hi Linux MM developers,
> 
> Hi,
> 
>>
>> I am reviewing the page fault handling code in the memory management
>> subsystem and came across a point that puzzled me regarding the use of
>> flush_cache_page().
>>
>> When I referred to Documentation/core-api/cachetlb.rst, it describes a
>> typical sequence in a page fault scenario:
>>
>> flush_cache_page(vma, addr, pfn);
>> set_pte(pte_pointer, new_pte_val);
>> flush_tlb_page(vma, addr);
>>
> 
> The number of archs that actually implement flush_cache_page() is limited, so likely there is not a lot of interest around optimizing some out.
> 
>>
>> That is, first the CPU cache is flushed, then the PTE is updated, and
>> finally the TLB is flushed for the corresponding virtual address. This
>> makes sense: when the virtual-to-physical mapping changes, flushing the
>> CPU cache is necessary to prevent stale or corrupted cache from affecting
>> the new mapping.
>>
>> However, in wp_page_reuse(), the virtual-to-physical mapping does
>> not change. The function looks like this:
>>
>> static inline void wp_page_reuse(struct vm_fault *vmf)
>> {
>>      struct vm_area_struct *vma = vmf->vma;
>>      struct page *page = vmf->page;
>>      pte_t entry;
>>
>>      if (page)
>>          page_cpupid_xchg_last(page, (1 << LAST_CPUPID_SHIFT) - 1);
>>
>>      flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
>>      entry = pte_mkyoung(vmf->orig_pte);
>>      entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>>      if (ptep_set_access_flags(vma, vmf->address, vmf->pte, entry, 1))
>>          update_mmu_cache(vma, vmf->address, vmf->pte);
>> }
> 
> Note that an "old" PTE might, depending on the arch, not actually have the hw-present bit set, so next access would trigger a page fault to maintain the clean/old PTE bit in software.
> 
> So it could easily be that we are transitioning from non-hw-present to hw-present here.
> 
> If that requires a flush_cache_page(), I really don't know :)
> 
>>
>>
>> Since the mapping itself does not change, I am puzzled whether this
>> flush_cache_page() call is actually necessary.
> 
> Same here. I have no idea if the scenario described above would require it, or only when we are changing something present to something non-present or differently-present.
> 
>>
>> A similar situation appears in do_huge_pmd_wp_page(), which handles
>> huge pages. When the system reaches the reuse branch after the relevant
>> checks, the code looks like this:
>>
>> reuse:
>>     entry = pmd_mkyoung(orig_pmd);
>>     entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
>>     if (pmdp_set_access_flags(vma, haddr, vmf->pmd, entry, 1))
>>         update_mmu_cache_pmd(vma, vmf->address, vmf->pmd);
>>
>>
>> Here, flush_cache_page() is not called. Instead, the code relies on
>> updating PMD access flags and calling update_mmu_cache_pmd() to refresh
>> the TLB.
> 
> Does any architecture that cares about flush_cache_page() actually support PMD THPs?
> 
> I mean, looking at mm/khugepaged.c, I cannot spot any flush_cache_*(). Maybe it's implicit, but my best guess is that no architecture that supports THPs really requires it?
>