Messages in this thread | | | Date | Mon, 27 May 2024 15:17:01 +0200 | Subject | Re: [PATCH] mm/hugetlb: Move vmf_anon_prepare upfront in hugetlb_wp | From | David Hildenbrand <> |
| |
Am 27.05.24 um 10:53 schrieb Oscar Salvador: > On Tue, May 21, 2024 at 09:34:46AM +0200, Oscar Salvador wrote: >> I did not hit this bug, I just spotted this because I was looking at hugetlb_wp >> for some other reason. And I did not want to get creative to see if I could >> trigger this so I could get a backtrace. >> My assumption is that we could trigger this if 1) this was a shared mapping, >> so no anon_vma and 2) we call in GUP code with FOLL_WRITE, which would cause >> the FLAG_UNSHARE to be passed, so we will end up in hugetlb_wp(). > > So I checked this again and I have to confess I am bit confused. > > hugetlb_wp() can be called from either hugetlb_fault() or hugetlb_no_page(). > > hugetlb_fault()->hugetlb_wp() upon FAULT_FLAG_{WRITE,UNSHARE} > hugetlb_no_page->hugetlb_wp()-> upon FAULT_FLAG_WRITE && !VM_SHARED > > hugetlb_no_page()->vmf_anon_prepare() upon !VM_SHARED, which means that VM_SHARED > mappings do not have vma->anon_vma, while others do. > > hugetlb_wp() will call set_huge_ptep_writable() right away and return if it sees > that the mapping is shared. > So the only other we have to end up in hugetlb_wp() is via FAULT_FLAG_UNSHARE. > For that to happen gup_must_unshare() must return true, which means the following > assumptions must hold. > > - For Anonymous pages: > 1) !PageAnonExclusive > - For Filebacked pages: > 2) We do not have a vma > 3) It is a COW mapping > > 1) If gup_must_unshare() returns true for Anonymous pages because the page is not > exclusive and must be unshared, hugetlb_wp() will already see the > vma->anon_prepare being initialized because of the previous > hugetlb_no_page()->vmf_anon_prepare. > > 2) I do not quite understand this case.
gup_must_unshare() without a VMA is only used for GUP-fast. Before triggering a page fault we always fallback to GUP-slow first, where we have a VMA.
IMHO we better not make assumptions that hugetlb_wp() will always already have an anon VMA.
-- Thanks,
David / dhildenb
| |