On 08/05/22 14:12, Peter Xu wrote: > On Fri, Aug 05, 2022 at 01:03:29PM +0200, David Hildenbrand wrote: > > Let's add a safety net if we ever get (again) a write-fault on a R/O-mapped > > page in a shared mapping, in which case we simply have to map the > > page writable. > > > > + if (WARN_ON_ONCE(!(vma->vm_flags & VM_WRITE))) > > + return VM_FAULT_SIGSEGV; > > I had a feeling that you just want to double check we have write > permission, but IIUC this should be checked far earlier or we'll have > problem. Back when I was exploring hugetlb softdirty support, this patch handled this condition by not calling into hugetlb_wp (was hugetlb_cow). https://lore.kernel.org/linux-mm/20210211000322.159437-3-mike.kravetz@xxxxxxxxxx/ Here is a quickly updated version that was only tested with David's program. -- Mike Kravetz diff --git a/mm/hugetlb.c b/mm/hugetlb.c index a23f1cffaf49..86d38d41fddf 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -5758,9 +5758,10 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, * spinlock. For private mappings, we also lookup the pagecache * page now as it is used to determine if a reservation has been * consumed. + * Only private/non-shared mappings are sent to hugetlb_wp. */ if ((flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) && - !huge_pte_write(entry)) { + !huge_pte_write(entry) && !(vma->vm_flags & VM_MAYSHARE)) { if (vma_needs_reservation(h, vma, haddr) < 0) { ret = VM_FAULT_OOM; goto out_mutex; @@ -5768,8 +5769,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, /* Just decrements count, does not deallocate */ vma_end_reservation(h, vma, haddr); - if (!(vma->vm_flags & VM_MAYSHARE)) - pagecache_page = hugetlbfs_pagecache_page(h, + pagecache_page = hugetlbfs_pagecache_page(h, vma, haddr); } @@ -5815,9 +5815,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) { if (!huge_pte_write(entry)) { - ret = hugetlb_wp(mm, vma, address, ptep, flags, + if (!(vma->vm_flags & VM_MAYSHARE)) { + ret = hugetlb_wp(mm, vma, address, ptep, flags, pagecache_page, ptl); - goto out_put_page; + goto out_put_page; + } + + entry = huge_pte_mkwrite(entry); + entry = huge_pte_mkdirty(entry); } else if (likely(flags & FAULT_FLAG_WRITE)) { entry = huge_pte_mkdirty(entry); }
