Re: [PATCH v1 2/2] mm/hugetlb: support write-faults in shared mappings

[Mike Kravetz <mike.kravetz@oracle.com>]

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@oracle.com/

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);
 		}