Re: [PATCH] arm64: hugetlb: Prevent write fault livelocks on stale TLB entries

[Catalin Marinas <catalin.marinas@arm.com>]

On Thu, Nov 12, 2026 at 11:08:02AM +0800, Huang, Ying wrote:
> Chris Down <chris@chrisdown.name> writes:
> 
> > In Meta production, we have had tasks get stuck in a fault loop while
> > using hugetlb on arm64. During my investigations I found that affected
> > tasks share a private hugetlb page.
> >
> > Looking at the stuck task states, I found the following ordering:
> >
> >   CPU0                                 CPU1
> >   ----                                 ----
> >   read page
> >   [read only entry cached in TLB]
> >                                        write page
> >                                        hugetlb_fault()
> >                                          hugetlb_wp()
> >                                          [reuse folio]
> >                                          set_huge_ptep_maybe_writable()
> >                                            set_huge_ptep_writable()
> >                                          [flush local TLB only]
> >
> >   write page using stale entry
> >   hugetlb_fault()
> >     huge_ptep_set_access_flags()
> >     [entry already writable, no flush]
> >   retry write using stale entry
> >   [fault indefinitely. well, that's not ideal!]
> >
> > The reason is that when a write fault makes a non-contiguous hugetlb
> > entry writable, it only invalidates the local TLB. However, another CPU
> > can keep the old read-only entry, so its write will fault even though
> > the page table already permits it. hugetlb_fault() then finds nothing to
> > change and leaves the stale entry in place, so the same write keeps
> > going round in circles.
> 
> Good catch!  Thanks!
> 
> > This is particularly bad because, if the kernel is copying to user
> > memory, the retry stays inside the kernel and the task can't act on the
> > pending signal.
> >
> > It can even remain unkillable while holding locks across the copy,
> > resulting in further failures. For example, a read into a hugetlb
> > buffer can hold the pipe mutex and block other users of the pipe until
> > the loop ends, causing cascading lockups across the machine.
> >
> > To fix this, when a write fault finds nothing to change in a
> > non-contiguous hugetlb entry, let's invalidate the local entry.
> >
> > Fixes: cb1fa2e99955 ("arm64, tlbflush: don't TLBI broadcast if page reused in write fault")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Chris Down <chris@chrisdown.name>
> > ---
> >  arch/arm64/mm/hugetlbpage.c | 21 ++++++++++++++++++---
> >  1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> > index 8e799c1fe0aa..ff1f53e435e1 100644
> > --- a/arch/arm64/mm/hugetlbpage.c
> > +++ b/arch/arm64/mm/hugetlbpage.c
> > @@ -429,9 +429,24 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,
> >  	VM_WARN_ON(!pte_present(pte));
> >  	ncontig = num_contig_ptes(huge_page_size(hstate_vma(vma)), &pgsize);
> >  
> > -	if (!pte_cont(pte))
> > -		return __ptep_set_access_flags_anysz(vma, addr, ptep, pte,
> > -						     dirty, pgsize);
> > +	if (!pte_cont(pte)) {
> > +		if (__ptep_set_access_flags_anysz(vma, addr, ptep, pte,
> > +						  dirty, pgsize))
> > +			return 1;
> > +
> > +		/*
> > +		 * Another CPU may have made the entry writable without
> > +		 * invalidating our stale read-only TLB entry. To avoid
> > +		 * indefinitely faulting on this entry, flush it.
> > +		 */
> > +		if (dirty)
> > +			__flush_hugetlb_tlb_range(vma, addr, addr + pgsize,
> > +						  pgsize,
> > +						  TLBF_NOWALKCACHE |
> > +						  TLBF_NOBROADCAST |
> > +						  TLBF_NONOTIFY);
> 
> IIUC, this is similar to fix_spurious_fault() for non-hugetlb folios.
> Can we use similar logic?

I think we should but that's something going forward, not as a
backportable fix. I'll take this patch for now and we can look into
wiring up something similar to fix_spurious_fault() for hugetlb.

Currently fix_spurious_fault() only handles pte/pmd faults, so we can't
simply do this:

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index da5d04edf7c1..e0e582f264e8 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6297,6 +6297,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 	if (huge_ptep_set_access_flags(vma, vmf.address, vmf.pte, vmf.orig_pte,
 						flags & FAULT_FLAG_WRITE))
 		update_mmu_cache(vma, vmf.address, vmf.pte);
+	else
+		fix_spurious_fault(vmf, PGTABLE_LEVEL_PTE);
 out_ptl:
 	spin_unlock(vmf.ptl);
 out_mutex:

because the pgtable level is not known to the core code. We'd need some
arch specific hooks and handle it in hugetlb.

With Chris's fix we just miss one more retry before invalidation. But
yes, having similar mechanism in the core code makes sense to me.

-- 
Catalin