Re: Bug: Performance regression in 1013af4f585f: mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race

[David Hildenbrand <david@redhat.com>]

On 01.09.25 12:58, Jann Horn wrote:
> Hi!
> 
> On Fri, Aug 29, 2025 at 4:30 PM Uschakow, Stanislav <suschako@amazon.de> wrote:
>> We have observed a huge latency increase using `fork()` after ingesting the CVE-2025-38085 fix which leads to the commit `1013af4f585f: mm/hugetlb: fix huge_pmd_unshare() vs GUP-fast race`. On large machines with 1.5TB of memory with 196 cores, we identified mmapping of 1.2TB of shared memory and forking itself dozens or hundreds of times we see a increase of execution times of a factor of 4. The reproducer is at the end of the email.
> 
> Yeah, every 1G virtual address range you unshare on unmap will do an
> extra synchronous IPI broadcast to all CPU cores, so it's not very
> surprising that doing this would be a bit slow on a machine with 196
> cores.
> 
>> My observation/assumption is:
>>
>> each child touches 100 random pages and despawns
>> on each despawn `huge_pmd_unshare()` is called
>> each call to `huge_pmd_unshare()` syncrhonizes all threads using `tlb_remove_table_sync_one()` leading to the regression
> 
> Yeah, makes sense that that'd be slow.
> 
> There are probably several ways this could be optimized - like maybe
> changing tlb_remove_table_sync_one() to rely on the MM's cpumask
> (though that would require thinking about whether this interacts with
> remote MM access somehow), or batching the refcount drops for hugetlb
> shared page tables through something like struct mmu_gather, or doing
> something special for the unmap path, or changing the semantics of
> hugetlb page tables such that they can never turn into normal page
> tables again. However, I'm not planning to work on optimizing this.

I'm currently looking at the fix and what sticks out is "Fix it with an 
explicit broadcast IPI through tlb_remove_table_sync_one()".

(I don't understand how the page table can be used for "normal, 
non-hugetlb". I could only see how it is used for the remaining user for 
hugetlb stuff, but that's different question)

How does the fix work when an architecture does not issue IPIs for TLB 
shootdown? To handle gup-fast on these architectures, we use RCU.

So I'm wondering whether we use RCU somehow.

But note that in gup_fast_pte_range(), we are validating whether the PMD 
changed:

if (unlikely(pmd_val(pmd) != pmd_val(*pmdp)) ||
     unlikely(pte_val(pte) != pte_val(ptep_get(ptep)))) {
	gup_put_folio(folio, 1, flags);
	goto pte_unmap;
}


So in case the page table got reused in the meantime, we should just 
back off and be fine, right?

-- 
Cheers

David / dhildenb