Re: [PATCH 12/13] mm/treewide: Remove pXd_huge()

From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: "peterx@redhat.com" <peterx@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Matthew Wilcox <willy@infradead.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"x86@kernel.org" <x86@kernel.org>,
	Mike Rapoport <rppt@kernel.org>,
	Muchun Song <muchun.song@linux.dev>,
	"sparclinux@vger.kernel.org" <sparclinux@vger.kernel.org>,
	Jason Gunthorpe <jgg@nvidia.com>
Subject: Re: [PATCH 12/13] mm/treewide: Remove pXd_huge()
Date: Thu, 14 Mar 2024 08:56:59 +0000	[thread overview]
Message-ID: <7e93ab99-c956-42d0-9afd-3c856f3ad951@csgroup.eu> (raw)
In-Reply-To: <20240313214719.253873-13-peterx@redhat.com>

Le 13/03/2024 à 22:47, peterx@redhat.com a écrit :
> From: Peter Xu <peterx@redhat.com>
> 
> This API is not used anymore, drop it for the whole tree.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   arch/arm/mm/Makefile                          |  1 -
>   arch/arm/mm/hugetlbpage.c                     | 29 -------------------
>   arch/arm64/mm/hugetlbpage.c                   | 10 -------
>   arch/loongarch/mm/hugetlbpage.c               | 10 -------
>   arch/mips/include/asm/pgtable-32.h            |  2 +-
>   arch/mips/include/asm/pgtable-64.h            |  2 +-
>   arch/mips/mm/hugetlbpage.c                    | 10 -------
>   arch/parisc/mm/hugetlbpage.c                  | 11 -------
>   .../include/asm/book3s/64/pgtable-4k.h        | 10 -------
>   .../include/asm/book3s/64/pgtable-64k.h       | 25 ----------------
>   arch/powerpc/include/asm/nohash/pgtable.h     | 10 -------
>   arch/riscv/mm/hugetlbpage.c                   | 10 -------
>   arch/s390/mm/hugetlbpage.c                    | 10 -------
>   arch/sh/mm/hugetlbpage.c                      | 10 -------
>   arch/sparc/mm/hugetlbpage.c                   | 10 -------
>   arch/x86/mm/hugetlbpage.c                     | 16 ----------
>   include/linux/hugetlb.h                       | 24 ---------------
>   17 files changed, 2 insertions(+), 198 deletions(-)
>   delete mode 100644 arch/arm/mm/hugetlbpage.c
> 

> diff --git a/arch/mips/include/asm/pgtable-32.h b/arch/mips/include/asm/pgtable-32.h
> index 0e196650f4f4..92b7591aac2a 100644
> --- a/arch/mips/include/asm/pgtable-32.h
> +++ b/arch/mips/include/asm/pgtable-32.h
> @@ -129,7 +129,7 @@ static inline int pmd_none(pmd_t pmd)
>   static inline int pmd_bad(pmd_t pmd)
>   {
>   #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
> -	/* pmd_huge(pmd) but inline */
> +	/* pmd_leaf(pmd) but inline */

Shouldn't this comment have been changed in patch 11 ?

>   	if (unlikely(pmd_val(pmd) & _PAGE_HUGE))

Unlike pmd_huge() which is an outline function, pmd_leaf() is a macro so 
it could be used here instead of open coping.

>   		return 0;
>   #endif
> diff --git a/arch/mips/include/asm/pgtable-64.h b/arch/mips/include/asm/pgtable-64.h
> index 20ca48c1b606..7c28510b3768 100644
> --- a/arch/mips/include/asm/pgtable-64.h
> +++ b/arch/mips/include/asm/pgtable-64.h
> @@ -245,7 +245,7 @@ static inline int pmd_none(pmd_t pmd)
>   static inline int pmd_bad(pmd_t pmd)
>   {
>   #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
> -	/* pmd_huge(pmd) but inline */
> +	/* pmd_leaf(pmd) but inline */

Same

>   	if (unlikely(pmd_val(pmd) & _PAGE_HUGE))

Same

>   		return 0;
>   #endif

> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable-64k.h b/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
> index 2fce3498b000..579a7153857f 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
> @@ -4,31 +4,6 @@
>   
>   #ifndef __ASSEMBLY__
>   #ifdef CONFIG_HUGETLB_PAGE
> -/*
> - * We have PGD_INDEX_SIZ = 12 and PTE_INDEX_SIZE = 8, so that we can have
> - * 16GB hugepage pte in PGD and 16MB hugepage pte at PMD;
> - *
> - * Defined in such a way that we can optimize away code block at build time
> - * if CONFIG_HUGETLB_PAGE=n.
> - *
> - * returns true for pmd migration entries, THP, devmap, hugetlb
> - * But compile time dependent on CONFIG_HUGETLB_PAGE
> - */

Should we keep this comment somewhere for documentation ?

> -static inline int pmd_huge(pmd_t pmd)
> -{
> -	/*
> -	 * leaf pte for huge page
> -	 */
> -	return !!(pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE));
> -}
> -
> -static inline int pud_huge(pud_t pud)
> -{
> -	/*
> -	 * leaf pte for huge page
> -	 */
> -	return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PTE));
> -}
>   
>   /*
>    * With 64k page size, we have hugepage ptes in the pgd and pmd entries. We don't