Re: [PATCH v2 3/7] mm/hugetlb: Rename avoid_reserve to cow_from_owner

[Oscar Salvador <osalvador@suse.de>]

On Mon, Jan 13, 2025 at 11:19:27AM -0500, Peter Xu wrote:
> Oscar,
> 
> On Mon, Jan 13, 2025 at 12:20:34PM +0100, Oscar Salvador wrote:
> > On Tue, Jan 07, 2025 at 03:39:58PM -0500, Peter Xu wrote:
> > > The old name "avoid_reserve" can be too generic and can be used wrongly in
> > > the new call sites that want to allocate a hugetlb folio.
> > > 
> > > It's confusing on two things: (1) whether one can opt-in to avoid global
> > > reservation, and (2) whether it should take more than one count.
> > > 
> > > In reality, this flag is only used in an extremely hacky path, in an
> > > extremely hacky way in hugetlb CoW path only, and always use with 1 saying
> > > "skip global reservation".  Rename the flag to avoid future abuse of this
> > > flag, making it a boolean so as to reflect its true representation that
> > > it's not a counter.  To make it even harder to abuse, add a comment above
> > > the function to explain it.
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > 
> > I agree that the current name is quite misleading, and this patch
> > improves the situation substantially.
> > The only thing I am missing here is that the comment you added could be
> > more explanatory as to why new call sites do not want to make use of the
> > flag.
> > 
> > IIRC, not using so, will bypass all vma level reservations as you
> 
> s/not using/using/?  Only using the flag (setting to true) will bypass vma,
> but I could have misunderstood this line..

Yes, sorry.

> > mentioned, which means that the child can get killed if the parent
> > makes use of the page, as it is the parent the only one that made a
> > reservation.
> 
> The paragraph I added on top of alloc_hugetlb_folio() is trying to suggest
> nobody should set this to true in any new paths.

Yes, you are right.
I thought we could be more categoric, but curent comment seems fine.

> So far, the reservation path should have nothing relevant to stealing page
> on its own (if that is what you meant above..) - page stealing in hugetlb
> private is done separately within the unmap_ref_private() helper.  Here the
> parent needs to bypass vma reservation because it must have consumed it
> with the folio installed in the pgtable (which is write-protected).  That
> may or may not be relevant to page stealing, e.g. if global pool still has
> free page it doesn't need to affect child from using its hugetlb pages.

No, I kind of misinterpreted this.

Now, let me see if I get this straight:

1) parent maps a hugetlb page, but not yet fault in.
2) forks, child faults-in the page
3) child doesn't have any reservation, when 'cow_from_owner' set to true
   we check whether we have a spare hugetlb page to satisfy that
4) parent faults in the page
5) we do not have spare hugetlb pages, so we 'steal' it from the child
   with unmap_ref_private.


Is my understanding correct?

-- 
Oscar Salvador
SUSE Labs