Message-ID: <Zoug1swoTOqNUPJo@localhost.localdomain>
Date: Mon, 8 Jul 2024 10:18:30 +0200
From: Oscar Salvador <osalvador@...e.de>
To: David Hildenbrand <david@...hat.com>
Cc: Peter Xu <peterx@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Muchun Song <muchun.song@...ux.dev>, SeongJae Park <sj@...nel.org>,
	Miaohe Lin <linmiaohe@...wei.com>, Michal Hocko <mhocko@...e.com>,
	Matthew Wilcox <willy@...radead.org>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Jason Gunthorpe <jgg@...dia.com>
Subject: Re: [PATCH 00/45] hugetlb pagewalk unification

On Thu, Jul 04, 2024 at 05:23:30PM +0200, David Hildenbrand wrote:
> My thinking was if "remove hugetlb_entry" cannot wait for "remove
> page_walk", because we found a reasonable way to do it better and convert
> the individual users. Maybe it can't.
> 
> I've not given up hope that we can end up with something better and clearer
> than the current page_walk API :)

Hi David,

I agree that the current page_walk might be a bit convoluted, and that the
indirect functions approach is a bit of a hassle.
Having said that, let me clarify something.

Although this patchset touches the page_walk API wrt. getting rid of
hugetlb special casing all over the place, my goal was not as focused on
the page_walk as it was on the hugetlb code to gain hability to be
interpreted on PUD/PMD level.

One of the things, among other things, that helped in creating this
mess/duplication we have wrt. hugetlb code vs mm core is that hugetlb
__always__ operates on ptes, which means that we cannot rely on the mm
core to do the right thing, and we need a bunch of hugetlb-pte functions
that knows about their thing, so we lean on that.

IMHO, that was a mistake to start with, but I was not around when it was
introduced and maybe there were good reasons to deal with that the way
it is done.
But, the thing is that my ultimate goal, is for hugetlb code to be able
to deal with PUD/PMD (pte and cont-pte is already dealt with) just like
mm core does for THP (PUD is not supported by THP, but you get me), and
that is not that difficult to do, as this patchset tries to prove.

Of course, for hugetlb to gain the hability to operate on PUD/PMD, this
means we need to add a fairly amount of code. e.g: for operating
hugepages on PUD level, code for markers on PUD/PMD level for
uffd/poison stuff (only dealt
on pmd/pte atm AFAIK), swap functions for PUD (is_swap_pud for PUD), etc.
Basically, almost all we did for PMD-* stuff we need it for PUD as well,
and that will be around when THP gains support for PUD if it ever does
(I guess that in a few years if memory capacity keeps increasing).

E.g: pud_to_swp_entry to detect that a swp entry is hwpoison with
     is_hwpoison_entry

Yes, it is a hassle to have more code around, but IMO, this new code
will help us in 1) move away from __always__ operate on ptes 2) ease
integrate hugetlb code into mm core.

I will keep working on this patchset not because of pagewalk savings,
but because I think it will help us in have hugetlb more mm-core ready,
since the current pagewalk has to test that a hugetlb page can be
properly read on PUD/PMD/PTE level no matter what: uffd for hugetlb on PUD/PMD,
hwpoison entries for swp on PUD/PMD, pud invalidating, etc.

If that gets accomplished, I think that a fair amount of code that lives
in hugetlb.c can be deleted/converted as less special casing will be needed.

I might be wrong and maybe I will hit a brick wall, but hopefully not.



-- 
Oscar Salvador
SUSE Labs

Powered by blists - more mailing lists