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@xxxxxxxxxx> > > > > 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
