| 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
| ||
|
Message-ID: <ZfME8vjFhl8AdyFe@x1n>
Date: Thu, 14 Mar 2024 10:08:50 -0400
From: Peter Xu <peterx@...hat.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
Matthew Wilcox <willy@...radead.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
Andrew Morton <akpm@...ux-foundation.org>,
"x86@...nel.org" <x86@...nel.org>, Mike Rapoport <rppt@...nel.org>,
Muchun Song <muchun.song@...ux.dev>,
"sparclinux@...r.kernel.org" <sparclinux@...r.kernel.org>,
Jason Gunthorpe <jgg@...dia.com>
Subject: Re: [PATCH 12/13] mm/treewide: Remove pXd_huge()
On Thu, Mar 14, 2024 at 08:56:59AM +0000, Christophe Leroy wrote:
>
>
> Le 13/03/2024 à 22:47, peterx@...hat.com a écrit :
> > From: Peter Xu <peterx@...hat.com>
> >
> > This API is not used anymore, drop it for the whole tree.
> >
> > Signed-off-by: Peter Xu <peterx@...hat.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 ?
IMHO it's fine to be here, as this is the patch to finally drop _huge().
Patch 11 only converts the callers to use _leaf()s. So this comment is
still valid until this patch, because this patch removes that definition.
>
> > 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.
I worry it will break things as pmd_leaf() can sometimes be defined after
arch *pgtable.h headers. So I avoided touching it except what I think I'm
confident. I had a feeling it's inlined just because of a similar reason
for the old _huge().
>
> > 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 ?
The 2nd/3rd paragraphs are definitely obsolete, so should be dropped.
OTOH, I'm not sure how much that will help if e.g. I move that over to
pmd_leaf(): a check over cpu_to_be64(_PAGE_PTE) is an implementation as
simple as it could be to explain itself with even no comment to me..
I also don't fully digest why that 1st paragraph discusses PGD entries: for
example, there's no pgd_huge() defined. It may not mean that the comment
is wrong, perhaps it means that I may lack some knowledge around this area
on Power..
Would you suggest how I should move paragraph 1 (and help to explain what
it is describing)? Or maybe we can provide a separate patch for Power's
huge page sizes but posted separately (and very possibly I'm not the best
candidate then..).
>
> > -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
--
Peter Xu
Powered by blists - more mailing lists