Date:   Sat, 13 Oct 2018 10:50:39 -0700
From:   Joel Fernandes <joel@...lfernandes.org>
To:     dancol@...gle.com
Cc:     David Miller <davem@...emloft.net>, kirill@...temov.name,
        linux-kernel <linux-kernel@...r.kernel.org>,
        kernel-team@...roid.com, Minchan Kim <minchan@...nel.org>,
        Ramon Pantin <pantin@...gle.com>,
        Hugh Dickins <hughd@...gle.com>,
        Lokesh Gidra <lokeshgidra@...gle.com>,
        Michal Hocko <mhocko@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        aryabinin@...tuozzo.com, luto@...nel.org, bp@...en8.de,
        catalin.marinas@....com, Chris Zankel <chris@...kel.net>,
        dave.hansen@...ux.intel.com, elfring@...rs.sourceforge.net,
        fenghua.yu@...el.com, geert@...ux-m68k.org, gxt@....edu.cn,
        deller@....de, mingo@...hat.com, jejb@...isc-linux.org,
        jdike@...toit.com, Jonas Bonn <jonas@...thpole.se>,
        Julia Lawall <Julia.Lawall@...6.fr>,
        kasan-dev@...glegroups.com, kvmarm@...ts.cs.columbia.edu,
        lftan@...era.com, linux-alpha@...r.kernel.org,
        linux-hexagon@...r.kernel.org, linux-ia64@...r.kernel.org,
        linux-m68k@...ts.linux-m68k.org, linux-mips@...ux-mips.org,
        linux-mm <linux-mm@...ck.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,
        linux-snps-arc@...ts.infradead.org, linux-um@...ts.infradead.org,
        linux-xtensa@...ux-xtensa.org, Max Filippov <jcmvbkbc@...il.com>,
        nios2-dev@...ts.rocketboards.org,
        Peter Zijlstra <peterz@...radead.org>, richard@....at
Subject: Re: [PATCH v2 2/2] mm: speed up mremap by 500x on large regions

On Fri, Oct 12, 2018 at 07:25:08PM -0700, Daniel Colascione wrote:
[...] 
> > But anyway, I think this runtime detection thing is not needed. THP is
> > actually expected to be as fast as this anyway, so if that's available then
> > we should already be as fast.
> 
> Ah, I think the commit message is confusing. (Or else I'm misreading
> the patch now.) It's not quite that we're disabling the feature when
> THP is enabled anywhere, but rather that we use the move_huge_pmd path
> for huge PMDs and use the new code only for non-huge PMDs. (Right?) If
> that's the case, the commit message shouldn't say "Incase THP is
> enabled, the optimization is skipped". Even if THP is enabled on a
> system generally, we might use the new PMD-moving code for mapping
> types that don't support THP-ization, right?

That is true. Ok, I guess I can update the commit message to be more accurate
about that.

> > This is for non-THP where THP cannot be enabled
> > and there is still room for some improvement. Most/all architectures will be
> > just fine with this. This flag is more of a safety-net type of thing where in
> > the future if there is this one or two weird architectures that don't play
> > well, then they can turn it off at the architecture level by not selecting
> > the flag. See my latest patches for the per-architecture compile-time
> > controls. Ideally we'd like to blanket turn it on on all, but this is just
> > playing it extra safe as Kirill and me were discussing on other threads.
> 
> Sure. I'm just pointing out that the 500x performance different turns
> the operation into a qualitatively different feature, so if we expect
> to actually ship a mainstream architecture without support for this
> thing, we should make it explicit. If we're not, we shouldn't.

We can make it explicit by enabling it in such a mainstream architecture is
my point. Also if the optimization is not doing what its supposed to, then
userspace will also just know by measuring the time.

thanks,

 - Joel

Powered by blists - more mailing lists