Re: [PATCH v1 06/11] mm: support GUP-triggered unsharing via FAULT_FLAG_UNSHARE (!hugetlb)

[Linus Torvalds]

On Fri, Dec 17, 2021 at 6:17 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> I think the hugepage code should use the exact same logic that the
> regular wp fault code does.

IOW, I think that this stupid (AND UNTESTED) patch should likely just
fix David's test-case with the hugepage and splice thing..

Or at least be somewhat close.  But it should be paired with the GUP
side doing the right thing too, of course. Maybe it already does,
maybe it doesn't, I didn't check...

And maybe there's something subtle about the page_count() of a THP
entry. Again, I can't really claim to have tested this all, but I'm
hoping this makes somebody go "Ahh, now I see what Linus means"

            Linus
 mm/huge_memory.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index e5483347291c..9f52389eb031 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1304,25 +1304,11 @@ vm_fault_t do_huge_pmd_wp_page(struct vm_fault *vmf)
 	VM_BUG_ON_PAGE(!PageHead(page), page);
 
 	/* Lock page for reuse_swap_page() */
-	if (!trylock_page(page)) {
-		get_page(page);
-		spin_unlock(vmf->ptl);
-		lock_page(page);
-		spin_lock(vmf->ptl);
-		if (unlikely(!pmd_same(*vmf->pmd, orig_pmd))) {
-			spin_unlock(vmf->ptl);
-			unlock_page(page);
-			put_page(page);
-			return 0;
-		}
-		put_page(page);
-	}
+	if (!trylock_page(page))
+		goto failed_to_lock;
 
-	/*
-	 * We can only reuse the page if nobody else maps the huge page or it's
-	 * part.
-	 */
-	if (reuse_swap_page(page, NULL)) {
+	/* Reuse the page as-is if this pmd entry is the only user */
+	if (page_count(page) == 1) {
 		pmd_t entry;
 		entry = pmd_mkyoung(orig_pmd);
 		entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
@@ -1334,6 +1320,7 @@ vm_fault_t do_huge_pmd_wp_page(struct vm_fault *vmf)
 	}
 
 	unlock_page(page);
+failed_to_lock:
 	spin_unlock(vmf->ptl);
 fallback:
 	__split_huge_pmd(vma, vmf->pmd, vmf->address, false, NULL);