Re: [PATCH 2/5] mm,hugetlb: Document the reason to lock the folio in the faulting path

[Oscar Salvador <osalvador@suse.de>]

On Mon, Jun 16, 2025 at 11:22:43AM +0200, David Hildenbrand wrote:
 
> hugetlb_fault->hugetlb_no_page->hugetlb_wp
> 
> already *mapped* the pagecache page into the page table.
> 
> See
> 	if (anon_rmap)
> 		hugetlb_add_new_anon_rmap(folio, vma, vmf->address);
> 	else
> 		hugetlb_add_file_rmap(folio);
> 
> So at that point it would be "stable" unless I am missing something?
> 
> So once we are in hugetlb_wp(), that path much rather corresponds to
> do_wp_page()->wp_page_copy.

Yes, that's right.
That's something I've been thinking over the weekend.

E.g: do_cow_fault, first copies the page from the pagecache to a new one
and __then__ maps the that page into the page tables.
While in hugetlb_no_page->hugetlb_wp, the workflow is a bit different.

We first map it and then we copy it if we need to.

What do you mean by stable?
In the generic faulting path, we're not worried about the page going away
because we hold a reference, so I guess the lock must be to keep content stable?

I mean, yes, after we have mapped the page privately into the pagetables,
we don't have business about content-integrity anymore, so given this rule, yes,
I guess hugetlb_wp() wouldn't need the lock (for !anonymous) because we already
have mapped it privately at that point.

But there's something I don't fully understand and makes me feel uneasy.
If the lock in the generic faultin path is to keep content stable till we
have mapped it privately, wouldn't be more correct to also hold it
during the copy in hugetlb_wp, to kinda emulate that?


-- 
Oscar Salvador
SUSE Labs