linux-kernel - Re: [PATCH v2 3/7] mm/hugetlb: Rename avoid_reserve to cow_from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4U9Dy-2hnHpCBog@x1n>
Date: Mon, 13 Jan 2025 11:19:27 -0500
From: Peter Xu <peterx@...hat.com>
To: Oscar Salvador <osalvador@...e.de>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Breno Leitao <leitao@...ian.org>, Rik van Riel <riel@...riel.com>,
	Muchun Song <muchun.song@...ux.dev>,
	Naoya Horiguchi <nao.horiguchi@...il.com>,
	Roman Gushchin <roman.gushchin@...ux.dev>,
	Ackerley Tng <ackerleytng@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 3/7] mm/hugetlb: Rename avoid_reserve to cow_from_owner

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@...hat.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..

> 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.

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.

However again I could have totally misunderstood your comment..

> 
> So maybe dropping a hint would be nice.
> 
> Reviewed-by: Oscar Salvador <osalvador@...e.de>

Thanks for taking a look!

-- 
Peter Xu