linux-kernel - Re: [PATCH 3/3] mm/hugetlb: Introduce HAVE_ARCH_CLEAR_HUGEPAGE

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]

Date:   Sat, 25 Apr 2020 20:01:24 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Anshuman Khandual <anshuman.khandual@....com>
Cc:     linux-mm@...ck.org, Russell King <linux@...linux.org.uk>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Tony Luck <tony.luck@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        "James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
        Helge Deller <deller@....de>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Rich Felker <dalias@...c.org>,
        "David S. Miller" <davem@...emloft.net>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Mike Kravetz <mike.kravetz@...cle.com>, x86@...nel.org,
        linux-arm-kernel@...ts.infradead.org, linux-ia64@...r.kernel.org,
        linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, linux-riscv@...ts.infradead.org,
        linux-s390@...r.kernel.org, linux-sh@...r.kernel.org,
        sparclinux@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] mm/hugetlb: Introduce
 HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS

On Sun, 26 Apr 2020 08:13:17 +0530 Anshuman Khandual <anshuman.khandual@....com> wrote:

> 
> 
> On 04/26/2020 06:25 AM, Andrew Morton wrote:
> > On Tue, 14 Apr 2020 17:14:30 +0530 Anshuman Khandual <anshuman.khandual@....com> wrote:
> > 
> >> There are multiple similar definitions for arch_clear_hugepage_flags() on
> >> various platforms. This introduces HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS for those
> >> platforms that need to define their own arch_clear_hugepage_flags() while
> >> also providing a generic fallback definition for others to use. This help
> >> reduce code duplication.
> >>
> >> ...
> >>
> >> --- a/include/linux/hugetlb.h
> >> +++ b/include/linux/hugetlb.h
> >> @@ -544,6 +544,10 @@ static inline int is_hugepage_only_range(struct mm_struct *mm,
> >>  }
> >>  #endif
> >>  
> >> +#ifndef HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS
> >> +static inline void arch_clear_hugepage_flags(struct page *page) { }
> >> +#endif
> >> +
> >>  #ifndef arch_make_huge_pte
> >>  static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
> >>  				       struct page *page, int writable)
> > 
> > This is the rather old-school way of doing it.  The Linus-suggested way is
> > 
> > #ifndef arch_clear_hugepage_flags
> > static inline void arch_clear_hugepage_flags(struct page *page)
> > {
> > }
> > #define arch_clear_hugepage_flags arch_clear_hugepage_flags
> 
> Do we need that above line here ? Is not that implicit.

It depends if other header files want to test whether
arch_clear_hugepage_flags is already defined.  If the header heorarchy
is well-defined and working properly, they shouldn't need to, because
we're reliably indluding the relevant arch header before (or early
within) include/linux/hugetlb.h.

It would be nice if

#define arch_clear_hugepage_flags arch_clear_hugepage_flags
#define arch_clear_hugepage_flags arch_clear_hugepage_flags

were to generate an compiler error but it doesn't.  If it did we could
detect these incorrect inclusion orders.

> > #endif
> > 
> > And the various arch headers do
> > 
> > static inline void arch_clear_hugepage_flags(struct page *page)
> > {
> > 	<some implementation>
> > }
> > #define arch_clear_hugepage_flags arch_clear_hugepage_flags
> > 
> > It's a small difference - mainly to avoid adding two variables to the
> > overall namespace where one would do.
> 
> Understood, will change and resend.

That's OK - I've queued up that fix.