On Wed, Mar 06, 2024 at 06:41:38PM +0800, pet...@redhat.com wrote:
> From: Peter Xu <pet...@redhat.com>
>
> This patch partly reverts below commits:
>
> 3a194f3f8ad0 ("mm/hugetlb: make pud_huge() and follow_huge_pud() aware of
> non-present pud entry")
> cbef8478bee5 ("mm/hugetlb: pmd_huge() returns true for non-present hugepage")
>
> Right now, pXd_huge() definition across kernel is unclear. We have two
> groups that think differently on swap entries:
>
> - x86/sparc: Allow pXd_huge() to accept swap entries
> - all the rest: Doesn't allow pXd_huge() to accept swap entries
>
> This is so confusing. Since the sparc helpers seem to be added in 2016,
> which is after x86's (2015), so sparc could have followed a trend. x86
> proposed such swap handling in 2015 to resolve hugetlb swap entries hit in
> GUP, but now GUP guards swap entries with !pXd_present() in all layers so
> we should be safe.
>
> We should define this API properly, one way or another, rather than keep
> them defined differently across archs.
>
> Gut feeling tells me that pXd_huge() shouldn't include swap entries, and it
> turns out that I am not the only one thinking so, the question was raised
> when the current pmd_huge() for x86 was proposed by Ville Syrjälä:
>
> https://lore.kernel.org/all/y2wq7i4lxh8iu...@intel.com/
>
> I might also be missing something obvious, but why is it even necessary
> to treat PRESENT==0+PSE==0 as a huge entry?
>
> It is also questioned when Jason Gunthorpe reviewed the other patchset on
> swap entry handlings:
>
> https://lore.kernel.org/all/20240221125753.gq13...@nvidia.com/
>
> Revert its meaning back to original. It shouldn't have any functional
> change as we should be ready with guards on !pXd_present() explicitly
> everywhere.
>
> Note that I also dropped the "#if CONFIG_PGTABLE_LEVELS > 2", it was there
> probably because it was breaking things when 3a194f3f8ad0 was proposed,
> according to the report here:
>
> https://lore.kernel.org/all/y2lyxitkqyajt...@intel.com/
>
> Now we shouldn't need that.
>
> Instead of reverting to _PAGE_PSE raw check, leverage pXd_leaf().
>
> Cc: Naoya Horiguchi <naoya.horigu...@nec.com>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: Ingo Molnar <mi...@redhat.com>
> Cc: Borislav Petkov <b...@alien8.de>
> Cc: Dave Hansen <dave.han...@linux.intel.com>
> Cc: x...@kernel.org
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> arch/x86/mm/hugetlbpage.c | 18 ++++--------------
> 1 file changed, 4 insertions(+), 14 deletions(-)
I think this is the right thing to do, callers should be more directly
sensitive to swap entries not back into it indirectly from a helper
like this.
Jason
