On 25.07.24 22:41, Andrew Morton wrote:
On Thu, 25 Jul 2024 20:39:53 +0200 David Hildenbrand <david@xxxxxxxxxx> wrote:
Working on another generic page table walker that tries to avoid
special-casing hugetlb, I found a page table locking issue with hugetlb
folios that are not mapped using a single PMD/PUD.
For some hugetlb folio sizes, GUP will take different page table locks
when walking the page tables than hugetlb when modifying the page tables.
I did not actually try reproducing an issue, but looking at
follow_pmd_mask() where we might be rereading a PMD value multiple times
it's rather clear that concurrent modifications are rather unpleasant.
In follow_page_pte() we might be better in that regard -- ptep_get() does
a READ_ONCE() -- but who knows what else could happen concurrently in
some weird corner cases (e.g., hugetlb folio getting unmapped and freed).
Did some basic sanity testing with various hugetlb sizes on x86-64 and
arm64. Maybe I'll find some time to actually write a simple reproducer in
the common weeks, so this wouldn't have to be all-theoretical for now.
When can we be confident that this change is merge-worthy?
I'm convinced that it is the right thing to do, but I don't think we
have to rush this.
As Baolin notes, we fixed the same issue in the past, unfortunately also
without a reproducer IIUC, so I'll try to reproduce the race, but I'm
not 100% sure if I'll manage to do so..
So it's certainly merge-worthy after it had a bit of exposure to -next,
but no need to rush this upstream.
Thanks!
--
Cheers,
David / dhildenb
