linux-kernel - Re: [PATCH v6 01/11] asm-generic/pgtable: Adds generic functions to track lockless pgtable walks

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:   Thu, 06 Feb 2020 23:19:45 -0300
From:   Leonardo Bras <leonardo@...ux.ibm.com>
To:     Christophe Leroy <christophe.leroy@....fr>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Arnd Bergmann <arnd@...db.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>,
        Nicholas Piggin <npiggin@...il.com>,
        Steven Price <steven.price@....com>,
        Robin Murphy <robin.murphy@....com>,
        Mahesh Salgaonkar <mahesh@...ux.vnet.ibm.com>,
        Balbir Singh <bsingharora@...il.com>,
        Reza Arbab <arbab@...ux.ibm.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Allison Randal <allison@...utok.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Mike Rapoport <rppt@...ux.ibm.com>,
        Michal Suchanek <msuchanek@...e.de>
Cc:     linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        kvm-ppc@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCH v6 01/11] asm-generic/pgtable: Adds generic functions to
 track lockless pgtable walks

Hello Christophe, thanks for the feedback!

On Thu, 2020-02-06 at 06:54 +0100, Christophe Leroy wrote:
> > A memory barrier was also added just to make sure there is no speculative
> > read outside the interrupt disabled area. Other than that, it is not
> > supposed to have any change of behavior from current code.
> 
> Is that speculative barrier necessary for all architectures ? Does it 
> impact performance ? Shouldn't this be another patch ?

It makes sense, better keep the code as much as possible as it was. If
any arch finds this barrier needed, it can implement it's own version
of this function (or another patch to add this to generic, if proved to
be needed in every arch).

> > +#ifndef __HAVE_ARCH_LOCKLESS_PGTBL_WALK_CONTROL
> > +/*
> > + * begin_lockless_pgtbl_walk: Must be inserted before a function call that does
> > + *   lockless pagetable walks, such as __find_linux_pte()
> > + */
> > +static inline
> > +unsigned long begin_lockless_pgtbl_walk(void)
> 
> What about keeping the same syntax as local_irq_save(), something like:
> 
> #define begin_lockless_pgtbl_walk(flags) \
> do {
> 	local_irq_save(flags);
> 	smp_mb();
> } while (0)
> 

Makes sense. But wouldn't inlining have the same code output? 

Best regards,
Leonardo Bras


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)